Skip to content

Conversation

@KidkArolis
Copy link
Contributor

This would allow to override the removeAccessToken() method to conditionally not remove the token. In particular my use case is something like:

if (err.code === 401 || err.code === 404) {
  // remove the token
} else {
  // temporary issue, show an error screen
}

Not sure this implementation is the best, maybe an option of something like removeTokenOnError: err => bool in auth would be nicer?

@KidkArolis KidkArolis force-pushed the conditional-remove-token branch from fbb6b69 to 54b5656 Compare August 15, 2019 11:08
@KidkArolis
Copy link
Contributor Author

Introducing new options should probably be avoided.. so also wondering about alternative solutions.

@KidkArolis KidkArolis changed the title Pass error to removeAccessToken so that the behaviour can be customised Conditional removeAccessToken to not remove token on 401/404 Aug 16, 2019
@KidkArolis KidkArolis changed the title Conditional removeAccessToken to not remove token on 401/404 Conditional removeAccessToken for 401/404 case Aug 16, 2019
@daffl
Copy link
Member

daffl commented Aug 21, 2019

Could you maybe add a test case for how the flow would look like? It's also always possible to extend the client class and provide your own functionality.

@KidkArolis
Copy link
Contributor Author

I can add the test case, if you think this feature is something worth adding.

My thinking is that if we only cleared token on 401/404 errors, that might be a better default. Right now, if you drop a connection because the server is restarting (you only have one, or kubernetes is spinning your pod up and down), Feathers will try to reconnect (which is great), but if that errors with some temporary 502 Bad Gateway, the token gets wiped.

I'm ok with having to extend the service, but what pushed me to explore a PR is that there is no "good" way to extend. Pretty much the only option is to completely reimplement reAuthenticate method, since removeToken doesn't receive the error.

What I ended up doing instead:

  // feathers removes access token too agressively
  // in case of temporary server errors, e.g. 500 or timeouts
  // we don't want to remove the token
  feathers.authentication._removeAccessToken =
    feathers.authentication.removeAccessToken
  feathers.authentication.removeAccessToken = function done() {
    return Promise.resolve(true)
  }

And then calling feathers.authentication._removeAccessToken manually in my logout function. Although, this means that my reconnect might be broken now. Unless it keeps retrying.. Hm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants