Code monitors: truncate long Slack blocks#50083
Conversation
|
Here's an example of truncating lines based on the character limit: |
a6d67dc to
70d4d9d
Compare
| // We limit the bytes to ensure we don't hit Slack's max block size of 3000 | ||
| // characters. To be conservative, we truncate to 2500 bytes. We also limit the |
There was a problem hiding this comment.
q: Is it possible that multi-byte character (eg. emojis or CJK character) will be split up by using this truncation method? If yes, is there a way we can prevent this?
There was a problem hiding this comment.
We don't actually truncate the text by byte. We always truncate entire lines -- if a line will push us over the 2500 limit, then we truncate it and all subsequent lines. This seemed simplest and also produces a clean output. I now see this comment is not clear, I can update it :)
For context, we could use a rune/ character limit instead, but bytes seems safest so we don't count on the Slack API's meaning of "character".
There was a problem hiding this comment.
Got it, makes sense! So if the first line was already 2500+ characters, no preview will be sent?
There was a problem hiding this comment.
We will send a result preview that looks like this:
This is very unlikely to happen in practice though:
- Diff matches begin with the file name, which are not likely to exceed 2500 bytes
- Commit message matches begin with the (short) title of the commit
There was a problem hiding this comment.
Awesome, thanks for looking into this, it all sounds very reasonable.
There was a problem hiding this comment.
No problem, I had all the same questions when I was developing the fix!
Slack requires that text blocks be no longer than 3000 characters. We already limit the number of lines in the match blocks, but if the lines are long then the block can still exceed the character limit. With this change, we also truncate lines if they will push the block over the character limit. Fixes #49979.
Slack requires that text blocks be no longer than 3000 characters. We already limit the number of lines in the match blocks, but if the lines are long then the block can still exceed the character limit. With this change, we also truncate lines if they will push the block over the character limit.
Fixes #49979.
Test plan
New unit test for character-based truncation.
Also tested manually -- reproduced the error, then verified this change resolves it.