irc: Fix maximal message length

[mrshu]

• The maximal message length has previously been set to 510 (probably in
  order to accomodate the b'\r\n' that are necessary for each IRC
  message). This turns out not to be enough, as the message starts with
  the obligatory PRIVMSG <to> : and ends with the EXT (b'\x03) mark.

• The size limit was therefore set to 499 in order to accomodate all
  these obligatory parts.

• Furthermore, a new configuration option has been added
  (MESSAGE_SIZE_LIMIT_INCLUDES_ADDRESEE), which takes care of the fact
  that <to> -- the addressee of the message -- comes directly after
  PRIVMSG and is thus part of the message. Note that this is currently
  only applied in the case of IRC.

• Fixes IRC: Error sending messages bigger than 512 bytes. #989

Signed-off-by: mr.Shu [email protected]

[mrshu]

@gbin @zoni just a quick comment on the failed Travis test -- it seems to be related to flows and not really to the changes introduced in this PR.

With regards to this PR, I am happy to add some tests, although I did not find one for the IRC backend in the core repo.

[zoni]

This is nice!

However, this MESSAGE_SIZE_LIMIT_INCLUDES_ADDRESEE is quite IRC specific, which I'd prefer not to have leak through into errbot itself quite so much by introducing a new parameter in bot_config. I've suggested a different approach, let me know your thoughts.

[zoni]

How about taking a slightly different approach of modifying the signature of split_and_send_message to:

def split_and_send_message(self, msg, max_length=None):
    if max_length is None:
        max_length = self.bot_config.MESSAGE_SIZE_LIMIT

    for part in split_string_after(msg.body, size_limit):
        partial_message = msg.clone()
        partial_message.body = part
        self.send_message(partial_message)

You can then do the max_length -= len(str(msg.to)) calculation in the IRC backend directly and pass max_length to split_and_send_message explicitly. That way it's completely self-contained within the IRC backend.

[mrshu]

@zoni Sorry for my late reply here.

Your solution would indeed be much better but sadly, split_and_send_message is not called from the IRC backend. The IRC backend could still set some sort of object variable/flag on IRCBackend (which inherits from ErrBot which in turn contains split_and_send_message) that would cause the split_and_send_message function to execute code similar to what there currently is in the PR, without the global config option.

I may also not fully understand what you meant, so please feel free to point out if I missed anything.

Thanks!

[GenPage]

@mrshu Hey! Following up for @zoni

I think the goal here is trying to keep this specific IRC code in the backend. While split_and_send_message is not called from IRCBackend, I agree that it still could be self-contained in the backend.

I recommend looking at the SlackBackend's implementation for their size limits:

errbot/errbot/backends/slack.py

Lines 651 to 668 in fe64c25

	limit = min(self.bot_config.MESSAGE_SIZE_LIMIT, SLACK_MESSAGE_LIMIT)
	parts = self.prepare_message_body(body, limit)
	timestamps = []
	for part in parts:
	data = {
	'channel': to_channel_id,
	'text': part,
	'unfurl_media': 'true',
	'link_names': '1',
	'as_user': 'true',
	}
	# Keep the thread_ts to answer to the same thread.
	if 'thread_ts' in msg.extras:
	data['thread_ts'] = msg.extras['thread_ts']
	result = self.api_call('chat.postMessage', data=data)

it's completely self contained. Is there a reason to have a toggleable option for additional size limitation? Wouldn't all IRC messages contain ADDRESEE? or is that only for PRIVMSG? It's bene a while since I've interacted with the IRC backend.

[mrshu]

@zoni I am sorry for pinging you again -- would you happen to find a moment to consider my reply above?

Thanks a ton!

[GenPage]

@mrshu Sorry for the delay! I'm following up on your comments!