Conversation
… connected Previously, a connection that had been connected for longer than the suspend timer period (set in onConnected) would have immediately transitioned to suspended when the transport became unavailable. This is because the timer is not reset the first time the transport fails. This change fixes this by resetting the suspend timer when the transport becomes unavailable, if the current connection state is connected. Fixes #925
ikbalkaya
approved these changes
Mar 17, 2023
Contributor
ikbalkaya
left a comment
ikbalkaya
left a comment
There was a problem hiding this comment.
Two questions, but looks good to me
paddybyers
requested changes
Mar 20, 2023
Member
paddybyers
left a comment
paddybyers
left a comment
There was a problem hiding this comment.
Don't we also need to remove
?
Contributor
Author
There are places elsewhere (such as auth errors) that explicitly request the state Disconnected and rely on this line to reset the timer, so we'd want to make sure the timer is properly set in these instances. Admittedly, this would mean the timer gets set twice when we have Will have a look to see if this can be tided up a bit! |
This reverts commit 925f5f0.
Can lead to race conditions, plus `onTransportUnavailable` is inherently called by clearing
Much easier to see what's going on when debugging.
The connection re-attempt supersedes the transport, which prevents the onTransportUnavailable call from setting the suspension timer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, a connection that had been connected for longer than the suspend timer period (set in onConnected) would have immediately transitioned to suspended when the transport became unavailable. This is because the timer is not reset the first time the transport fails.
This change fixes this by resetting the suspend timer when the transport becomes unavailable, if the current connection state is connected.
Also included are some unit tests on the connection manager which force the connection into a desired state, and then test the behaviour.
Fixes #925