Detect ping timeout #76

Closed
csimi opened this Issue Dec 15, 2011 · 12 comments

Projects

None yet

8 participants

@csimi

I think there's no ping timeout detection in the library right now. I waited for around 5 minutes to see if something happens but nothing did.

Steps to reproduce:
1. Connect to a server.
2. Disconnect from the internet (disconnect button on router webui for example).
3. Reconnect to internet.
4. The library doesn't detect that you got ping timeouted from the server, therefore it doesn't even try to reconnect.

I'm gonna temporarily fix this issue by manually pinging the server every few minutes but this puts (a minimal amount from my single bot) extra stress on the server so you might want to find something more elegant.
15 Dec 21:59:39 - SEND: PING :PONG
15 Dec 21:59:39 - Connection got "close" event
15 Dec 21:59:39 - Disconnected: reconnecting
15 Dec 21:59:39 - Waiting 2000ms before retrying

@bpaf

Even worse, if after step 4 you do client.disconnect() and then client.connect() the process dies with the following Error: write EPIPE (on node 0.6.6): https://gist.github.com/1606000

@damianb

regarding this, a regular irc PING to the server is actually the standard solution for most clients.

@rretzbach

Thanks for the workaround csimi and damianb. I am currently using this to keep my irc web client online after a daily disconnect in the night:

setInterval(function(){client.send('PONG', 'empty');}, 5*60*1000);

Though I'd rather like node-irc to send a close event if a PING hasn't been received in a specified timeout. For example if a PING usually arrives every 90s then it would be odd if the last PING is 180s or 270s ago.

@damianb

@rretzbach from my observations, best to wait 5 minutes for ping timeout - perhaps after making a server ping yourself and waiting a minute.

@Jnull

Please Close this. Ping timeout support was added from #176

@Jnull

Oh okay, thanks, support was lost due to the code reconstruction. #214 should be reopened.

@jirwin
Collaborator

Done.

@JorgenPhid

This really needs to be implemented.

@philip-peterson

I'm working on this now, should be done by the end of the night.

@Jnull

Awesome! 👍

@jirwin jirwin added the enhancement label Jan 29, 2016
@jirwin
Collaborator

Fixed in #418.

@jirwin jirwin closed this Mar 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment