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

reconnect_failed gets never fired #652

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

reconnect_failed gets never fired #652

Pita opened this issue Nov 19, 2011 · 27 comments

Comments

@Pita
Copy link
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
Copy link

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
Copy link

rkirks commented Jan 30, 2012

+1

@Selvatico
Copy link

the same

@dhruv-bhatia
Copy link

+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
Copy link

Same, the issue is still here.

@edwardbc
Copy link

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
Copy link

rakyll commented Apr 17, 2012

Same issue here.

@nynexman4464
Copy link

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
Copy link

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

@robbrit
Copy link

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
Copy link

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

@veeru-as
Copy link

@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
Copy link

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.

@jrmyio
Copy link

jrmyio commented Sep 6, 2012

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

@jsilveira
Copy link

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

@3rd-Eden
Copy link
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.

@haddow777
Copy link

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
Copy link

Same.

@eephillip
Copy link

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

@matbee-eth
Copy link

Can we please get this pushed in? ffs.

@godratio
Copy link

godratio commented Mar 1, 2013

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

@kommander
Copy link

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
Copy link

@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
Copy link

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
Copy link

rfink commented Jun 3, 2013

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

@ludoblues
Copy link

Would be nice, still need it 2 years later.

@robpieke
Copy link

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests