Conversation
…e stack string `componentDidCatch` called `captureException(info.componentStack)`, which sends a STRING. Every boundary-caught crash therefore arrived in Sentry as an opaque message with no error type, no message and no stack trace — the three things that make a crash diagnosable. This boundary wraps the whole app (`App.tsx`), so that applied to every unhandled render error in the extension. Reports the Error itself now, with the component stack carried as context rather than as the payload. `captureException(exception, hint)` takes a CaptureContext as its second argument in @sentry/browser 10.x. Adds the first test coverage for this component: one asserting the reported value is an Error and not a string — confirmed to fail against the old call — and one asserting the fallback renders instead of the crashed subtree. Found while auditing error paths for the token price chart, where this boundary is the last line of defence for a render-time throw. Split out because it is a pre-existing bug in shared code and should not wait on that feature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Corrects popup crash reporting by sending the original error to Sentry while retaining React’s component stack as context.
Changes:
- Reports the caught
Errorrather than a stack string. - Adds regression coverage for Sentry reporting and fallback rendering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
ErrorBoundary/index.tsx |
Improves Sentry exception payloads. |
ErrorBoundary/__tests__/index.test.tsx |
Tests reporting and fallback behavior. |
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-d37ef93024a4b3251652 |
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.
TL;DR
Every crash caught by the app's error boundary has been arriving in Sentry as an opaque string with no error type, no message and no stack trace — because
componentDidCatchreportedinfo.componentStack(a string) instead of theErroritself. This boundary wraps the whole extension, so it applies to every unhandled render error we have.One line. The component stack is still sent, as context rather than as the payload.
Implementation details (for agents)
What changed —
ErrorBoundary/index.tsx:captureException(exception: unknown, hint?: ExclusiveEventHintOrCaptureContext)— checked against the installed@sentry/browser10.63.0 rather than assumed, so the context object is the supported second argument.Why it matters.
ErrorBoundarywraps the app inApp.tsx, so it is the catch-all for render-time throws. Reporting the stack string means Sentry groups those events by a string it cannot parse, with no exception type to filter on and no stack frames to resolve against sourcemaps. Anything the boundary caught was effectively undiagnosable.Tests. This component had none; the PR adds two:
Error, not a string, with its message preserved and the component stack present in contextThe first was confirmed to fail against the old call — reverted the fix, watched it go red, restored it. It is a regression guard, not a passing assertion.
Verification:
tsc --noEmitclean, both tests pass, no other files touched.Provenance. Found while auditing error paths for the token price chart (
freighter-private#32), where this boundary is the last line of defence for a render-time throw. Split out because it is a pre-existing bug in shared code that benefits every crash report today, and should not wait on that feature.🤖 Generated with Claude Code