-
Notifications
You must be signed in to change notification settings - Fork 463
Use the same error message code across platforms #378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if (ex != null) { | ||
| promise.reject("Failed to fetch configuration", getErrorMessage(ex)); | ||
| promise.reject("token_refresh_failed", getErrorMessage(ex)); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be service_configuration_fetch_error right?
From https://github.com/openid/AppAuth-Android/releases Minor bug fixes: - Synchronizes multiple actions when requiring token refresh (FormidableLabs#332) - Make handling of non-standard expires_at more tolerant (FormidableLabs#336) - Changes related to Android tool changes between v25 and v27 (FormidableLabs#341, FormidableLabs#363) - Fix encoding of client ids and secrets for auth (FormidableLabs#345) - Handle CustomTabsSession.newSession failures (FormidableLabs#362) - Do not automatically pass scope on token exchange request (FormidableLabs#364) - Do not override tab title setting (FormidableLabs#365) - Respect default browser of the user correctly (FormidableLabs#379) - Updated custom tab definitions, including Firefox (FormidableLabs#378, FormidableLabs#383)
|
@kadikraman @henninghall Is there something pending still with this one? This would be a nice improvement to get released, as the errors are difficult to handle when there are platform differences. |
|
I'll give an example of what I'm doing and why I need these improvements to the error messages: Currently I have a problem that sometimes refreshing the token errors out because of network issues or the connection timing out. In these cases it makes sense to just retry the refresh as the token can be still refreshed. With these changes I could do something like this for both Android and iOS (the message might have some differences on Android/iOS) : // an example, not working or production ready code.
refresh(AUTH_CONFIG, { refreshToken })
.catch(err => {
if (err.code === "token_refresh_failed" && err.message === "something about bad network") {
refresh(AUTH_CONFIG, { refreshToken })
}
}) |
|
@kristerkari It's all good from my side. |
|
Released in v5.0.0 🎉 |
Fixes #377
Description
The error codes are not the same across platforms, e.g. when the token refresh fails, Android error code is
Failed to refresh tokenand iOS isRNAppAuth Error. This ensures that the error codes are the same across platforms.Error codes
The following error messages can be thrown:
service_configuration_fetch_error- could not fetch the service configurationauthentication_failed- user authentication failedtoken_refresh_failed- could not exchange the refresh token for a new JWT