Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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
Owner

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

@rauchg
Owner

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
Owner

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?

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 Owner
rauchg added a note

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
This page is out of date. Refresh to see the latest.
Showing with 23 additions and 1 deletion.
  1. +8 −0 lib/manager.js
  2. +5 −0 lib/socket.js
  3. +10 −1 test/connection.js
View
8 lib/manager.js
@@ -404,6 +404,14 @@ Manager.prototype.reconnect = function(){
debug('will wait %dms before reconnect attempt', delay);
this.reconnecting = true;
+
+ for (var nsp in this.nsps) {
+ var socket = this.nsps[nsp];
+ if (socket.shouldReconnect){
+ socket.emit('reconnecting');
+ }
+ }
+
var timer = setTimeout(function(){
debug('attempting reconnect');
self.emit('reconnect_attempt');
View
5 lib/socket.js
@@ -28,6 +28,7 @@ module.exports = exports = Socket;
var events = {
connect: 1,
disconnect: 1,
+ reconnecting: 1,
error: 1
};
@@ -53,6 +54,7 @@ function Socket(io, nsp){
this.buffer = [];
this.connected = false;
this.disconnected = true;
+ this.shouldReconnect = true;
}
/**
@@ -341,6 +343,9 @@ Socket.prototype.destroy = function(){
Socket.prototype.close =
Socket.prototype.disconnect = function(){
+ // shouldn't try to reconnect socket
+ this.shouldReconnect = false;
+
if (!this.connected) return this;
debug('performing disconnect (%s)', this.nsp);
View
11 test/connection.js
@@ -54,6 +54,15 @@ describe('connection', function() {
});
});
+ it('should emit reconnecting event', function(done) {
+ var socket = io();
+ socket.on('reconnecting', function(){
+ done();
+ });
+ socket.io.engine.close();
+ });
+
+
it('should try to reconnect twice and fail when requested two attempts with incorrect address and reconnect enabled', function(done) {
var manager = io.Manager('http://localhost:3940', { reconnection: true, reconnectionAttempts: 2, reconnectionDelay: 10 });
var socket = manager.socket('/asd');
@@ -226,4 +235,4 @@ if (global.Blob && null != textBlobBuilder('xxx')) {
done();
});
}
-});
+});
Something went wrong with that request. Please try again.