maxLineLength is not set by default and can crash the bot

[swichers]

If you connect with the bot and try to send something before _updateMaxLineLength is called then the bot will crash. This code should duplicate the problem by trying to message NickServ before _updateMaxLineLength has been called.

var irc = require('irc');
var NickServ = require('nickserv');

var bot = new irc.Client('irc.example.com',
  'registerednick', {
  channels: ['#example'],
});

var nickserv = new NickServ('registerednick', {
  password: 'password',
  email: '[email protected]',
});
nickserv.attach('irc', bot);

nickserv.ready(function(err) {
  if (err) throw err;
});

[ghost]

can you post the error that you get? This is because there is no "onConnection" or "onReady" events yet, and as far as i know you only need to try to send a message to get the same error: i.e. client.say('#yourchannel', "I'm a bot!");

[swichers]

/mnt/nfs/projects/irc/node_modules/irc/lib/irc.js:926
    if (c.match(/\s/)) {
         ^

TypeError: Cannot read property 'match' of undefined
    at Client._splitLongLines (/mnt/nfs/projects/irc/node_modules/irc/lib/irc.js:926:10)
    at /mnt/nfs/projects/irc/node_modules/irc/lib/irc.js:966:36
    at Array.forEach (native)
    at Client._speak (/mnt/nfs/projects/irc/node_modules/irc/lib/irc.js:965:12)
    at Client.say (/mnt/nfs/projects/irc/node_modules/irc/lib/irc.js:952:10)
    at Nick.send (/mnt/nfs/projects/irc/node_modules/nickserv/lib/nickserv.js:76:27)
    at Nick.module.exports.Nick._nickserv (/mnt/nfs/projects/irc/node_modules/nickserv/lib/nickserv.js:112:19)
    at Nick.module.exports.Nick.info (/mnt/nfs/projects/irc/node_modules/nickserv/lib/nickserv.js:243:19)
    at Nick.module.exports.Nick.isRegistered (/mnt/nfs/projects/irc/node_modules/nickserv/lib/nickserv.js:195:19)
    at Nick.module.exports.Nick.ready (/mnt/nfs/projects/irc/node_modules/nickserv/lib/nickserv.js:167:21)

The notice event that nickserv uses fires before _updateMaxLineLength gets called. This means when nickserv tries to use say it crashes the bot. Forcing an early default (bot init time) for the max line length resolves the problem.

[ghost]

Right, so this causes the same error:

var irc = require('irc');

var client = new irc.Client('irc.example.com',
  'registerednick', {
  channels: ['#example'],
});

client.say('#yourchannel', "I'm a bot!");

Can you post your temporary work around too?

[swichers]

Ah, well my workaround was to not use the NickServ helper because it didn't seem to want to work even after fixing this problem. :)

Here is what I did just to get on with my testing:

Starting at line 95, note the self.maxLineLength.

    if (self.opt.floodProtection) {
        self.activateFloodProtection();
    }

    self.hostMask = '';
    self.maxLineLength = 450;

    // TODO - fail if nick or server missing
    // TODO - fail if username has a space in it
    if (self.opt.autoConnect === true) {
        self.connect();
    }

I don't think this is the appropriate workaround, but it is a quick fix with minimal impact.

[ghost]

That gives a whole new error :(

C:\Users\Void\corbanisking\node_modules\irc\lib\irc.js:751
                        throw err;
                              ^
Error: Uncaught, unspecified "error" event.
    at Error (native)
    at Client.emit (events.js:87:13)
    at Client.<anonymous> (C:\Users\Void\corbanisking\node_modules\irc\lib\irc.js:592:26)
    at Client.emit (events.js:107:17)
    at iterator (C:\Users\Void\corbanisking\node_modules\irc\lib\irc.js:748:26)
    at Array.forEach (native)
    at Socket.<anonymous> (C:\Users\Void\corbanisking\node_modules\irc\lib\irc.js:743:15)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)

Perhaps those TODO's would actually solve this:

    // TODO - fail if nick or server missing
    // TODO - fail if username has a space in it

I'm not sure where the server sub-object lives, but my guess that needs to be checked and return a code error, i haven't looked at the ./lib/codes.js but there may already be a code for "the server is currently not available" or something similar.

[brandonros]

I have the exact same error trying to send something I'm guessing too quickly. I do see a 'ready' event. Would that be useful to wait for?

[ghost]

sorry for beating around the bush, but after i went through the docs again i found that this does indeed work:

var irc = require('irc');
var client = new irc.Client('irc.example.com',
  'NickNameHere', {
  channels: ['#testing'],
});

client.addListener('registered', function () {
client.say('#testing', 'This test message is sent after the client connects');
});

@brandonros i tried as a 'ready' event, but that did not work, where did you see that?

[moshmage]

@jnull that works BECAUSE the message waits untill the server tells the user that that nickname is registered. if you try to client.say without a addListener you'll be in for a error log :)

I propose, as a workaround, to add maxLineLength to the self.opt object and have it default to the RFC and customizable as a init-option. On the long run, those rpl_welcome we are ignoring? AFAIK maxLineLength comes in those ^^'

[swichers]

@jnull I can confirm that this works because it is ultimately the solution I went with. The NickServ module goes about things a different way which is how I ran into the problem. It does seem like a bug to me, and it sounds like @moshmage's idea is the way to go.

[ghost]

@moshmage

Lines 116-131:

            case 'rpl_welcome':
                // Set nick to whatever the server decided it really is
                // (normally this is because you chose something too long and
                // the server has shortened it
                self.nick = message.args[0];
                // Note our hostmask to use it in splitting long messages.
                // We don't send our hostmask when issuing PRIVMSGs or NOTICEs,
                // of course, but rather the servers on the other side will
                // include it in messages and will truncate what we send if
                // the string is too long. Therefore, we need to be considerate
                // neighbors and truncate our messages accordingly.
                var welcomeStringWords = message.args[1].split(/\s+/);
                self.hostMask = welcomeStringWords[welcomeStringWords.length - 1];
                self._updateMaxLineLength();
                self.emit('registered', message);
                break;

I don't believe that's right, maxLineLength was only a symptom, the soonest you can send a message is after you have a valid nick. The server may change your nick on the connection registration so self._updateMaxLineLength(); needs to be called, after all of that, the event 'registered' is emitted (IMHO, the overall best way to handle the situation).

@swichers btw 'nickserv' module was abandoned by the owner, the last commit was June 2014, you may want to look for a different 'NickServ' project or fork as they suggested.

[ghost]

I suggest we move this to closed, as the solution was found to use the event "registered".

[swichers]

Do you disagree that it is a bug that sending a message on connection, but before certain events have fired, can cause the bot to crash? I feel like that should, at minimum, just return false instead of pooping the bed.

[ghost]

I actually prefer a noisy crash because it reminds us that something isn't perfect, but a silent crash will become much, much more frustrating to troubleshoot for the users.

I think this is an overdue enhancement for all commands to gracefully fail #421.

[moshmage]

@jnull my bad, I was referring to the following code (below) - I remember seeing (on occasion, and through my znc) the server sending back a bunch of properties - I gather the updateMaxLineLength prop comes in one of those messages (but maybe not these that we are ignoring).

I'll read the RFC, soon as I get some time to "waste", and come back with newfound ideas :)

            case 'rpl_yourhost':
            case 'rpl_created':
            case 'rpl_luserclient':
            case 'rpl_luserop':
            case 'rpl_luserchannels':
            case 'rpl_luserme':
            case 'rpl_localusers':
            case 'rpl_globalusers':
            case 'rpl_statsconn':
                // Random welcome crap, ignoring

[ghost]

Ya, those are just unhandled messages for different server(s) during the irc client "registration"/connection phase. As the comment suggests:

// Random welcome crap, ignoring

They are benign, one of them might be better for listening for something else, but you can't get any closer to the successful connection then code 001 'rpl_welcome'.

Lets move our discussion to #421 for graceful error messages so we can close this issue.

[fent]

Stumbled on this issue due to someone opening an issue on the nickserv module. I think I abandoned it because different NickServ servers with different messages made it inconsistent.

Anyway, I fixed the issue, anyone that was trying it, feel free to try it again.