Skip to content
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

Add specific exception for auth errors being sent to to the Sync Session error handler #1643

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cmelchior
Copy link
Contributor

Closes #1640

This adds a specific exception class for auth errors being sent to the SyncSession error handler. This is relevant as this indicates the user's credentials are no longer valid and they need to log in again.

Some open questions:

  • Should it be possible to update the refresh token on an already open Realm? (this will require changes in Core)
  • It is a bit annoying to create an "auth" type exception under SyncException when we already have AuthException under AppException. But relaxing the interface would be a breaking change and besides, this is the only "App" type exception that would surface here anyway.
  • Naming... alternatives considered: SyncSessionExpiredException, SyncAuthException, SyncSessionAuthExpired. Choosing AuthExpiredException was somewhat arbitrary, except it felt short and to the point. Not sure if Sync or Session should somehow be part of the name to separate it more from the generic AuthException that is an AppException. All our other specific exception types end with Exception

@cla-bot cla-bot bot added the cla: yes label Jan 29, 2024
@cmelchior cmelchior marked this pull request as draft January 29, 2024 10:37
@cmelchior
Copy link
Contributor Author

@nirinchev believes that Core would automatically trigger a reconnect if you log the same user back in again. If this happens, users do not need to close the Realm. We need to test the behaviour.

@@ -79,6 +80,7 @@ internal fun convertSyncError(syncError: SyncError): SyncException {
val errorCode = syncError.errorCode
val message = createMessageFromSyncError(errorCode)
return when (errorCode.errorCode) {
ErrorCode.RLM_ERR_AUTH_ERROR -> AuthExpiredException(message)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also send this to the AuthenticationListener

Copy link
Contributor

@rorbech rorbech Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also send this to the AuthenticationListener

We should probably update the AuthenticationListener to take advantage of realm/realm-core#7302 instead of issuing these events ourselves.

Would also be fixed by #1676

@cmelchior cmelchior removed their assignment Jan 31, 2024
@rorbech rorbech self-assigned this Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When to do re-login after token expiration
2 participants