Skip to content

disconnect does not appear to send the reason to the server #89

Closed
ericblade opened this Issue May 21, 2012 · 9 comments

7 participants

@ericblade

Calling disconnect("user pressed disconnect button") gives me a quit message of "Client Quit". Either it's not sending the message with the QUIT command, or it's just flat disconnecting before sending..

@tuhoojabotti

For me the disconnect doesn't do anything other than call the callback. I tried doing this.send('QUIT', 'message'); manually, but that didn't do anything either.

@ericblade

Occasionally, the quit message does come out, so it looks like it's actually disconnecting the connection without actually waiting to make sure that the quit command has happened.

@martynsmith
Owner

Hmmm, it seems to be working for me every time.

Perhaps it's over lower latency links or something.

Are you still experiencing this problem?, What IRC server are you seeing the issue on?

@ericblade

I've only used it on Freenode, from webOS TouchPads running node 0.4..

@tuhoojabotti

I have the following code for quitting. I'm running the bot on IRCNet on a local operator's server.

process.stdin.resume();
process.on('SIGINT', function () {
  console.log('%s received SIGINT', network.name.green);

  network.disconnect('SIGINT', function () {
    console.log('We have now quit from', network.name.green);
    setTimeout(function () { console.log('Still quit.'); }, 1000);
  });
});

What I get:

We have now quit from IRCNet
Still quit.

And on IRC: -!- tuhBot2 [^tuhoojabo@hilla.kapsi.fi] has quit [EOF From client]

I added the timeout to make sure that Node doesn't exit too soon, but it didn't make any difference. Edit: Using node 0.7.0 currently. Edit2: Tried with 0.7.12, no change.

@tuhoojabotti

I have found the root cause. If you use flood protection the send is queued in cmdQueue and the socket is closed before the queue is drained in Client.prototype.activateFloodProtection. If flood protection is off the message is sent correctly.

@valscion

👍 please fix this. I've made a dirty hack to overcome this problem but it's just that - a hack.

@wizonesolutions

Is disabling flood protection the best workaround for this atm?

@jirwin jirwin added the bug label Jan 29, 2016
@ward
ward commented Mar 27, 2016

I believe this was fixed by commit 574f2b0

You can follow the trail via PR #100 which was closed for bug #138 which was closed by said commit.

@jirwin jirwin closed this Mar 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.