fix(tweet-preview): guard JSON.parse to avoid crashing the modal#996
Open
vimzh wants to merge 3 commits into
Open
fix(tweet-preview): guard JSON.parse to avoid crashing the modal#996vimzh wants to merge 3 commits into
vimzh wants to merge 3 commits into
Conversation
…weet data TweetPreview was passing the result of `JSON.parse(data)` straight into `enrichTweet` whenever `data` was a string. If a document's stored tweet metadata was ever a non-JSON string, truncated JSON, or `"null"` / `"123"`, the unguarded parse threw and crashed the React tree rendering the document modal or memory card. Extract the parse into a `parseTweetData` helper that returns `null` when the input is missing, fails to parse, or doesn't resolve to an object. TweetPreview now renders nothing in those cases instead of crashing. Also widen the prop type from `Tweet` to `Tweet | string` to reflect the runtime input both call sites already pass.
The previous guard accepted any non-null `typeof === "object"`, which means
`JSON.parse("[]")` or `JSON.parse("[1,2,3]")` slipped through and reached
`enrichTweet` — exactly the class of crash the helper was added to prevent.
Add an `Array.isArray` check so only plain objects are passed downstream.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR makes TweetPreview resilient to tweet data being passed as a JSON string by adding safe parsing and avoiding render-time exceptions on invalid input.
Changes:
- Added
parseTweetDatahelper to safely parse tweet JSON strings withtry/catch. - Updated
TweetPreviewprop type to acceptTweet | string. - Short-circuited rendering (
return null) when parsing fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d data Copilot review flagged three things: 1. Casting an arbitrary parsed object to Tweet without validating shape can still crash inside enrichTweet. Added an isTweetLike type guard that requires the 'user' field, which both CustomTweetHeader and the downstream enrichTweet expect. 2. Returning null on bad data silently hid the failure. Now render a small 'Tweet preview unavailable' fallback (matching the style of the existing fallback in document-modal/content/tweet.tsx) and emit a console.warn so failures show up during debugging. 3. JSON.parse was running on every render. Wrapped in useMemo keyed by the data prop so reparsing only happens when the input changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TweetPreview runs
JSON.parse(data)on the data prop with no try/catch. If a document's stored Twitter metadata is ever malformed (truncated JSON, corrupted serialization, exotic stringified values like"null"or"123"), the parse throws and the React tree rendering the document modal or memory card crashes.This wraps the parse in a small
parseTweetDatahelper that returns null when:typeof === "object"check and crash deeper in enrichTweet)When parsing returns null the component renders nothing instead of crashing. Also widened the prop type from
TweettoTweet | stringto match what both call sites already pass viaas Tweet/as unknown as Tweet.One thing worth flagging for reviewers: in
apps/web/components/document-modal/content/tweet.tsxthe TweetPreview branch is entered whenever tweetMetadata is truthy, so malformed metadata now renders an empty container rather than the "Tweet preview unavailable" fallback further down that file. That is strictly better than crashing but not the friendliest UX. Happy to move the validation up into TweetContent in a follow-up if you would prefer the explicit fallback.