Send Keep-Alive Ping Immediately When Previous Ping Is Overdue #63195
Conversation
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
dotnet-policy-service
bot
added
the
community-contribution
|
Thanks for your PR, @@ReaganYuan. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
| @@ -726,7 +726,12 @@ export class HubConnection { | |||
| { | |||
| let nextPing = this._nextKeepAlive - new Date().getTime(); | |||
| if (nextPing < 0) { | |||
| nextPing = 0; | |||
| if (this._connectionState === HubConnectionState.Connected) { | |||
| this._sendMessage(this._cachedPingMessage).catch((e) => { | |||
There was a problem hiding this comment.
This change is different from what you showed me. The check is now inside the if (this._pingServerHandle === undefined) condition, but isn't the problem that we set a timer but it isn't being fired? So this new code will never be hit in the problematic case?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Send Keep-Alive Ping Immediately When Previous Ping Is Overdue
Trigger Client-to-Server Keep-Alive ping immediately if nextPing is overdue. The behavior should remain in parity as sending it immediately vs adding it to setTimeout with 0 delay are the same.
Description
We've observed that some browser clients occasionally enter a hidden (but not frozen) state, during which JavaScript timers are paused. As a result, the Client-to-Server Keep-Alive ping fails to fire, causing the server to reach the ClientTimeoutInterval and disconnect the client.
The client then attempts to reconnect, but because the browser remains in a hidden state, setTimeout continues to be paused, preventing the ping from firing again. This leads to repeated disconnections and reconnections until the browser either becomes frozen or returns to an active state. This behavior results in unnecessary reconnect traffic on the server.
Note: The frozen state works as expected, where Server will disconnect Client once the ClientTimeoutInterval has been exceeded. And the Client will not reconnect until the browser is unfrozen.
This issue doesn't occur consistently across all hidden browser states. We suspect it may be influenced by the following,
We've patched our SignalR client (v8.0.7) to immediately fire the Keep-Alive ping if nextPing is in the past, rather than relying on setTimeout, which may be paused. This change helped resolve the repeated reconnections for such sessions.
{Detail}