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

Reconnection fails if server holds connection #297

Closed
3rd-Eden opened this issue Sep 29, 2014 · 4 comments
Closed

Reconnection fails if server holds connection #297

3rd-Eden opened this issue Sep 29, 2014 · 4 comments

Comments

@3rd-Eden
Copy link
Member

There's a missing feature in the reconnection implementation which is the ability to specify a timeout for the reconnect attempt. Currently it's possible that you attempt to connect to a server which never answers the requests causing the request to hang indefinitely.

The easiest way to reproduce this is to trigger a reconnection from the server side and remove all the upgrade listeners from the server and add an empty function which will handle the request. For example:

primus.on('connection', function (spark) {
  setTimeout(function () {
    primus.server.removeAllListeners('upgrade');
    primus.server.on('upgrade', function () {});
    spark.end(null, { reconnect: true });
  }, 200);
});

While nobody in their sane minds would do something like the code above it's certainly possible that load balancers or proxies buffer the connection while an application is dead or restarting.

Originally reported by @developerdizzle which ran into these issues at Cloud9

@lpinca
Copy link
Member

lpinca commented Sep 29, 2014

I think that this is an issue of the proxy / load balancer.
That said, adding a timeout to our reconnection attempt is not a bad idea.

@3rd-Eden
Copy link
Member Author

@lpinca Yes, it's definitely an edge case, but still something we should support.

@3rd-Eden
Copy link
Member Author

3rd-Eden commented Oct 4, 2014

FYI: Wrote a new module to fix this behavior: http://npmjs.org/package/recovery . So this issue will be fixed once the browserify branch is completed and merged in to master (as the browserify branch implements this new reconnect module)

@3rd-Eden
Copy link
Member Author

Resolved in master

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

No branches or pull requests

2 participants