martynsmith / node-irc

NodeJS IRC client library
GNU General Public License v3.0
1.33k stars 424 forks source link

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

Closed ericblade closed 8 years ago

ericblade commented 12 years ago

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..

villelahdenvuo commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

villelahdenvuo commented 12 years ago

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.

villelahdenvuo commented 12 years ago

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 commented 11 years ago

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

wizonesolutions commented 11 years ago

Is disabling flood protection the best workaround for this atm?

ward commented 8 years ago

I believe this was fixed by commit https://github.com/martynsmith/node-irc/commit/574f2b0a730e558eb23447af5ea374ed4e629b91

You can follow the trail via PR https://github.com/martynsmith/node-irc/pull/100 which was closed for bug https://github.com/martynsmith/node-irc/issues/138 which was closed by said commit.