Split up messages if longer than 4000 chars

[redhotvengeance]

This PR addresses and fixes #104.

Presently, if a message longer than 4000 chars / 16k bytes in size is sent, the RTM API will reject the message and close the connection to Hubot. This is easily reproducible by calling hubot help once a sizable amount of scripts are added to the bot. Hubot will still be alive, but it stays disconnected from Slack and requires a restart before we can start talking to our robotic friend again.

This PR checks to see if a message is longer than 4000 chars before it is sent, and if it is longer than 4000 chars, it will split the message into chunks and send each one at a time. It will also attempt to split the message at the last occurring line break (within the 4000 char limit) to try to result in cleaner message chunks.

[evansolomon]

Cool. Couple small suggestions

• I'd probably set 4000 to a constant like MAX_MESSAGE_LENGTH so that it is more obvious what it's doing
• I think getting rid of the conditional and just always making an array of <4k messages is a little more readable, and it's likely only a tiny performance sacrifice
• I would cache the call to chunk.lastIndexOf mostly because I think it will improve readability (and also is probably a tiny performance gain)

[redhotvengeance]

@evansolomon Sure, I'll make those tweaks. I'll need to slightly rework the logic to hit your second note, because otherwise it'll always split up messages with line breaks, even if they are less than 4000 chars. I'll push an update in a bit.

[evansolomon]

Good point re: sub-4k line breaks. In that case, maybe a better approach is just to handle sub-4k messages and return early. Something like...

return channel.send msg if msg.length <= 4000

while ...

Mostly I just think it's nice to avoid big blocks of indented code when it's reasonable to do it because it makes thing a little harder to read.

Edit: I just realized even that part is insider of another loop (so returning will cause problems). So maybe my whole idea about no conditionals is just not worth it here.

[redhotvengeance]

@evansolomon Alright, I've pushed the update. When it comes to the conditional in question, I swapped the order so that the larger block occurs on the else, making it a bit more readable when it comes to the single message sending line. I figured that was the best compromise after exploring the options (since, as you noted, using a return wasn't doable since it was nested inside another loop).

[evansolomon]

Thinking about this a little more, I think the sub-4k chunk problem still exists.

For example, let's say the message is "Hello [3994 characters...] foo\nbar". I think we'll end up with 3 messages when we only want 2. We should get one message with "Hello [bunch of characters]" and a second message with "foo\nbar". But instead the second message will be split on the last index of a newline, so we'll get "foo" then "bar".

We should check to see if the substring'd chunk is <= 4k characters, and if it is skip the whole line break thing.

[redhotvengeance]

@evansolomon Great catch. Fixed in latest push.

[evansolomon]

Thanks!

[redhotvengeance]

@evansolomon Thanks right back!