Skip to content

fix(eio/upgrade): 25s upgrade latency due to multiple polling req while upgrading #487

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
merged 3 commits into from
Mar 21, 2025

Conversation

Totodore
Copy link
Owner

@Totodore Totodore commented Mar 15, 2025

Motivation

As reported in #479, the socket.io client might send multiple HTTP polling requests while upgrading. Socketioxide is sending exactly one NOOP packet to the first polling request and then do not expect more polling requests. When this is happening (probably because of some state desynchronisation on the client side). The client is waiting for the end of the second polling request which is caused by the ping request packet only after the ping interval (25s by default).

Solution

Socketioxide is now sending a NOOP packet to any number of incoming polling request if the socket is currently being upgraded.
Socketioxide is now pausing the heartbeat mechanism when upgrading to avoid resending a ping packet on the polling connection before the upgrade is fully complete.

@Totodore Totodore added C-Bug Something isn't working A-engineioxide Area related to engineioxide P-High High priority labels Mar 15, 2025
@Totodore Totodore force-pushed the fix-multiple-polling-req branch 2 times, most recently from a6af282 to 4f73318 Compare March 18, 2025 22:16
@Totodore Totodore merged commit c2ea005 into main Mar 21, 2025
30 checks passed
@Totodore Totodore deleted the fix-multiple-polling-req branch March 21, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-engineioxide Area related to engineioxide C-Bug Something isn't working P-High High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant