Skip to content

Don't swallow CancellationException #895

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

Merged

Conversation

sproctor
Copy link
Contributor

Add checks that the coroutine is still active in places that can catch CancellationException

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

#889

What is the new behavior?

Throws a CancellationException in Exception handling logic. See: https://medium.com/better-programming/the-silent-killer-thats-crashing-your-coroutines-9171d1e8f79b and Kotlin/kotlinx.coroutines#3658 for the rationale.

@jan-tennert
Copy link
Collaborator

Were you able to test if this fixes the problem in #889?

@sproctor
Copy link
Contributor Author

It fixes #889. The overall behavior seems to be better, but I'm not sure if it was these changes or something else. I'm still getting:

ClosedReceiveChannelException: Channel was closed
Error while listening for messages. Trying again in 7s

I think that might be unavoidable when the access token expires while connected. I guess I could add some logic to close the connection and refresh the token before it expires if this turns out to be problematic.

Thanks for your help and your work on this.

@jan-tennert
Copy link
Collaborator

I think that might be unavoidable when the access token expires while connected. I guess I could add some logic to close the connection and refresh the token before it expires if this turns out to be problematic.

Well under normal circumstances, this should not happen as this is already handled:

supabaseClient.pluginManager.getPluginOrNull(Auth)?.sessionStatus?.collect {
if(status.value == Realtime.Status.CONNECTED) {
when(it) {
is SessionStatus.Authenticated -> setAuth(it.session.accessToken)
is SessionStatus.NotAuthenticated -> {
if(config.disconnectOnSessionLoss) {
Realtime.logger.w { "No auth session found, disconnecting from realtime websocket"}
disconnect()
}
}
else -> {}
}
}
}

The tokens are refreshed via the Auth plugin 20% before their expiration time and Realtime waits for any refreshes or session changes

Copy link
Collaborator

@jan-tennert jan-tennert left a comment

Choose a reason for hiding this comment

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

Thanks!

@jan-tennert jan-tennert merged commit 705fff9 into supabase-community:master Apr 13, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants