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

Handling reconnect scenarios properly when socket is hung #1375

Merged

Conversation

markshiz
Copy link
Contributor

In situations where the socket connection is lost via a SO_ERROR, even though reconnect is enabled, the socket manager hangs the socket until a manual disconnect. Symptom would be: the app would output something like the following:

2019-01-09 12:15:49.005630+0000 App [13746:3543026] [] nw_socket_handle_socket_event [C10.1:1] Socket SO_ERROR [61: Connection refused]
2019-01-09 12:15:49.016640+0000 App [13746:3543026] [] nw_socket_handle_socket_event [C10.2:1] Socket SO_ERROR [61: Connection refused]

Socket-IO would be logging the retry requests over and over again with the following log:

Tried connecting an already active socket

In this case, the active socket is hung, so a retry would never be attempted. This condition can be simulated by running an app on a simulator while turning on Network Link Conditioner with 100% packet loss profile.

This fix simply recycles the engine in the case that a connecting status has 1 or more retry attempts. This seems to resolve the issue.

@WestFlow127
Copy link

@nuclearace this PR fixes the issue for 1415#

This bug is critical. Can we please get this merged?

@nuclearace nuclearace changed the base branch from master to development November 13, 2023 20:33
@nuclearace
Copy link
Member

@WestFlow127 Would you be able to test on development if I merge? I don't have capacity this week

@WestFlow127
Copy link

@nuclearace Yes, I can do that.

@nuclearace nuclearace merged commit 71a627c into socketio:development Nov 13, 2023
@WestFlow127
Copy link

Thank you! @nuclearace

@ricardopereira
Copy link

@WestFlow127 did it passed your testing?
Also, any idea when this fix will be released? Thanks

@WestFlow127
Copy link

WestFlow127 commented Jan 31, 2024

@ricardopereira Yes, I have just recently fully tested this against my companies socket server.
My testing passed, and worked well. I have tagged the repo owner for requesting a merge of this PR in master:
#1464

@WestFlow127 WestFlow127 mentioned this pull request Jan 31, 2024
@ricardopereira
Copy link

I also tested it. I was facing issues with reconnections and this PR fixes it. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants