fix: message parser breaking workspace#40306
fix: message parser breaking workspace#40306nazabucciarelli wants to merge 10 commits intodevelopfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 43e6955 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40306 +/- ##
===========================================
- Coverage 69.81% 69.80% -0.02%
===========================================
Files 3296 3297 +1
Lines 119173 119269 +96
Branches 21468 21509 +41
===========================================
+ Hits 83198 83251 +53
- Misses 32675 32703 +28
- Partials 3300 3315 +15
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
✅ Layne — scan passed No security issues found on latest push. |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent the client-side message parsing pipeline from causing severe performance issues (up to “breaking” the workspace) when handling very large message payloads, by introducing a maximum parse size and bypassing parsing/rendering paths when the content exceeds that threshold.
Changes:
- Introduces a
useMaxMessageParseSizehook based on theMessage_MaxAllowedSizesetting, capped by a new hard limit constant. - Updates message normalization/body hooks to skip parsing when
msg.lengthexceeds the max parse size (falling back to raw text / a minimal AST). - Wires the new limit through message rendering variants and quote attachment rendering, and adds unit tests for the new behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx | Adds maxMessageParseSize parameter and bypasses parsing for oversized messages. |
| apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts | Adds unit tests validating the new bypass behavior and parser invocation conditions. |
| apps/meteor/client/lib/constants.ts | Introduces MESSAGE_PARSE_HARD_LIMIT constant used to cap parsing size. |
| apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx | Passes max parse size into normalization to avoid parsing oversized messages in thread view. |
| apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx | Passes max parse size into normalization to avoid parsing oversized messages in room view. |
| apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx | Uses max parse size when computing preview body to avoid parsing oversized parent messages. |
| apps/meteor/client/components/message/hooks/useNormalizedMessage.ts | Adds max-size bypass; returns minimal md AST for oversized messages. |
| apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts | Adds tests ensuring parsing is skipped for oversized messages and attachments are preserved. |
| apps/meteor/client/components/message/hooks/useMaxMessageParseSize.ts | New hook computing max parse size from settings with a hard cap. |
| apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx | Avoids rendering parsed md for oversized quote attachment text. |
| apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx | Adds tests for quote attachment md rendering vs plain-text fallback based on size. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes (including videos or screenshots)
Prevents the message parser from blocking the client on large messages.
Benchmarks show the parser has superlinear complexity on nested markdown tokens — at ~315KB it takes ~10s, freezing the workspace (you can see the results here). This PR adds a
MESSAGE_PARSE_HARD_LIMITof 20,000 characters: messages exceeding it are rendered as plain text instead of being parsed.Changes:
useMaxMessageParseSizehook to centralize the limit logicuseNormalizedMessage(room/thread messages) anduseMessageBody(thread previews) to skip parsing when the limit is exceededQuoteAttachmentto apply the same limit for consistencyi18nDescriptiontoMessage_MaxAllowedSizesetting to warn admins that values above the hard limit affect markdown renderingIssue(s)
SUP-1019
Steps to test or reproduce
Further comments