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

Multiple sequential errors cause multiple reconnection attempts #125

Merged

Conversation

dpatty
Copy link
Contributor

@dpatty dpatty commented Jun 25, 2019

I was seeing some weird behavior with a spotty server connection.

When the server caused some sort of parse error or network exchange error, the EventSource client would attempt to reconnect to the server (via onConnectionClosed).
The problem was that in between recognizing the network error, and actually reconnecting, there was a chance for another error to also force a reconection attempt. This would result in a new connection attempted for each error in that window. Thus 1 connection to the server with 2 errors would result in 2 new connections to the server. (see images below)
Network error
Duplicating Requests

Repeat this a few times and you get a compounding problem:
More network errors
Duplicating Multiple Requests

At some point in this process the server began interfering with the error states meaning some connections were kept open without an error, but others would continue to proliferate double connections
SSE received preventing network error

I expected to only see a single re-connection after each error. After the fix this is the behavior.
After Fix

@dpatty dpatty changed the title Multiple sequential errors causes multiple requests to reconnect Multiple sequential errors causes multiple reconnection attempts Jun 25, 2019
@dpatty dpatty changed the title Multiple sequential errors causes multiple reconnection attempts Multiple sequential errors cause multiple reconnection attempts Jun 25, 2019
@rexxars rexxars merged commit 7a4627a into EventSource:master Jan 25, 2020
1 check passed
@rexxars
Copy link
Member

@rexxars rexxars commented Jan 25, 2020

Thanks for contributing!

icy-fish added a commit to icy-fish/eventsource that referenced this issue May 21, 2020
Since the PR EventSource#125 fixed duplicate connections after reconnection by using a `connectionInProgress` lock to avoid function `connect()` be called duplicately.

But it forgot to release the `connectionInProgress` lock when request error happends, in that case, our client can only retry for 1 time and never get the lock again.

So it's needed to release the `connectionInProgress` lock when error happends.

Signed-off-by: icy_fish <keith519@qq.com>
icy-fish added a commit to icy-fish/eventsource that referenced this issue May 21, 2020
Since the PR EventSource#125 fixed duplicate connections after reconnection by using a `connectionInProgress` lock to avoid function `connect()` be called duplicately.

But it forgot to release the `connectionInProgress` lock when request error happends, in that case, our client can only retry for 1 time and never get the lock again.

So it's needed to release the `connectionInProgress` lock when error happends.

Signed-off-by: icy_fish <keith519@qq.com>
icy-fish added a commit to icy-fish/eventsource that referenced this issue May 24, 2020
Since the PR EventSource#125 fixed duplicate connections after reconnection by using a `connectionInProgress` lock to avoid function `connect()` be called duplicately.

But it forgot to release the `connectionInProgress` lock when request error happends, in that case, our client can only retry for 1 time and never get the lock again.

So it's needed to release the `connectionInProgress` lock when error happends.

Signed-off-by: icy_fish <keith519@qq.com>
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.

None yet

3 participants