chore: return failed refresh errors on external auth as string (was boolean) #13402
Conversation
Failed refreshes should return errors. These errors are captured as validate errors. A fair classification of the the error
I was considering that too, I just dislike that I would need to return an If I was to go that route, I'd remove the boolean return entirely. Just Let me give that a go with a custom error type and see how it looks |
|
Sweet! |
| @@ -95,17 +95,31 @@ func (c *Config) GenerateTokenExtra(token *oauth2.Token) (pqtype.NullRawMessage, | |||
| }, nil | |||
| } | |||
|
|
|||
| // InvalidTokenError is a case where the "RefreshToken" failed to complete | |||
| // as a result of invalid credentials. Error contains the reason of the failure. | |||
| type InvalidTokenError string | |||
There was a problem hiding this comment.
nit: might wanna make this a struct instead, just so we can do e.ValidationMessage or something!
There was a problem hiding this comment.
It's more an InvalidationMessage, but more specifically it could be related to refreshing, a deployment configuration (disabled refreshes), or a validation error.
In all cases, it is an error. And the error is related to being invalid for w/e reason. I think continuing to use it as an err.Error() makes sense
Failed refreshing of tokens should return the error for debugging. The function no longer returns a boolean, and only returns an error. So the error can be checked if it is a validation error, or a runtime error.
This was added because multiple reports of failed refreshes have occurred. Debugging these failures are extremely difficult since errors are silenced. This will begin to raise said errors.
We might want to clean up errors from IDPs in the future if they get a bit too technical in their verbage.