From f88ba2d00c96421dd6628487188f5f25e647949a Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Wed, 27 May 2026 14:45:11 +0200 Subject: [PATCH 1/2] Wrap push, replace, navigate, back, dismiss in addition to prefetch --- packages/core/etc/sentry-react-native.api.md | 11 +- packages/core/src/js/tracing/expoRouter.ts | 173 +++++++++++++--- packages/core/src/js/tracing/origin.ts | 1 + .../js/tracing/pendingExpoRouterNavigation.ts | 39 ++++ .../core/src/js/tracing/reactnavigation.ts | 6 + packages/core/test/tracing/expoRouter.test.ts | 191 +++++++++++++++++- 6 files changed, 373 insertions(+), 48 deletions(-) create mode 100644 packages/core/src/js/tracing/pendingExpoRouterNavigation.ts diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index cb69dab67e..3031016c76 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -272,12 +272,13 @@ export interface ExpoRouter { // (undocumented) back?: () => void; // (undocumented) + dismiss?: (count?: number) => void; + // (undocumented) navigate?: (...args: unknown[]) => void; + // Warning: (ae-forgotten-export) The symbol "ExpoRouterHref" needs to be exported by the entry point index.d.ts + // // (undocumented) - prefetch?: (href: string | { - pathname?: string; - params?: Record; - }) => void | Promise; + prefetch?: (href: ExpoRouterHref) => void | Promise; // (undocumented) push?: (...args: unknown[]) => void; // (undocumented) @@ -761,7 +762,7 @@ export function wrapExpoRouter(router: T): T; // src/js/feedback/integration.ts:21:5 - (ae-forgotten-export) The symbol "ScreenshotButtonProps" needs to be exported by the entry point index.d.ts // src/js/feedback/integration.ts:23:5 - (ae-forgotten-export) The symbol "FeedbackFormTheme" needs to be exported by the entry point index.d.ts // src/js/tracing/reactnativetracing.ts:94:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts -// src/js/tracing/reactnavigation.ts:219:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts +// src/js/tracing/reactnavigation.ts:220:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/packages/core/src/js/tracing/expoRouter.ts b/packages/core/src/js/tracing/expoRouter.ts index 8401ecdedb..dfb10f86fd 100644 --- a/packages/core/src/js/tracing/expoRouter.ts +++ b/packages/core/src/js/tracing/expoRouter.ts @@ -1,49 +1,87 @@ -import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core'; +import { addBreadcrumb, SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core'; -import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from './origin'; +import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from './origin'; +import { setPendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; + +type ExpoRouterHref = string | { pathname?: string; params?: Record }; /** - * Type definition for Expo Router's router object + * Type definition for Expo Router's router object. */ export interface ExpoRouter { - prefetch?: (href: string | { pathname?: string; params?: Record }) => void | Promise; - // Other router methods can be added here if needed + prefetch?: (href: ExpoRouterHref) => void | Promise; push?: (...args: unknown[]) => void; replace?: (...args: unknown[]) => void; - back?: () => void; navigate?: (...args: unknown[]) => void; + back?: () => void; + dismiss?: (count?: number) => void; +} + +type NavigationMethod = 'push' | 'replace' | 'navigate' | 'back' | 'dismiss'; + +interface ParsedHref { + href?: unknown; + routeName: string; + pathname?: string; + params?: Record; } /** - * Wraps Expo Router. It currently only does one thing: extends prefetch() method - * to add automated performance monitoring. + * Wraps Expo Router methods to add automated performance monitoring and breadcrumbs. + * + * Currently wraps: + * - `prefetch` — wraps the call in a `navigation.prefetch` span. + * - `push` / `replace` / `navigate` / `back` / `dismiss` — adds a navigation + * breadcrumb, wraps the call in a short-lived span that mirrors prefetch's + * error/status handling, and tags the subsequent idle navigation transaction + * with the initiating `navigation.method` so the resulting span can be + * attributed back to the call site. * - * This function instruments the `prefetch` method of an Expo Router instance - * to create performance spans that measure how long route prefetching takes. + * Safe to call repeatedly — guarded by a single `__sentryWrapped` flag. * * @param router - The Expo Router instance from `useRouter()` hook - * @returns The same router instance with an instrumented prefetch method + * @returns The same router instance with instrumented methods */ export function wrapExpoRouter(router: T): T { - if (!router?.prefetch) { + if (!router) { return router; } - // Check if already wrapped to avoid double-wrapping - if ((router as T & { __sentryPrefetchWrapped?: boolean }).__sentryPrefetchWrapped) { + const wrappedRouter = router as T & { __sentryWrapped?: boolean }; + if (wrappedRouter.__sentryWrapped) { return router; } - const originalPrefetch = router.prefetch.bind(router); + if (router.prefetch) { + wrapPrefetch(router); + } - router.prefetch = ((href: Parameters>[0]) => { - // Extract route name from href for better span naming - let routeName = 'unknown'; - if (typeof href === 'string') { - routeName = href; - } else if (href && typeof href === 'object' && 'pathname' in href && href.pathname) { - routeName = href.pathname; - } + if (router.push) { + router.push = wrapNavigationMethod(router, 'push', router.push.bind(router)); + } + if (router.replace) { + router.replace = wrapNavigationMethod(router, 'replace', router.replace.bind(router)); + } + if (router.navigate) { + router.navigate = wrapNavigationMethod(router, 'navigate', router.navigate.bind(router)); + } + if (router.back) { + router.back = wrapNavigationMethod(router, 'back', router.back.bind(router)) as NonNullable; + } + if (router.dismiss) { + const originalDismiss = router.dismiss.bind(router) as (...args: unknown[]) => unknown; + router.dismiss = wrapNavigationMethod(router, 'dismiss', originalDismiss) as NonNullable; + } + + wrappedRouter.__sentryWrapped = true; + return router; +} + +function wrapPrefetch(router: T): void { + const originalPrefetch = router.prefetch!.bind(router); + + router.prefetch = ((href: ExpoRouterHref) => { + const { routeName } = parseHref(href); const span = startInactiveSpan({ op: 'navigation.prefetch', @@ -58,7 +96,6 @@ export function wrapExpoRouter(router: T): T { try { const result = originalPrefetch(href); - // Handle both promise and synchronous returns if (result && typeof result === 'object' && 'then' in result && typeof result.then === 'function') { return result .then(res => { @@ -71,21 +108,93 @@ export function wrapExpoRouter(router: T): T { span?.end(); throw error; }); - } else { - // Synchronous completion - span?.setStatus({ code: SPAN_STATUS_OK }); - span?.end(); - return result; } + + span?.setStatus({ code: SPAN_STATUS_OK }); + span?.end(); + return result; } catch (error) { span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) }); span?.end(); throw error; } }) as NonNullable; +} - // Mark as wrapped to prevent double-wrapping - (router as T & { __sentryPrefetchWrapped?: boolean }).__sentryPrefetchWrapped = true; +function wrapNavigationMethod( + router: ExpoRouter, + method: NavigationMethod, + original: (...args: unknown[]) => unknown, +): (...args: unknown[]) => unknown { + return (...args: unknown[]) => { + const parsed = parseMethodArgs(method, args); - return router; + addBreadcrumb({ + category: 'navigation', + type: 'navigation', + message: `Expo Router ${method}${parsed.pathname ? ` to ${parsed.pathname}` : ''}`, + data: { + method, + ...(parsed.href !== undefined ? { href: serializeHref(parsed.href) } : undefined), + ...(parsed.pathname ? { pathname: parsed.pathname } : undefined), + ...(parsed.params ? { params: parsed.params } : undefined), + }, + }); + + setPendingExpoRouterNavigation({ + method, + href: parsed.href, + pathname: parsed.pathname, + params: parsed.params, + }); + + const span = startInactiveSpan({ + op: `navigation.${method}`, + name: `Navigation ${method}${parsed.pathname ? ` to ${parsed.pathname}` : ''}`, + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': method, + ...(parsed.href !== undefined ? { 'route.href': serializeHref(parsed.href) } : undefined), + ...(parsed.routeName ? { 'route.name': parsed.routeName } : undefined), + }, + }); + + try { + const result = original.apply(router, args); + span?.setStatus({ code: SPAN_STATUS_OK }); + span?.end(); + return result; + } catch (error) { + span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) }); + span?.end(); + throw error; + } + }; +} + +function parseMethodArgs(method: NavigationMethod, args: unknown[]): ParsedHref { + if (method === 'back' || method === 'dismiss') { + return { routeName: method }; + } + return parseHref(args[0] as ExpoRouterHref | undefined); +} + +function parseHref(href: ExpoRouterHref | undefined): ParsedHref { + if (typeof href === 'string') { + return { href, routeName: href, pathname: href }; + } + if (href && typeof href === 'object') { + const pathname = typeof href.pathname === 'string' ? href.pathname : undefined; + return { + href, + routeName: pathname ?? 'unknown', + pathname, + params: href.params, + }; + } + return { routeName: 'unknown' }; +} + +function serializeHref(href: unknown): string { + return typeof href === 'string' ? href : JSON.stringify(href); } diff --git a/packages/core/src/js/tracing/origin.ts b/packages/core/src/js/tracing/origin.ts index 3b2fd4ca32..7e2863485a 100644 --- a/packages/core/src/js/tracing/origin.ts +++ b/packages/core/src/js/tracing/origin.ts @@ -12,5 +12,6 @@ export const SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY = 'auto.ui.time_to_display'; export const SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY = 'manual.ui.time_to_display'; export const SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH = 'auto.expo_router.prefetch'; +export const SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION = 'auto.expo_router.navigation'; export const SPAN_ORIGIN_AUTO_RESOURCE_EXPO_IMAGE = 'auto.resource.expo_image'; export const SPAN_ORIGIN_AUTO_RESOURCE_EXPO_ASSET = 'auto.resource.expo_asset'; diff --git a/packages/core/src/js/tracing/pendingExpoRouterNavigation.ts b/packages/core/src/js/tracing/pendingExpoRouterNavigation.ts new file mode 100644 index 0000000000..31f3368988 --- /dev/null +++ b/packages/core/src/js/tracing/pendingExpoRouterNavigation.ts @@ -0,0 +1,39 @@ +/** + * Cross-module hand-off between {@link wrapExpoRouter} and the + * {@link reactNavigationIntegration} idle navigation span. + * + * When an Expo Router method (push / replace / navigate / back / dismiss) is + * called, it stores the initiating method here. The next idle navigation span + * consumes (and clears) this value so the span can be attributed to the call + * site via the `navigation.method` attribute. + */ + +export interface PendingExpoRouterNavigation { + /** The Expo Router method that initiated the navigation. */ + method: 'push' | 'replace' | 'navigate' | 'back' | 'dismiss'; + /** The target href (string or object), if any. */ + href?: unknown; + /** Parsed pathname from the href, if any. */ + pathname?: string; + /** Parsed params from the href, if any. */ + params?: Record; +} + +let pending: PendingExpoRouterNavigation | undefined; + +/** Stores the initiating Expo Router navigation call. Overwrites any previous pending value. */ +export function setPendingExpoRouterNavigation(value: PendingExpoRouterNavigation): void { + pending = value; +} + +/** Returns and clears the pending Expo Router navigation, if any. */ +export function consumePendingExpoRouterNavigation(): PendingExpoRouterNavigation | undefined { + const value = pending; + pending = undefined; + return value; +} + +/** Test helper — clears the pending value without consuming it. */ +export function clearPendingExpoRouterNavigation(): void { + pending = undefined; +} diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 0490285510..e8a698d270 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -27,6 +27,7 @@ import { markRootSpanForDiscard, } from './onSpanEndUtils'; import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin'; +import { consumePendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; import { getReactNativeTracingIntegration } from './reactnativetracing'; import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes'; import { @@ -433,6 +434,11 @@ export const reactNavigationIntegration = ({ latestNavigationSpan = startGenericIdleNavigationSpan(finalSpanOptions, { ...idleSpanOptions, isAppRestart }); latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION); latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType); + + const pendingExpoRouter = consumePendingExpoRouterNavigation(); + if (pendingExpoRouter && latestNavigationSpan) { + latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method); + } if (ignoreEmptyBackNavigationTransactions) { ignoreEmptyBackNavigation(getClient(), latestNavigationSpan); } diff --git a/packages/core/test/tracing/expoRouter.test.ts b/packages/core/test/tracing/expoRouter.test.ts index 5d76155e18..bd7e54d375 100644 --- a/packages/core/test/tracing/expoRouter.test.ts +++ b/packages/core/test/tracing/expoRouter.test.ts @@ -1,15 +1,24 @@ import { SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core'; import { type ExpoRouter, wrapExpoRouter } from '../../src/js/tracing'; -import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from '../../src/js/tracing/origin'; +import { + SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, +} from '../../src/js/tracing/origin'; +import { + clearPendingExpoRouterNavigation, + consumePendingExpoRouterNavigation, +} from '../../src/js/tracing/pendingExpoRouterNavigation'; const mockStartInactiveSpan = jest.fn(); +const mockAddBreadcrumb = jest.fn(); jest.mock('@sentry/core', () => { const actual = jest.requireActual('@sentry/core'); return { ...actual, startInactiveSpan: (...args: unknown[]) => mockStartInactiveSpan(...args), + addBreadcrumb: (...args: unknown[]) => mockAddBreadcrumb(...args), }; }); @@ -28,6 +37,7 @@ describe('wrapExpoRouter', () => { setAttribute: jest.fn(), }; mockStartInactiveSpan.mockReturnValue(mockSpan); + clearPendingExpoRouterNavigation(); }); it('returns the router unchanged if router is null or undefined', () => { @@ -166,20 +176,179 @@ describe('wrapExpoRouter', () => { expect(mockStartInactiveSpan).toHaveBeenCalledTimes(1); }); - it('preserves other router methods', () => { - const mockPush = jest.fn(); - const mockBack = jest.fn(); + it('still wraps prefetch when other methods are absent', () => { const mockPrefetch = jest.fn(); - const router = { - prefetch: mockPrefetch, - push: mockPush, - back: mockBack, - } as unknown as ExpoRouter; + const router = { prefetch: mockPrefetch } as ExpoRouter; const wrapped = wrapExpoRouter(router); + expect(wrapped.prefetch).not.toBe(mockPrefetch); + }); + + describe.each(['push', 'replace', 'navigate'] as const)('wraps %s', method => { + it(`creates a span and breadcrumb with string href for ${method}`, () => { + const original = jest.fn(); + const router = { [method]: original } as unknown as ExpoRouter; + + const wrapped = wrapExpoRouter(router); + wrapped[method]?.('/details/123'); + + expect(original).toHaveBeenCalledWith('/details/123'); + expect(mockStartInactiveSpan).toHaveBeenCalledWith({ + op: `navigation.${method}`, + name: `Navigation ${method} to /details/123`, + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': method, + 'route.href': '/details/123', + 'route.name': '/details/123', + }, + }); + expect(mockAddBreadcrumb).toHaveBeenCalledWith({ + category: 'navigation', + type: 'navigation', + message: `Expo Router ${method} to /details/123`, + data: { + method, + href: '/details/123', + pathname: '/details/123', + }, + }); + expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_OK }); + expect(mockSpan.end).toHaveBeenCalled(); + }); + + it(`creates a span and breadcrumb with object href for ${method}`, () => { + const original = jest.fn(); + const router = { [method]: original } as unknown as ExpoRouter; + const href = { pathname: '/profile', params: { id: '456' } }; + + const wrapped = wrapExpoRouter(router); + wrapped[method]?.(href); + + expect(original).toHaveBeenCalledWith(href); + expect(mockStartInactiveSpan).toHaveBeenCalledWith({ + op: `navigation.${method}`, + name: `Navigation ${method} to /profile`, + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': method, + 'route.href': JSON.stringify(href), + 'route.name': '/profile', + }, + }); + expect(mockAddBreadcrumb).toHaveBeenCalledWith({ + category: 'navigation', + type: 'navigation', + message: `Expo Router ${method} to /profile`, + data: { + method, + href: JSON.stringify(href), + pathname: '/profile', + params: { id: '456' }, + }, + }); + }); + + it(`sets pending navigation so it can be consumed for ${method}`, () => { + const original = jest.fn(); + const router = { [method]: original } as unknown as ExpoRouter; + + wrapExpoRouter(router)[method]?.({ pathname: '/profile', params: { id: '7' } }); + + const pending = consumePendingExpoRouterNavigation(); + expect(pending).toEqual({ + method, + href: { pathname: '/profile', params: { id: '7' } }, + pathname: '/profile', + params: { id: '7' }, + }); + // consumed exactly once + expect(consumePendingExpoRouterNavigation()).toBeUndefined(); + }); - expect(wrapped.push).toBe(mockPush); - expect(wrapped.back).toBe(mockBack); + it(`reports errors via SPAN_STATUS_ERROR for ${method}`, () => { + const error = new Error(`${method} failed`); + const original = jest.fn(() => { + throw error; + }); + const router = { [method]: original } as unknown as ExpoRouter; + + expect(() => wrapExpoRouter(router)[method]?.('/x')).toThrow(`${method} failed`); + expect(mockSpan.setStatus).toHaveBeenCalledWith({ + code: SPAN_STATUS_ERROR, + message: `Error: ${method} failed`, + }); + expect(mockSpan.end).toHaveBeenCalled(); + }); + }); + + it('wraps back() with no args', () => { + const mockBack = jest.fn(); + const router = { back: mockBack } as unknown as ExpoRouter; + + wrapExpoRouter(router).back?.(); + + expect(mockBack).toHaveBeenCalledWith(); + expect(mockStartInactiveSpan).toHaveBeenCalledWith({ + op: 'navigation.back', + name: 'Navigation back', + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': 'back', + 'route.name': 'back', + }, + }); + expect(mockAddBreadcrumb).toHaveBeenCalledWith({ + category: 'navigation', + type: 'navigation', + message: 'Expo Router back', + data: { method: 'back' }, + }); + expect(consumePendingExpoRouterNavigation()).toEqual({ method: 'back' }); + }); + + it('wraps dismiss() and forwards optional count', () => { + const mockDismiss = jest.fn(); + const router = { dismiss: mockDismiss } as unknown as ExpoRouter; + + wrapExpoRouter(router).dismiss?.(2); + + expect(mockDismiss).toHaveBeenCalledWith(2); + expect(mockStartInactiveSpan).toHaveBeenCalledWith({ + op: 'navigation.dismiss', + name: 'Navigation dismiss', + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': 'dismiss', + 'route.name': 'dismiss', + }, + }); + expect(mockAddBreadcrumb).toHaveBeenCalledWith({ + category: 'navigation', + type: 'navigation', + message: 'Expo Router dismiss', + data: { method: 'dismiss' }, + }); + }); + + it('does not double-wrap navigation methods', () => { + const mockPush = jest.fn(); + const router = { push: mockPush } as unknown as ExpoRouter; + + const wrapped1 = wrapExpoRouter(router); + const wrappedPushAfterFirst = wrapped1.push; + const wrapped2 = wrapExpoRouter(wrapped1); + + expect(wrapped2.push).toBe(wrappedPushAfterFirst); + + wrapped2.push?.('/x'); + expect(mockStartInactiveSpan).toHaveBeenCalledTimes(1); + }); + + it('returns the router unchanged if no known methods exist', () => { + const router = { somethingElse: jest.fn() } as unknown as ExpoRouter; + const wrapped = wrapExpoRouter(router); + expect(wrapped).toBe(router); }); it('binds prefetch method correctly to maintain context', () => { From 6d909518878e759b02b9caba4852d23e8d4ef78f Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 28 May 2026 14:21:26 +0200 Subject: [PATCH 2/2] fix(tracing): Guard Expo Router navigation breadcrumbs and spans behind sendDefaultPii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expo Router `push`/`replace`/`navigate`/`back`/`dismiss` (and `prefetch`) wrappers previously emitted the raw `href` and route `params` on both the navigation breadcrumb and span attributes regardless of the client options. Dynamic segments like `/users/123` and parameter values such as `{ id: '123' }` can carry user identifiers and other PII, so the existing `reactnavigation.ts` integration already gates them behind `sendDefaultPii`. The new Expo Router wrappers should follow the same contract. This change: - Reads `sendDefaultPii` from the client options and only attaches `href` (raw string or stringified object form) and `params` to the breadcrumb `data` and span attributes when it is enabled. - Keeps the non-PII fields — `method`, `pathname` (route template), and `route.name` — unconditional so navigations remain visible and groupable in Sentry without leaking user data. - Applies the same guard to the `prefetch` span's `route.href` attribute, which was newly added in this PR. - Adds dedicated `sendDefaultPii` test cases and updates the existing default-off assertions for all wrapped methods. - Adds a changelog entry for #6221. --- CHANGELOG.md | 1 + packages/core/src/js/tracing/expoRouter.ts | 20 ++++++-- packages/core/test/tracing/expoRouter.test.ts | 50 +++++++++++++++---- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3b27c9e49..4d55450d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Add `disableAutoUpload` option to Expo plugin to disable source map and debug symbol uploads ([#6195](https://github.com/getsentry/sentry-react-native/pull/6195)) - Expose `pauseAppHangTracking` and `resumeAppHangTracking` APIs on iOS ([#6192](https://github.com/getsentry/sentry-react-native/pull/6192)) - Better route and dynamic param extraction for Expo Router ([#6197](https://github.com/getsentry/sentry-react-native/pull/6197)) +- Instrument Expo Router `push`, `replace`, `navigate`, `back`, and `dismiss` (in addition to `prefetch`) with breadcrumbs and spans, and tag the resulting idle navigation span with the initiating `navigation.method` ([#6221](https://github.com/getsentry/sentry-react-native/pull/6221)) ### Fixes diff --git a/packages/core/src/js/tracing/expoRouter.ts b/packages/core/src/js/tracing/expoRouter.ts index dfb10f86fd..50e88c4705 100644 --- a/packages/core/src/js/tracing/expoRouter.ts +++ b/packages/core/src/js/tracing/expoRouter.ts @@ -1,4 +1,4 @@ -import { addBreadcrumb, SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core'; +import { addBreadcrumb, getClient, SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core'; import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from './origin'; import { setPendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; @@ -88,8 +88,10 @@ function wrapPrefetch(router: T): void { name: `Prefetch ${routeName}`, attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, - 'route.href': typeof href === 'string' ? href : JSON.stringify(href), 'route.name': routeName, + // `route.href` may contain dynamic segment values (e.g. `/users/123`) + // or stringified `params`, so it is gated behind `sendDefaultPii`. + ...(isSendDefaultPiiEnabled() ? { 'route.href': serializeHref(href) } : undefined), }, }); @@ -128,6 +130,7 @@ function wrapNavigationMethod( ): (...args: unknown[]) => unknown { return (...args: unknown[]) => { const parsed = parseMethodArgs(method, args); + const sendPii = isSendDefaultPiiEnabled(); addBreadcrumb({ category: 'navigation', @@ -135,9 +138,12 @@ function wrapNavigationMethod( message: `Expo Router ${method}${parsed.pathname ? ` to ${parsed.pathname}` : ''}`, data: { method, - ...(parsed.href !== undefined ? { href: serializeHref(parsed.href) } : undefined), ...(parsed.pathname ? { pathname: parsed.pathname } : undefined), - ...(parsed.params ? { params: parsed.params } : undefined), + // `href` (raw URL form) and `params` may contain user identifiers or + // other PII (e.g. `/users/42`, `{ id: '42' }`). Mirror the behavior of + // `reactnavigation.ts` and only include them when `sendDefaultPii` is on. + ...(sendPii && parsed.href !== undefined ? { href: serializeHref(parsed.href) } : undefined), + ...(sendPii && parsed.params ? { params: parsed.params } : undefined), }, }); @@ -154,8 +160,8 @@ function wrapNavigationMethod( attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, 'navigation.method': method, - ...(parsed.href !== undefined ? { 'route.href': serializeHref(parsed.href) } : undefined), ...(parsed.routeName ? { 'route.name': parsed.routeName } : undefined), + ...(sendPii && parsed.href !== undefined ? { 'route.href': serializeHref(parsed.href) } : undefined), }, }); @@ -198,3 +204,7 @@ function parseHref(href: ExpoRouterHref | undefined): ParsedHref { function serializeHref(href: unknown): string { return typeof href === 'string' ? href : JSON.stringify(href); } + +function isSendDefaultPiiEnabled(): boolean { + return getClient()?.getOptions()?.sendDefaultPii ?? false; +} diff --git a/packages/core/test/tracing/expoRouter.test.ts b/packages/core/test/tracing/expoRouter.test.ts index bd7e54d375..b2ce80a18e 100644 --- a/packages/core/test/tracing/expoRouter.test.ts +++ b/packages/core/test/tracing/expoRouter.test.ts @@ -12,6 +12,7 @@ import { const mockStartInactiveSpan = jest.fn(); const mockAddBreadcrumb = jest.fn(); +let mockSendDefaultPii = false; jest.mock('@sentry/core', () => { const actual = jest.requireActual('@sentry/core'); @@ -19,6 +20,7 @@ jest.mock('@sentry/core', () => { ...actual, startInactiveSpan: (...args: unknown[]) => mockStartInactiveSpan(...args), addBreadcrumb: (...args: unknown[]) => mockAddBreadcrumb(...args), + getClient: () => ({ getOptions: () => ({ sendDefaultPii: mockSendDefaultPii }) }), }; }); @@ -37,6 +39,7 @@ describe('wrapExpoRouter', () => { setAttribute: jest.fn(), }; mockStartInactiveSpan.mockReturnValue(mockSpan); + mockSendDefaultPii = false; clearPendingExpoRouterNavigation(); }); @@ -51,7 +54,7 @@ describe('wrapExpoRouter', () => { expect(wrapped).toBe(router); }); - it('wraps prefetch method and creates a span with string href', () => { + it('wraps prefetch method and creates a span with string href (no PII attributes by default)', () => { const mockPrefetch = jest.fn(); const router = { prefetch: mockPrefetch } as ExpoRouter; @@ -63,7 +66,6 @@ describe('wrapExpoRouter', () => { name: 'Prefetch /details/123', attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, - 'route.href': '/details/123', 'route.name': '/details/123', }, }); @@ -73,7 +75,8 @@ describe('wrapExpoRouter', () => { expect(mockSpan.end).toHaveBeenCalled(); }); - it('wraps prefetch method and creates a span with object href', () => { + it('includes `route.href` on prefetch span only when sendDefaultPii is enabled', () => { + mockSendDefaultPii = true; const mockPrefetch = jest.fn(); const router = { prefetch: mockPrefetch } as ExpoRouter; const href = { pathname: '/profile', params: { id: '456' } }; @@ -86,8 +89,8 @@ describe('wrapExpoRouter', () => { name: 'Prefetch /profile', attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, - 'route.href': JSON.stringify(href), 'route.name': '/profile', + 'route.href': JSON.stringify(href), }, }); @@ -109,7 +112,6 @@ describe('wrapExpoRouter', () => { name: 'Prefetch unknown', attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, - 'route.href': JSON.stringify(href), 'route.name': 'unknown', }, }); @@ -185,7 +187,7 @@ describe('wrapExpoRouter', () => { }); describe.each(['push', 'replace', 'navigate'] as const)('wraps %s', method => { - it(`creates a span and breadcrumb with string href for ${method}`, () => { + it(`creates a PII-free span and breadcrumb with string href for ${method}`, () => { const original = jest.fn(); const router = { [method]: original } as unknown as ExpoRouter; @@ -199,7 +201,6 @@ describe('wrapExpoRouter', () => { attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, 'navigation.method': method, - 'route.href': '/details/123', 'route.name': '/details/123', }, }); @@ -209,7 +210,6 @@ describe('wrapExpoRouter', () => { message: `Expo Router ${method} to /details/123`, data: { method, - href: '/details/123', pathname: '/details/123', }, }); @@ -217,7 +217,7 @@ describe('wrapExpoRouter', () => { expect(mockSpan.end).toHaveBeenCalled(); }); - it(`creates a span and breadcrumb with object href for ${method}`, () => { + it(`creates a PII-free span and breadcrumb with object href for ${method}`, () => { const original = jest.fn(); const router = { [method]: original } as unknown as ExpoRouter; const href = { pathname: '/profile', params: { id: '456' } }; @@ -232,7 +232,6 @@ describe('wrapExpoRouter', () => { attributes: { 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, 'navigation.method': method, - 'route.href': JSON.stringify(href), 'route.name': '/profile', }, }); @@ -242,8 +241,37 @@ describe('wrapExpoRouter', () => { message: `Expo Router ${method} to /profile`, data: { method, - href: JSON.stringify(href), pathname: '/profile', + }, + }); + }); + + it(`includes href and params in ${method} span/breadcrumb only when sendDefaultPii is enabled`, () => { + mockSendDefaultPii = true; + const original = jest.fn(); + const router = { [method]: original } as unknown as ExpoRouter; + const href = { pathname: '/profile', params: { id: '456' } }; + + wrapExpoRouter(router)[method]?.(href); + + expect(mockStartInactiveSpan).toHaveBeenCalledWith({ + op: `navigation.${method}`, + name: `Navigation ${method} to /profile`, + attributes: { + 'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, + 'navigation.method': method, + 'route.name': '/profile', + 'route.href': JSON.stringify(href), + }, + }); + expect(mockAddBreadcrumb).toHaveBeenCalledWith({ + category: 'navigation', + type: 'navigation', + message: `Expo Router ${method} to /profile`, + data: { + method, + pathname: '/profile', + href: JSON.stringify(href), params: { id: '456' }, }, });