search

litaio / lita-slack

A Slack adapter for Lita.
MIT License
141 stars 136 forks source link

Handling large messages #20

Open benodom opened 9 years ago

benodom commented 9 years ago

This is a continuation of the conversation at: https://github.com/josacar/lita-jobs/issues/3

At the moment, any messages larger than MAX_MESSAGE_BYTES are dropped, with an exception raised: https://github.com/kenjij/lita-slack/blob/master/lib/lita/adapters/slack/rtm_connection.rb#L103

It would be preferred in my humble opinion to auto-convert anything larger than MAX_MESSAGE_BYTES to a Slack text attachment. Chunking a message into 1..n pieces could still fill up a Slack room and be undesirable for users in the channel, whereas the Slack text attachment requires users to click-to-expand.

I'd be happy to submit a PR; thanks for your feedback.

jimmycuadra commented 9 years ago

I agree, this would be better behavior. Go ahead with a PR if you like, just make sure the code for actually submitting the Slack attachment is isolated from the code that uses it when a large message is encountered, as we'll probably want to introduce a mechanism for explicitly sending Slack attachments in the future and we'll want to reuse that code!

josacar commented 8 years ago

This should be closed as now in slack we can send attachments, maybe it can be interesting to use this on the send_message when it exceeds the maximum size.

Attachments still crop large messages and they don't throw an exception either.

jimmycuadra commented 8 years ago

I'm gonna leave it open because the issue is really about that large messages crash the adapter instead of falling back gracefully to an attachment. It's an easier change now that there's support in the adapter for attachments, though.