reconnect_failed gets never fired #652

Closed
Pita opened this Issue Nov 19, 2011 · 27 comments

Comments

Projects
None yet
@Pita
Contributor

Pita commented Nov 19, 2011

If I stop the node server so every reconnect of the client fails, the reconnect_failed event gets never fired

Here is the position in the code where reconnect_failed should get fired https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js#L501

for some reason it never enters the else part

@n1mmy

This comment has been minimized.

Show comment Hide comment
@n1mmy

n1mmy Dec 14, 2011

I am having the same issue. It is reproducible with a simple test case:

app.js:

var io = require('socket.io').listen(4000);

io.sockets.on('connection', function (socket) {
  socket.emit('news', { hello: 'world' });
  socket.on('my other event', function (data) {
    console.log(data);
  });
});

app.html:

<html>
<head>
<script src="http://localhost:4000/socket.io/socket.io.js"></script>
<script>
  var socket = io.connect('http://localhost:4000',
    {'reconnection delay': 100,
     'max reconnection attempts': 4});
  socket.on('news', function (data) {
    console.log(data);
    socket.emit('my other event', { my: 'data' });
  });

  socket.on('connect', function () {
    console.log("connect");
  });
  socket.on('disconnect', function () {
    console.log("disconnect");
  });
  socket.on('connecting', function (x) {
    console.log("connecting", x);
  });
  socket.on('connect_failed', function () {
    console.log("connect_failed");
  });
  socket.on('close', function () {
    console.log("close");
  });
  socket.on('reconnect', function (a, b) {
    console.log("reconnect", a, b);
  });
  socket.on('reconnecting', function (a, b) {
    console.log("reconnecting", a, b);
  });
  socket.on('reconnect_failed', function () {
    console.log("reconnect_failed");
  });

</script>
</head>
<body>
hello world
</body>
</html>

Run node on the app.js file. Open the HTML file in a browser. Open the debug console in the browser. Note that you got a 'connected' event. Control-c the node server. Note that you get a bunch of reconnecting messages, but never a reconnection_failed.

The reconnection options don't seem to make a difference. I included them here so you don't have to wait a long time to see the failure.

n1mmy commented Dec 14, 2011

I am having the same issue. It is reproducible with a simple test case:

app.js:

var io = require('socket.io').listen(4000);

io.sockets.on('connection', function (socket) {
  socket.emit('news', { hello: 'world' });
  socket.on('my other event', function (data) {
    console.log(data);
  });
});

app.html:

<html>
<head>
<script src="http://localhost:4000/socket.io/socket.io.js"></script>
<script>
  var socket = io.connect('http://localhost:4000',
    {'reconnection delay': 100,
     'max reconnection attempts': 4});
  socket.on('news', function (data) {
    console.log(data);
    socket.emit('my other event', { my: 'data' });
  });

  socket.on('connect', function () {
    console.log("connect");
  });
  socket.on('disconnect', function () {
    console.log("disconnect");
  });
  socket.on('connecting', function (x) {
    console.log("connecting", x);
  });
  socket.on('connect_failed', function () {
    console.log("connect_failed");
  });
  socket.on('close', function () {
    console.log("close");
  });
  socket.on('reconnect', function (a, b) {
    console.log("reconnect", a, b);
  });
  socket.on('reconnecting', function (a, b) {
    console.log("reconnecting", a, b);
  });
  socket.on('reconnect_failed', function () {
    console.log("reconnect_failed");
  });

</script>
</head>
<body>
hello world
</body>
</html>

Run node on the app.js file. Open the HTML file in a browser. Open the debug console in the browser. Note that you got a 'connected' event. Control-c the node server. Note that you get a bunch of reconnecting messages, but never a reconnection_failed.

The reconnection options don't seem to make a difference. I included them here so you don't have to wait a long time to see the failure.

@rkirks

This comment has been minimized.

Show comment Hide comment
@rkirks

rkirks Jan 30, 2012

+1

rkirks commented Jan 30, 2012

+1

@Selvatico

This comment has been minimized.

Show comment Hide comment
@Selvatico

Selvatico Feb 16, 2012

the same

the same

@dhruv-bhatia

This comment has been minimized.

Show comment Hide comment
@dhruv-bhatia

dhruv-bhatia Apr 10, 2012

+1, here's how I'm dealing with it:

Client side code (Coffeescript):

# Set max reconnects here
max_reconnects = 5

socket = io.connect("http://localhost:3000",
  "reconnection delay": 100
  "max reconnection attempts": max_reconnects
)

socket.on "reconnecting", (delay, attempt) ->
    console.log "attempting reconnect - " + attempt
    if attempt is max_reconnects
        console.log "all reconnect attempts failed"
        # Show the user an error etc.

This will return the following in the console.log after you Ctrl+C the server:

  1. attempting reconnect - 1
  2. attempting reconnect - 2
  3. attempting reconnect - 3
  4. attempting reconnect - 4
  5. attempting reconnect - 5
  6. all reconnect attempts failed

+1, here's how I'm dealing with it:

Client side code (Coffeescript):

# Set max reconnects here
max_reconnects = 5

socket = io.connect("http://localhost:3000",
  "reconnection delay": 100
  "max reconnection attempts": max_reconnects
)

socket.on "reconnecting", (delay, attempt) ->
    console.log "attempting reconnect - " + attempt
    if attempt is max_reconnects
        console.log "all reconnect attempts failed"
        # Show the user an error etc.

This will return the following in the console.log after you Ctrl+C the server:

  1. attempting reconnect - 1
  2. attempting reconnect - 2
  3. attempting reconnect - 3
  4. attempting reconnect - 4
  5. attempting reconnect - 5
  6. all reconnect attempts failed
@maxailloud

This comment has been minimized.

Show comment Hide comment
@maxailloud

maxailloud Apr 12, 2012

Same, the issue is still here.

Same, the issue is still here.

@edwardbc

This comment has been minimized.

Show comment Hide comment
@edwardbc

edwardbc Apr 12, 2012

Same here, testing on 0.9.5. The problem happens in the following block:
https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js#L520

Plus, redoTransports is always undefined on the first time. Seems this is not considering when try multiple transports has been disabled, which should make the the reconnect_failed to always fire in those cases as well.

Same here, testing on 0.9.5. The problem happens in the following block:
https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js#L520

Plus, redoTransports is always undefined on the first time. Seems this is not considering when try multiple transports has been disabled, which should make the the reconnect_failed to always fire in those cases as well.

@rakyll

This comment has been minimized.

Show comment Hide comment
@rakyll

rakyll Apr 17, 2012

Same issue here.

rakyll commented Apr 17, 2012

Same issue here.

@nynexman4464

This comment has been minimized.

Show comment Hide comment
@nynexman4464

nynexman4464 Apr 25, 2012

I ran into this problem as well. I've definitely found the cause, hopefully a fix. redoTransports is undefined at first because socket.io only tries multiple transports on the last reconnection attempt (this is a side note in the config doc). I think the intention here is to do one last connection attempt with all transports, and finally give up.

The problem is, the handshake is always done with XHR or jsonp. If the server is not reachable, the handshake will always fail. In the handshake code, there is a line:
if (xhr.status == 200) {
complete(xhr.responseText);
} else {
!self.reconnecting && self.onError(xhr.responseText);
}
}
this bizzare, hard to read line !self.reconnecting && self.onError(xhr.responseText); means that self.onError will only be called if self.reconnecting is false. (I don't like this kind of shorthand - use an else if). Normally, there is a reconnect timer that will fire, triggering another call to reconnect. However, the timer is not set up on the last reconnection attempt. This code path never runs, meaning reconnect_failed is never called.

The solution I think is to also start the timer on the final reconnection attempt. Once I figure out how, I'll submit a pull request for a patch.

I ran into this problem as well. I've definitely found the cause, hopefully a fix. redoTransports is undefined at first because socket.io only tries multiple transports on the last reconnection attempt (this is a side note in the config doc). I think the intention here is to do one last connection attempt with all transports, and finally give up.

The problem is, the handshake is always done with XHR or jsonp. If the server is not reachable, the handshake will always fail. In the handshake code, there is a line:
if (xhr.status == 200) {
complete(xhr.responseText);
} else {
!self.reconnecting && self.onError(xhr.responseText);
}
}
this bizzare, hard to read line !self.reconnecting && self.onError(xhr.responseText); means that self.onError will only be called if self.reconnecting is false. (I don't like this kind of shorthand - use an else if). Normally, there is a reconnect timer that will fire, triggering another call to reconnect. However, the timer is not set up on the last reconnection attempt. This code path never runs, meaning reconnect_failed is never called.

The solution I think is to also start the timer on the final reconnection attempt. Once I figure out how, I'll submit a pull request for a patch.

@eephillip

This comment has been minimized.

Show comment Hide comment
@eephillip

eephillip May 23, 2012

+1 I can't get connect_failed or reconnect_failed to emit

+1 I can't get connect_failed or reconnect_failed to emit

@robbrit

This comment has been minimized.

Show comment Hide comment
@robbrit

robbrit Jun 11, 2012

I managed to fix the problem by changing this section:

if (!self.redoTransports) {
  self.on('connect_failed', maybeReconnect);
  self.options['try multiple transports'] = true;
  self.transport = self.getTransport();
  self.redoTransports = true;
  self.connect();
} else {
  self.publish('reconnect_failed');
  reset();
}

After the self.connect() line I added:

self.publish('reconnecting', self.reconnectionDelay, self.reconnectionAttempts);
self.reconnectionTimer = setTimeout(maybeReconnect, self.reconnectionDelay);

This causes the event to get fired properly after the last failed attempt.

robbrit commented Jun 11, 2012

I managed to fix the problem by changing this section:

if (!self.redoTransports) {
  self.on('connect_failed', maybeReconnect);
  self.options['try multiple transports'] = true;
  self.transport = self.getTransport();
  self.redoTransports = true;
  self.connect();
} else {
  self.publish('reconnect_failed');
  reset();
}

After the self.connect() line I added:

self.publish('reconnecting', self.reconnectionDelay, self.reconnectionAttempts);
self.reconnectionTimer = setTimeout(maybeReconnect, self.reconnectionDelay);

This causes the event to get fired properly after the last failed attempt.

@eephillip

This comment has been minimized.

Show comment Hide comment
@eephillip

eephillip Jun 12, 2012

Nice, the connection failure event is trigging for me with that mod.

Nice, the connection failure event is trigging for me with that mod.

@veeru-as

This comment has been minimized.

Show comment Hide comment
@veeru-as

veeru-as Jun 15, 2012

@robbrit i just found that the changes what you mentioned are already there in my socket.io.js but still the reconnect_failed event is not getting fired. And also one more thing what i found it, even if i specify 10 reconnect attempts it tries only 5 or 6 times and stops. Please any one help to solve this.

@robbrit i just found that the changes what you mentioned are already there in my socket.io.js but still the reconnect_failed event is not getting fired. And also one more thing what i found it, even if i specify 10 reconnect attempts it tries only 5 or 6 times and stops. Please any one help to solve this.

@robbrit

This comment has been minimized.

Show comment Hide comment
@robbrit

robbrit Jun 15, 2012

@veeru-as The first code block is what is already there, the second code block is what I added. Also keep in mind that the reconnect delays are exponential, so after a while it seems like it has stopped but it is just waiting a long time.

robbrit commented Jun 15, 2012

@veeru-as The first code block is what is already there, the second code block is what I added. Also keep in mind that the reconnect delays are exponential, so after a while it seems like it has stopped but it is just waiting a long time.

@ConneXNL

This comment has been minimized.

Show comment Hide comment
@ConneXNL

ConneXNL Sep 6, 2012

Is there any partical reason why this reconnect_failed is in the docs but it never triggers?

ConneXNL commented Sep 6, 2012

Is there any partical reason why this reconnect_failed is in the docs but it never triggers?

@jsilveira

This comment has been minimized.

Show comment Hide comment
@jsilveira

jsilveira Nov 14, 2012

Is there any reason why this bug has not been fixed??

Is there any reason why this bug has not been fixed??

@3rd-Eden

This comment has been minimized.

Show comment Hide comment
@3rd-Eden

3rd-Eden Nov 14, 2012

Contributor

Because you didn't make a pull request for it yet?

On 14 nov. 2012, at 05:31, jsilveira notifications@github.com wrote:

Is there any reason why this bug has not been fixed??


Reply to this email directly or view it on GitHub.

Contributor

3rd-Eden commented Nov 14, 2012

Because you didn't make a pull request for it yet?

On 14 nov. 2012, at 05:31, jsilveira notifications@github.com wrote:

Is there any reason why this bug has not been fixed??


Reply to this email directly or view it on GitHub.

@haddow777

This comment has been minimized.

Show comment Hide comment
@haddow777

haddow777 Dec 15, 2012

I went through it in chrome. The variable causing the issue is "self.redoTransports" because it is always undefined. This is most likely a breakdown in the config section of the code.

I went through it in chrome. The variable causing the issue is "self.redoTransports" because it is always undefined. This is most likely a breakdown in the config section of the code.

@dizballanze

This comment has been minimized.

Show comment Hide comment
@dizballanze

dizballanze Dec 17, 2012

Same.

Same.

@eephillip

This comment has been minimized.

Show comment Hide comment
@eephillip

eephillip Jan 17, 2013

The fix from robbrit still appears to work for me in 0.9.11.

The fix from robbrit still appears to work for me in 0.9.11.

@acidhax

This comment has been minimized.

Show comment Hide comment
@acidhax

acidhax Feb 14, 2013

Can we please get this pushed in? ffs.

acidhax commented Feb 14, 2013

Can we please get this pushed in? ffs.

@godratio

This comment has been minimized.

Show comment Hide comment
@godratio

godratio Mar 1, 2013

@robbrit Thanks for the fix. Helped me fix the issue I was having.

godratio commented Mar 1, 2013

@robbrit Thanks for the fix. Helped me fix the issue I was having.

@kommander

This comment has been minimized.

Show comment Hide comment
@kommander

kommander Apr 22, 2013

Can anybody tell what redoTransports does? I cannot find it anywhere throughout the codebase and it is never set, so it always enters the if(!self.redoTransports)... which sucks. Removing the negation for the condition works just fine then: if(self.redoTransports)...

Can anybody tell what redoTransports does? I cannot find it anywhere throughout the codebase and it is never set, so it always enters the if(!self.redoTransports)... which sucks. Removing the negation for the condition works just fine then: if(self.redoTransports)...

@kurteknikk

This comment has been minimized.

Show comment Hide comment
@kurteknikk

kurteknikk May 27, 2013

@robbrit Have you ever had any problems with your fix ?

I implemented your fix, it fixed the issue with reconnect_failed BUT something seems to be going wrong with re-connecting to the server, the client is creating more 'new connections' than it should.

Maybe it's just how i'm starting socket.io from the client, these are my options.

socket = io.connect(url, {
        'connect timeout': 5000, // 5 seconds should be enough
        'try multiple transports': true,
        'reconnect': true,
        'reconnection delay': 500,
        'reconnection limit': 5000,
        'max reconnection attempts': 3,
        'sync disconnect on unload': false,
        'auto connect': true,
        'flash policy port': 10843,
        'force new connection': true //IMP: to be able to re-connect using our own logic
    });

I'm posting this just for the sake of other people who might come across the same issue. I'm not using re-connect failed anymore because i switched to: 'max reconnection attempts': Infinity

@robbrit Have you ever had any problems with your fix ?

I implemented your fix, it fixed the issue with reconnect_failed BUT something seems to be going wrong with re-connecting to the server, the client is creating more 'new connections' than it should.

Maybe it's just how i'm starting socket.io from the client, these are my options.

socket = io.connect(url, {
        'connect timeout': 5000, // 5 seconds should be enough
        'try multiple transports': true,
        'reconnect': true,
        'reconnection delay': 500,
        'reconnection limit': 5000,
        'max reconnection attempts': 3,
        'sync disconnect on unload': false,
        'auto connect': true,
        'flash policy port': 10843,
        'force new connection': true //IMP: to be able to re-connect using our own logic
    });

I'm posting this just for the sake of other people who might come across the same issue. I'm not using re-connect failed anymore because i switched to: 'max reconnection attempts': Infinity

@robbrit

This comment has been minimized.

Show comment Hide comment
@robbrit

robbrit May 27, 2013

@kurteknikk: we had a problem where we would get connections, but they would never fire the disconnect event and so the server would never know when the clients would disconnect. I didn't bother fixing it, we ended up switching over to SockJS, which solved a lot of the problems we were having.

robbrit commented May 27, 2013

@kurteknikk: we had a problem where we would get connections, but they would never fire the disconnect event and so the server would never know when the clients would disconnect. I didn't bother fixing it, we ended up switching over to SockJS, which solved a lot of the problems we were having.

@rfink

This comment has been minimized.

Show comment Hide comment
@rfink

rfink Jun 3, 2013

Can I suggest this at least goes in for the 1.0 release?

rfink commented Jun 3, 2013

Can I suggest this at least goes in for the 1.0 release?

@mokesmokes mokesmokes referenced this issue in socketio/socket.io-client Jul 17, 2013

Closed

reconnection fixes #564

@ludoblues

This comment has been minimized.

Show comment Hide comment
@ludoblues

ludoblues Feb 4, 2014

Would be nice, still need it 2 years later.

Would be nice, still need it 2 years later.

@robpieke

This comment has been minimized.

Show comment Hide comment
@robpieke

robpieke Mar 30, 2014

Agreed ... just ran into it myself this morning :(

Agreed ... just ran into it myself this morning :(

jbg-zuba added a commit to JackboxGames/socket.io-client that referenced this issue Dec 11, 2014

agilethomas added a commit to agilethomas/socket.io-client that referenced this issue May 5, 2015

This issue was closed.

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