-
-
Notifications
You must be signed in to change notification settings - Fork 361
feat(core): Capture Expo Router ErrorBoundary errors #6318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| import type { Scope } from '@sentry/core'; | ||
|
|
||
| import { | ||
| addBreadcrumb, | ||
| addExceptionMechanism, | ||
| captureException, | ||
| getActiveSpan, | ||
| getClient, | ||
| getRootSpan, | ||
| SPAN_STATUS_ERROR, | ||
| } from '@sentry/core'; | ||
| import * as React from 'react'; | ||
|
|
||
| import { getCurrentExpoRouterRouteInfo } from './expoRouterStore'; | ||
|
|
||
| /** | ||
| * The minimal shape of Expo Router's per-route `ErrorBoundary` props. | ||
| * | ||
| * We re-declare it here to avoid a hard dependency on `expo-router`. | ||
| */ | ||
| export interface ExpoRouterErrorBoundaryProps { | ||
| error: Error; | ||
| retry: () => Promise<void>; | ||
| } | ||
|
|
||
| /** | ||
| * Wraps Expo Router's per-route `ErrorBoundary` so that the SDK captures | ||
| * errors that hit the boundary instead of relying on the user's global error | ||
| * handler. | ||
| * | ||
| * Expo Router renders the boundary exported from a route file | ||
| * (`export { ErrorBoundary } from 'expo-router'`) when a component throws | ||
| * during render. Without this wrapper, Sentry only sees the error if it also | ||
| * reaches `ErrorUtils` โ which it often does not, because React swallows the | ||
| * error once a boundary handles it. | ||
| * | ||
| * For each new `error` instance the wrapper: | ||
| * - Captures the error to Sentry with `route.name`, `route.path`, and | ||
| * `route.params` attached, gated by `sendDefaultPii` for concrete fields. | ||
| * - Tags the active idle navigation span (and its root) with | ||
| * `SPAN_STATUS_ERROR` so the navigation transaction reflects the failure. | ||
| * - Adds a breadcrumb describing the boundary render. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * // app/_layout.tsx\n * import { ErrorBoundary as ExpoErrorBoundary } from 'expo-router';\n * import * as Sentry from '@sentry/react-native';\n *\n * export const ErrorBoundary = Sentry.wrapRouterErrorBoundary(ExpoErrorBoundary);\n * ```\n */ | ||
| export function wrapRouterErrorBoundary<P extends ExpoRouterErrorBoundaryProps>( | ||
| OriginalErrorBoundary: React.ComponentType<P>, | ||
| ): React.ComponentType<P> { | ||
| const Wrapped: React.FC<P> = props => { | ||
| // Track the last error instance we reported so a re-render with the same | ||
| // error does not produce a duplicate event. Resets when `retry()` clears | ||
| // the error and React unmounts the boundary. | ||
| const reportedErrorRef = React.useRef<unknown>(null); | ||
|
|
||
| if (reportedErrorRef.current !== props.error && props.error) { | ||
| reportedErrorRef.current = props.error; | ||
| reportRouterBoundaryError(props.error); | ||
| } | ||
|
|
||
|
Check warning on line 60 in packages/core/src/js/tracing/expoRouterErrorBoundary.tsx
|
||
| return <OriginalErrorBoundary {...props} />; | ||
|
Check warning on line 61 in packages/core/src/js/tracing/expoRouterErrorBoundary.tsx
|
||
| }; | ||
|
|
||
| Wrapped.displayName = `wrapRouterErrorBoundary(${ | ||
| OriginalErrorBoundary.displayName || OriginalErrorBoundary.name || 'ErrorBoundary' | ||
| })`; | ||
|
|
||
| return Wrapped as React.ComponentType<P>; | ||
| } | ||
|
|
||
| function reportRouterBoundaryError(error: Error): void { | ||
| const sendPii = getClient()?.getOptions()?.sendDefaultPii ?? false; | ||
| const route = getCurrentExpoRouterRouteInfo(); | ||
|
|
||
| const templatedPath = route?.templatedPath; | ||
| // `templatedPath` (e.g. `/users/[id]`) is structural and safe; concrete path | ||
| // (e.g. `/users/42`) and `params` may contain identifiers and are PII-gated. | ||
| const routeName = templatedPath ?? 'unknown'; | ||
| const concretePath = sendPii ? (route?.pathnameWithParams ?? route?.pathname) : undefined; | ||
|
|
||
| addBreadcrumb({ | ||
| category: 'expo-router.error_boundary', | ||
| type: 'error', | ||
| level: 'error', | ||
| message: `Expo Router ErrorBoundary rendered for ${routeName}`, | ||
| data: { | ||
| 'route.name': routeName, | ||
| ...(concretePath ? { 'route.path': concretePath } : undefined), | ||
| }, | ||
| }); | ||
|
|
||
| markActiveNavigationSpanErrored(); | ||
|
|
||
| captureException(error, (scope: Scope) => { | ||
| scope.setTag('expo_router.error_boundary', 'true'); | ||
| scope.setContext('route', { | ||
| name: routeName, | ||
| ...(concretePath ? { path: concretePath } : undefined), | ||
| ...(sendPii && route?.params ? { params: route.params } : undefined), | ||
| ...(route?.segments ? { segments: route.segments } : undefined), | ||
| }); | ||
| scope.addEventProcessor(event => { | ||
| addExceptionMechanism(event, { type: 'expo_router_error_boundary', handled: true }); | ||
| return event; | ||
| }); | ||
| return scope; | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sentry failure blocks error UIHigh Severity
Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit f313648. Configure here. |
||
| } | ||
|
|
||
| /** | ||
| * If an idle navigation span (or any child) is still open when the boundary | ||
| * renders, mark its root as errored so the resulting transaction reflects the | ||
| * navigation failure. Scoped to navigation roots so that a user-started | ||
| * custom span is not retroactively flipped to errored. No-op otherwise. | ||
| */ | ||
| function markActiveNavigationSpanErrored(): void { | ||
| const active = getActiveSpan(); | ||
| if (!active) { | ||
| return; | ||
| } | ||
| const root = getRootSpan(active); | ||
| const origin = (root as { attributes?: Record<string, unknown> })?.attributes?.['sentry.origin']; | ||
| if (typeof origin !== 'string' || !origin.startsWith('auto.navigation.')) { | ||
| return; | ||
| } | ||
| root.setStatus({ code: SPAN_STATUS_ERROR, message: 'expo_router_error_boundary' }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| /** | ||
| * Shared helpers for reading Expo Router's internal router store. | ||
| * | ||
| * Used by: | ||
| * - {@link expoRouterIntegration} to attach the current route to the idle | ||
| * navigation span via {@link RouteOverride}. | ||
| * - {@link wrapRouterErrorBoundary} to attach the current route to errors | ||
| * surfaced through Expo Router's per-route `ErrorBoundary`. | ||
| */ | ||
|
|
||
| export interface ExpoRouterNavigationRef { | ||
| current: unknown | null; | ||
| } | ||
|
|
||
| export interface ExpoRouterUrlObject { | ||
| unstable_globalHref?: string; | ||
| pathname?: string; | ||
| pathnameWithParams?: string; | ||
| params?: Record<string, unknown>; | ||
| segments?: string[]; | ||
| } | ||
|
|
||
| export interface ExpoRouterStore { | ||
| navigationRef?: ExpoRouterNavigationRef; | ||
| getRouteInfo?: () => ExpoRouterUrlObject; | ||
| } | ||
|
|
||
| export interface NormalizedExpoRouterRouteInfo { | ||
| /** | ||
| * Templated pathname with grouping segments (`(tabs)`) removed. Safe to send | ||
| * regardless of `sendDefaultPii`. Examples: | ||
| * ['(tabs)', 'profile', '[id]'] -> '/profile/[id]' | ||
| * ['posts', '[...slug]'] -> '/posts/[...slug]' | ||
| * [] -> '/' | ||
| */ | ||
| templatedPath: string; | ||
| /** Concrete pathname (may contain user identifiers). Caller decides PII handling. */ | ||
| pathname?: string; | ||
| /** Concrete pathname including query/params (may contain PII). */ | ||
| pathnameWithParams?: string; | ||
| params?: Record<string, unknown>; | ||
| segments?: string[]; | ||
| } | ||
|
|
||
| /** | ||
| * Returns Expo Router's internal router store, or `null` if `expo-router` is | ||
| * not installed or the build does not expose the expected module path. | ||
| */ | ||
| export function tryGetExpoRouterStore(): ExpoRouterStore | null { | ||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const mod = require('expo-router/build/global-state/router-store') as { | ||
| store?: ExpoRouterStore; | ||
| }; | ||
| return mod?.store ?? null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Builds a templated pathname from Expo Router's `segments`. Grouping segments | ||
| * (e.g. `(tabs)`, `(auth)`) are stripped because they do not appear in the URL. | ||
| */ | ||
| export function buildExpoRouterTemplatedPath(segments: string[] | undefined): string { | ||
| if (!segments || segments.length === 0) { | ||
| return '/'; | ||
| } | ||
| const filtered = segments.filter(s => !(s.startsWith('(') && s.endsWith(')'))); | ||
| return filtered.length === 0 ? '/' : `/${filtered.join('/')}`; | ||
| } | ||
|
|
||
| /** | ||
| * Reads the current route from Expo Router's store and normalizes it. Returns | ||
| * `undefined` if the store is not reachable or `getRouteInfo` throws. | ||
| */ | ||
| export function getCurrentExpoRouterRouteInfo(): NormalizedExpoRouterRouteInfo | undefined { | ||
| const store = tryGetExpoRouterStore(); | ||
| if (!store) { | ||
| return undefined; | ||
| } | ||
| let info: ExpoRouterUrlObject | undefined; | ||
| try { | ||
| info = store.getRouteInfo?.(); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| if (!info) { | ||
| return undefined; | ||
| } | ||
| return { | ||
| templatedPath: buildExpoRouterTemplatedPath(info.segments), | ||
| pathname: info.pathname, | ||
| pathnameWithParams: info.pathnameWithParams, | ||
| params: info.params, | ||
| segments: info.segments, | ||
| }; | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side effects (captureException, addBreadcrumb, span mutation) executed during React render phase
Calling
reportRouterBoundaryErrorsynchronously 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 auseEffect(() => { โฆ }, [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.render(...)synchronously without Strict Mode wrapping, so this double-invoke / discard scenario is not exercised.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:136Identified by Warden code-review ยท JQW-LSF