EventSource / eventsource

EventSource client for Node.js and Browser (polyfill)
MIT License
911 stars 252 forks source link

Duplicate connection seen when the server goes down and comes up again #89

Open sakathvenu opened 7 years ago

sakathvenu commented 7 years ago

Lets suppose the eventsource client has connected to the server and is receiving messages.

Lets suppose the server goes down The eventsource client callback onerror is fired The eventsource client keeps re-trying to connect The server comes online again Observation - eventsource client now makes two connections to the server.

Further - each message that the server sends is received twice by the eventsource client, due to the duplicate connection.

Is the above not a valid usecase to be supported by the module? Curious to know the reason why it was not considered?

aslakhellesoy commented 7 years ago

I'm unable to reproduce this with the example. How can we reproduce this?

sakathvenu commented 7 years ago

Thanks for the reply. The following setup should help reproduce the issue. environment : Nodejs, Visual Code

I created a simple sse server (using the sse-server.js from example) in VS code and running it. I created a simple sse-client (using sse-client.js from example) in VS Code and running it.

(Note - I never bring down the client at any point) Later - I bring down the server. Later - I run the server again. Observe the terminal in both server and client to see duplicate messages coming through

Please see the attachments below...

1 first time after server start and client start works as expected

2 after server restart once

3 after server restart four times

sakathvenu commented 7 years ago

Sidenote : https://github.com/tigertext/eventsource

seems to have solved this precise issue. please refer the changes section.

However - In the spirit of completeness...I hope the issue gets resolved in this amazing module as well.

Cheers

aslakhellesoy commented 7 years ago

If @mwolson submits a pull request with tests, we'd be happy to include this in the next release.

mwolson commented 7 years ago

@sakathvenu while trying to write a test for this, I wasn't able to repro the problem anymore, even with the same version of node.js I see in your debugger, and even using VSCode Insiders Build on OS X with a launch.json file to set up debug profiles for the sse-server and sse-client. Can you give me the git commit ID of the version of eventsource you have locally (using git show-ref HEAD), and your OS version?

sakathvenu commented 7 years ago

@mwolson

fullscreen capture

--> Client and Server.zip

End of message

shesek commented 6 years ago

I'm also seeing the same behavior. When the connection drops, two error events are being fired on the same EventSource object. I can then see two connection attempts being made every reconnectInterval in the developer tools. When the server goes back online, I see two successful connections and get two open events emitted from the same object. When the connection drops again, it grows to 4 connections. And the next time to 8, etc.

Using the browser's native EventSource instead of the polyfill resolved the issue. (my code accidentally always used the polyfill even when the native one was available)

I'm testing with Chromium 65.

eatnlunch commented 5 years ago

I know this is an old issue report, but we have seen the issue as well. As mentioned previously by @sakathvenu , this issue does appear to have been addressed quite simply in the forked https://github.com/tigertext/eventsource version of this module. Here is the commit. It would be great to see this fix applied and tested in this module.

hayden-t commented 5 years ago

ditto

icy-fish commented 4 years ago

Updates: Just notice that this PR #125 seems to fix this issue, when will it be released?

Got this issue too. Similar case with @sakathvenu , I hacked into the source file and found same thing as @shesek noted, when connection drops, two events are fired, one for http response close, the other for http request error, then onConnectionClosed was called twice.

I think the key to this issue is to avoid call function connect duplicately in ononConnectionClosed.

Although this commit in tigertext/eventsource seems resolved this issue, but it didn't prevent multiple workers from establish a connection between server and client, and that may end up with duplicate TCP connections. It works fine from first look beacause only one connection was set for data handling, but it do create multiple connections (can be checked by nestat), and rest of them are just hanging there and doing nothing.

BlackGlory commented 3 years ago

This package was last released in 2018. Now this problem can only be solved by importing the Github repository:

yarn add https://github.com/EventSource/eventsource#46fe04e03e54f4129a28bf75b3a1e5f4ab68b52a