feat(core): Capture Expo Router ErrorBoundary errors#6318
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 7 more 🤖 This preview updates automatically when you update the PR. |
Add `Sentry.wrapRouterErrorBoundary` to wrap Expo Router's per-route `ErrorBoundary` so render-phase errors that hit the fallback are captured with route context, the active navigation transaction is marked errored, and a breadcrumb is emitted. Closes #6160
alwx
force-pushed
the
feat/expo-router-error-boundary
branch
from
e252cc5 to
f313648
Compare
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Capture Expo Router ErrorBoundary errors ([#6318](https://github.com/getsentry/sentry-react-native/pull/6318))If none of the above apply, you can opt out of this check by adding |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sentry capture fires during render phase, risking duplicate events or missed deduplication on remount
Calling reportRouterBoundaryError in the render body rather than in a useEffect means the capture can fire during an abandoned Concurrent Mode render (error reported but boundary never committed) and will re-fire after any genuine unmount+remount because useRef resets on remount — the ref-based dedup only prevents duplicates within the same component lifetime. Consider moving the logic to useEffect(() => { if (props.error && reportedErrorRef.current !== props.error) { reportedErrorRef.current = props.error; reportRouterBoundaryError(props.error); } }, [props.error]).
Evidence
expoRouterErrorBoundary.tsxlines 56-58: the guard andcaptureExceptioncall are in the function body, not inside anyuseEffect.useRefis initialized tonullon every fresh mount; when Expo Router or a parent unmounts and remounts the boundary (e.g., navigation stack pop+push with the same error prop), a new ref starts atnull, bypassing the dedup check and emitting a second event for the same error.- In React 18 Concurrent Mode, renders that are interrupted and abandoned will still have executed lines 57-58, reporting to Sentry before the fallback UI ever commits.
- The test suite (
expoRouterErrorBoundary.test.tsx) only covers same-instance rerenders and does not exercise the unmount→remount path, so this scenario is untested.
Identified by Warden code-review
| reportedErrorRef.current = props.error; | ||
| reportRouterBoundaryError(props.error); | ||
| } | ||
|
|
There was a problem hiding this comment.
Side effects (captureException, addBreadcrumb, span mutation) executed during React render phase
Calling reportRouterBoundaryError synchronously inside the render body violates React's rule that renders must be pure; in React 18 concurrent mode, renders can be interrupted and the ref mutation on line 58 will suppress the deduplication guard without the error ever being reported after commit. Move to a useEffect(() => { … }, [props.error]) to tie the side effect to the commit phase.
Evidence
reportRouterBoundaryErrorcallsaddBreadcrumb,captureException, androot.setStatus— all observable external side effects.reportedErrorRef.current = props.error(line 58) mutates a ref during render, which React explicitly warns against in concurrent mode since the same render may run multiple times before committing.- If React discards the in-progress render (e.g. due to a higher-priority update), the ref is already set to the error value so the guard on line 57 will prevent a second attempt on the retry render, silently dropping the capture.
- The tests invoke
render(...)synchronously without Strict Mode wrapping, so this double-invoke / discard scenario is not exercised. - The correct pattern is
useEffect(() => { if (props.error && reportedErrorRef.current !== props.error) { reportedErrorRef.current = props.error; reportRouterBoundaryError(props.error); } }, [props.error]);
Also found at 1 additional location
packages/core/src/js/index.ts:136
Identified by Warden code-review · JQW-LSF
|
it's still a draft, don't review yet |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f313648. Configure here.
| return event; | ||
| }); | ||
| return scope; | ||
| }); |
There was a problem hiding this comment.
Sentry failure blocks error UI
High Severity
reportRouterBoundaryError runs synchronously during the wrapped boundary’s render, before OriginalErrorBoundary is returned. Nothing wraps addBreadcrumb, captureException, or span status updates, so any thrown Sentry instrumentation error aborts render and can prevent Expo Router’s fallback UI from showing.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit f313648. Configure here.
|
Really needed this, looking forward for it been merged |
2 participants
📢 Type of change
📜 Description
Adds
Sentry.wrapRouterErrorBoundary— wraps Expo Router's per-routeErrorBoundaryso render-phase errors that hit the fallback are captured with route context (route.name,route.path,route.params), the active navigation transaction is marked errored, and a breadcrumb is emitted. Concrete path/params are gated behindsendDefaultPii.💡 Motivation and Context
Closes #6160.
💚 How did you test it?
New unit tests in
expoRouterErrorBoundary.test.tsx. Full suite green locally.📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Investigate Metro/Babel auto-instrumentation of
export { ErrorBoundary } from 'expo-router're-exports as a follow-up.