martynsmith / node-irc

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

Crash if disconnected by the server while sending queued flood-protected messages #491

Open Protected opened 7 years ago

Protected commented 7 years ago
TypeError: Cannot read property 'requestedDisconnect' of null
    at Client.send (.../node_modules/irc/lib/irc.js:936:19)
    at Timeout.dequeue (.../node_modules/irc/lib/irc.js:966:22)
    at Timeout.wrapper [as _onTimeout] (timers.js:425:11)
    at tryOnTimeout (timers.js:232:11)
    at Timer.listOnTimeout (timers.js:202:5)

It doesn't matter why it happens, I believe. This crash should occur if the connection is lost half way through for any reason.

Client.prototype.send = function(command) {
    var args = Array.prototype.slice.call(arguments);

    // Note that the command arg is included in the args array as the first element

    if (args[args.length - 1].match(/\s/) || args[args.length - 1].match(/^:/) || args[args.length - 1] === '') {
        args[args.length - 1] = ':' + args[args.length - 1];
    }

    if (this.opt.debug)
        util.log('SEND: ' + args.join(' '));

    if (!this.conn.requestedDisconnect) {  // ########## this.conn is null! ##########
        this.conn.write(args.join(' ') + '\r\n');
    }
};

It's probably enough to check if conn is set in the if?

if (this.conn && !this.conn.requestedDisconnect) {
    this.conn.write(args.join(' ') + '\r\n');
}
paladox commented 7 years ago

would you be able to create a pull please?

I also have the same problem as you #485

Protected commented 7 years ago

I'm sorry, I don't know how I missed yours.

impca commented 7 years ago

If you disable cyclingPingTimer (you can just comment out //this.conn.cyclingPingTimer.start(); in irc.js), your connection won't get interrupted when you have too many queued messages. That means your hubot-irc adapter will no longer disconnect your bot when sending large amount of messages.