Add reconnecting event and relevant test #631

Closed
wants to merge 5 commits into
from

2 participants

@poohlty

@guille A pull request to make a Manager emits 'reconnecting' event when trying to reconnect. Also a test case is included :)

@rauchg

@poohlty we need to emit the reconnection events on the sockets, not the manager.

@rauchg

Aka if you do

var socket1 = io('/a');
var socket2 = io('/b');
socket2.disconnect();

And then you lose connection, socket1 should emit reconnecting but socket2 shouldn't.

@rauchg

Also, there are conflicts that prevent this from being mergeable. Same with the other pull request on socket.io

@poohlty

Ah, make sense. How do I know when the socket should emit 'reconnecting' though? It seems to me that the reconnecting logic is handled by a manager, which is hidden to a socket?

@poohlty

@guille do I need more updates on this?

@rauchg rauchg commented on an outdated diff Apr 3, 2014
test/connection.js
@@ -44,6 +44,15 @@ describe('connection', function() {
});
});
+ it('should emit reconnecting event', function(done) {
+ var socket = io('/a');
+ socket.on('reconnecting', function(){
+ socket.close();
+ done();
+ });
+ socket.io.reconnect();
@rauchg
rauchg added a line comment Apr 3, 2014

Since this is not public API, i'd rather us test by forcing a disconnetion, and expecting a reconnection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@poohlty

@guille Now the test closes the engine rather than calling reconnect :)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment