From ff90614e5081ff4b215e4e9731c6086fb2fdf6c1 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 9 Jun 2026 11:41:59 +0200 Subject: [PATCH 1/4] feat(tracing): Correlate deep links with the navigation they trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bridges `deeplinkIntegration` and `reactNavigationIntegration` so the idle navigation span started after a deep link arrival is tagged with the link that caused it. Previously the two timelines were unconnected — the breadcrumb recorded `Linking.getInitialURL()` / the `'url'` event, but the resulting navigation span had no way to know it had been triggered by a deep link, making it impossible to measure the gap between "URL received" and "navigation dispatched". Approach mirrors the existing `pendingExpoRouterNavigation` hand-off: * New `tracing/pendingDeepLink.ts` stores the raw URL plus a wall-clock receive timestamp. `consumePendingDeepLink(maxAgeMs)` discards values older than `routeChangeTimeoutMs` (default 1000ms) and clears the slot in all cases so a stale link cannot leak onto a later, unrelated nav. * `deeplinkIntegration` now calls `setPendingDeepLink` alongside its existing breadcrumb. The raw URL is stored — sanitization is deferred to attach time so `sendDefaultPii` is read at the right moment. * `reactNavigationIntegration` consumes the pending value in two places: - In `startIdleNavigationSpan`, covering the warm-open path (link arrives, then navigation dispatches). - In `updateLatestNavigationSpanWithCurrentRoute`, covering the cold start path where `getInitialURL()` resolves *after* the initial idle span has already been started in `afterAllSetup`. Attachment is idempotent per span via a `deepLinkAppliedToLatestSpan` flag, reset on discard and after each successful route change. When attached, the span gets: * `navigation.trigger`: `'deeplink'` * `deeplink.url`: sanitized via the existing `sanitizeDeepLinkUrl` (now exported from `integrations/deeplink.ts`), or raw when `sendDefaultPii` is enabled * `deeplink.received_at`: ms elapsed between URL receipt and the moment the span is annotated — captures the dispatch delay Tests cover warm open, cold-start ordering (span starts before pending is set), single-consume semantics, PII gating, stale rejection, and the "no deep link" baseline. Out of scope (and noted in the issue): wiring the native TTID fallback start time to deep-link arrival instead of navigation dispatch — that requires native-bridge plumbing. Fixes #6159 --- CHANGELOG.md | 1 + packages/core/etc/sentry-react-native.api.md | 2 +- packages/core/src/js/integrations/deeplink.ts | 22 +++- .../core/src/js/tracing/pendingDeepLink.ts | 50 ++++++++ .../core/src/js/tracing/reactnavigation.ts | 45 +++++++ .../core/test/tracing/pendingDeepLink.test.ts | 57 +++++++++ .../core/test/tracing/reactnavigation.test.ts | 113 ++++++++++++++++++ .../core/test/tracing/reactnavigationutils.ts | 12 ++ 8 files changed, 297 insertions(+), 5 deletions(-) create mode 100644 packages/core/src/js/tracing/pendingDeepLink.ts create mode 100644 packages/core/test/tracing/pendingDeepLink.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f7b3e9efd0..bea49d98df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Features +- Correlate deep links with the navigation transaction they trigger. The next idle navigation span started within `routeChangeTimeoutMs` of a deep link arrival is tagged with `navigation.trigger: 'deeplink'`, `deeplink.url` (sanitized, respects `sendDefaultPii`), and `deeplink.received_at` (ms gap between URL received and navigation dispatched). Covers both cold start (`Linking.getInitialURL()`) and warm open (`'url'` event) paths ([#6159](https://github.com/getsentry/sentry-react-native/issues/6159)) - Add memory, CPU, and frame measurements to Android profiling ([#6250](https://github.com/getsentry/sentry-react-native/pull/6250)) - Add `enableAutoConsoleLogs` option to opt out of automatic `console.*` capture while keeping `enableLogs: true` for manual `Sentry.logger.*` calls ([#6235](https://github.com/getsentry/sentry-react-native/pull/6235)) - 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)) diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index 7f63a2d1e3..d1fcf52958 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -857,7 +857,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:90:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" 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 +// src/js/tracing/reactnavigation.ts:222: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/integrations/deeplink.ts b/packages/core/src/js/integrations/deeplink.ts index e01f045fb1..2bafccec8a 100644 --- a/packages/core/src/js/integrations/deeplink.ts +++ b/packages/core/src/js/integrations/deeplink.ts @@ -2,6 +2,7 @@ import type { IntegrationFn } from '@sentry/core'; import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core'; +import { setPendingDeepLink } from '../tracing/pendingDeepLink'; import { sanitizeUrl } from '../tracing/utils'; export const INTEGRATION_NAME = 'DeepLink'; @@ -21,7 +22,14 @@ interface RNLinking { * * Only replaces segments that look like identifiers (all digits, UUIDs, or hex strings). */ -function sanitizeDeepLinkUrl(url: string): string { +/** + * Replaces dynamic path segments (UUID-like or numeric values) with a placeholder + * to avoid capturing PII in path segments when `sendDefaultPii` is off. + * + * Exported so the navigation integration can apply the same sanitization when + * attaching a deep link URL to a navigation span. + */ +export function sanitizeDeepLinkUrl(url: string): string { const stripped = sanitizeUrl(url); // Split off the scheme+authority (e.g. "myapp://host") so the regex @@ -55,7 +63,13 @@ function getBreadcrumbUrl(url: string): string { return sendDefaultPii ? url : sanitizeDeepLinkUrl(url); } -function addDeepLinkBreadcrumb(url: string): void { +function recordDeepLink(url: string): void { + // Hand off to the navigation integration so the next idle navigation span + // can attribute itself to this deep link. Always stores the raw URL — + // sanitization (if any) happens at attach time, based on the client's + // `sendDefaultPii` option at that moment. + setPendingDeepLink(url); + const breadcrumbUrl = getBreadcrumbUrl(url); addBreadcrumb({ category: 'deeplink', @@ -87,7 +101,7 @@ const _deeplinkIntegration: IntegrationFn = () => { .getInitialURL() .then((url: string | null) => { if (url) { - addDeepLinkBreadcrumb(url); + recordDeepLink(url); } }) .catch(() => { @@ -97,7 +111,7 @@ const _deeplinkIntegration: IntegrationFn = () => { // Warm open: deep link received while app is running subscription = linking.addEventListener('url', (event: { url: string }) => { if (event?.url) { - addDeepLinkBreadcrumb(event.url); + recordDeepLink(event.url); } }); diff --git a/packages/core/src/js/tracing/pendingDeepLink.ts b/packages/core/src/js/tracing/pendingDeepLink.ts new file mode 100644 index 0000000000..6d0b1e95f7 --- /dev/null +++ b/packages/core/src/js/tracing/pendingDeepLink.ts @@ -0,0 +1,50 @@ +/** + * Cross-module hand-off between the {@link deeplinkIntegration} and the + * {@link reactNavigationIntegration} idle navigation span. + * + * When a deep link is received (either via `Linking.getInitialURL()` on cold + * start or via the `'url'` event on warm open), the integration stores the + * raw URL together with a receive timestamp here. The navigation integration + * then attaches it to the next idle navigation span started within + * `routeChangeTimeoutMs` (default 1000ms), so traces can correlate + * "deep link → navigation" timing. + */ + +export interface PendingDeepLink { + /** Raw URL as received from React Native's `Linking` API. */ + url: string; + /** Wall-clock timestamp (ms since epoch) when the URL was received. */ + receivedAtMs: number; +} + +let pending: PendingDeepLink | undefined; + +/** + * Stores the most recently received deep link URL together with the current + * timestamp. Overwrites any previous pending value — only the latest link + * matters for correlation with the next navigation. + */ +export function setPendingDeepLink(url: string): void { + pending = { url, receivedAtMs: Date.now() }; +} + +/** + * Returns and clears the pending deep link, but only if it was received + * within `maxAgeMs` of "now". Stale entries are discarded. + */ +export function consumePendingDeepLink(maxAgeMs: number): PendingDeepLink | undefined { + const value = pending; + pending = undefined; + if (!value) { + return undefined; + } + if (Date.now() - value.receivedAtMs > maxAgeMs) { + return undefined; + } + return value; +} + +/** Test helper — clears the pending value without consuming it. */ +export function clearPendingDeepLink(): void { + pending = undefined; +} diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 9aefde5ebf..1ae32c7d9a 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -18,6 +18,7 @@ import type { UnsafeAction } from '../vendor/react-navigation/types'; import type { ReactNativeTracingIntegration } from './reactnativetracing'; import { getAppRegistryIntegration } from '../integrations/appRegistry'; +import { sanitizeDeepLinkUrl } from '../integrations/deeplink'; import { isSentrySpan } from '../utils/span'; import { RN_GLOBAL_OBJ } from '../utils/worldwide'; import { NATIVE } from '../wrapper'; @@ -27,6 +28,7 @@ import { markRootSpanForDiscard, } from './onSpanEndUtils'; import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin'; +import { consumePendingDeepLink } from './pendingDeepLink'; import { consumePendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; import { getReactNativeTracingIntegration } from './reactnativetracing'; import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes'; @@ -230,6 +232,7 @@ export const reactNavigationIntegration = ({ let latestNavigationSpan: Span | undefined; let latestNavigationSpanNameCustomized: boolean = false; let navigationProcessingSpan: Span | undefined; + let deepLinkAppliedToLatestSpan: boolean = false; let initialStateHandled: boolean = false; let isSetupComplete: boolean = false; @@ -463,6 +466,15 @@ export const reactNavigationIntegration = ({ if (pendingExpoRouter && latestNavigationSpan) { latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method); } + + // Try to attribute this span to a recently received deep link. Covers the + // warm-open case (link arrives → navigation dispatches). The cold-start + // case is handled again in `updateLatestNavigationSpanWithCurrentRoute` + // because `Linking.getInitialURL()` may resolve after this point. + deepLinkAppliedToLatestSpan = false; + if (latestNavigationSpan) { + deepLinkAppliedToLatestSpan = applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); + } if (ignoreEmptyBackNavigationTransactions) { ignoreEmptyBackNavigation(getClient(), latestNavigationSpan); } @@ -555,6 +567,13 @@ export const reactNavigationIntegration = ({ routeName = getPathFromState(navigationState) || route.name; } + // Cold-start fallback: if the deep link arrived *after* the idle navigation + // span was started (e.g. `getInitialURL()` resolved post-`afterAllSetup`), + // try again now that the route has mounted. + if (!deepLinkAppliedToLatestSpan) { + deepLinkAppliedToLatestSpan = applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); + } + navigationProcessingSpan?.updateName(`Navigation dispatch to screen ${routeName} mounted`); navigationProcessingSpan?.setStatus({ code: SPAN_STATUS_OK }); navigationProcessingSpan?.end(stateChangedTimestamp); @@ -600,6 +619,7 @@ export const reactNavigationIntegration = ({ } // Clear the latest transaction as it has been handled. latestNavigationSpan = undefined; + deepLinkAppliedToLatestSpan = false; }; /** Pushes a recent route key, and removes earlier routes when there is greater than the max length */ @@ -624,6 +644,7 @@ export const reactNavigationIntegration = ({ if (navigationProcessingSpan) { navigationProcessingSpan = undefined; } + deepLinkAppliedToLatestSpan = false; }; const clearStateChangeTimeout = (): void => { @@ -673,6 +694,30 @@ interface NavigationContainer { getState: () => NavigationState | undefined; } +/** + * Attempts to consume the pending deep link and attach it to the given span. + * + * Returns `true` when a deep link was consumed (and the span was annotated), + * `false` otherwise. Callers may invoke this multiple times against the same + * span — once the pending value has been consumed it will not be re-applied. + */ +function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): boolean { + const pending = consumePendingDeepLink(maxAgeMs); + if (!pending) { + return false; + } + + const sendDefaultPii = getClient()?.getOptions()?.sendDefaultPii ?? false; + const url = sendDefaultPii ? pending.url : sanitizeDeepLinkUrl(pending.url); + + span.setAttributes({ + 'navigation.trigger': 'deeplink', + 'deeplink.url': url, + 'deeplink.received_at': Math.max(0, Date.now() - pending.receivedAtMs), + }); + return true; +} + /** * Extracts the route name from a React Navigation dispatch action payload. * diff --git a/packages/core/test/tracing/pendingDeepLink.test.ts b/packages/core/test/tracing/pendingDeepLink.test.ts new file mode 100644 index 0000000000..bc6e0f7bff --- /dev/null +++ b/packages/core/test/tracing/pendingDeepLink.test.ts @@ -0,0 +1,57 @@ +import { clearPendingDeepLink, consumePendingDeepLink, setPendingDeepLink } from '../../src/js/tracing/pendingDeepLink'; + +describe('pendingDeepLink', () => { + afterEach(() => { + clearPendingDeepLink(); + }); + + it('returns undefined when no deep link has been set', () => { + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + }); + + it('returns the most recently stored URL with a receive timestamp', () => { + const before = Date.now(); + setPendingDeepLink('myapp://profile/123'); + const after = Date.now(); + + const pending = consumePendingDeepLink(1_000); + expect(pending?.url).toBe('myapp://profile/123'); + expect(pending?.receivedAtMs).toBeGreaterThanOrEqual(before); + expect(pending?.receivedAtMs).toBeLessThanOrEqual(after); + }); + + it('clears the value after a single consume', () => { + setPendingDeepLink('myapp://a'); + expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://a'); + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + }); + + it('overwrites a previous pending value', () => { + setPendingDeepLink('myapp://old'); + setPendingDeepLink('myapp://new'); + expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://new'); + }); + + it('drops values older than maxAgeMs and still clears the slot', () => { + const originalNow = Date.now; + const baseNow = originalNow(); + Date.now = (): number => baseNow; + setPendingDeepLink('myapp://stale'); + Date.now = (): number => baseNow + 5_000; + + try { + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + // Slot must be empty even though the value was rejected. + Date.now = originalNow; + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + } finally { + Date.now = originalNow; + } + }); + + it('clearPendingDeepLink removes the value without returning it', () => { + setPendingDeepLink('myapp://x'); + clearPendingDeepLink(); + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + }); +}); diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index 8e663e0a1c..5290bd5840 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -14,6 +14,7 @@ import type { NavigationRoute } from '../../src/js/tracing/reactnavigation'; import { nativeFramesIntegration, reactNativeTracingIntegration } from '../../src/js'; import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from '../../src/js/tracing/origin'; +import { clearPendingDeepLink, setPendingDeepLink } from '../../src/js/tracing/pendingDeepLink'; import { clearPendingExpoRouterNavigation, setPendingExpoRouterNavigation, @@ -312,6 +313,118 @@ describe('ReactNavigationInstrumentation', () => { expect(client.event?.contexts?.trace?.data?.['navigation.method']).toBeUndefined(); }); + test('deep-link hand-off | attaches trigger + sanitized URL on warm open', async () => { + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction + + setPendingDeepLink('myapp://profile/123?token=secret'); + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + expect(data['navigation.trigger']).toBe('deeplink'); + // sendDefaultPii defaults to false → query stripped, numeric id replaced. + expect(data['deeplink.url']).toBe('myapp://profile/'); + expect(typeof data['deeplink.received_at']).toBe('number'); + expect(data['deeplink.received_at']).toBeGreaterThanOrEqual(0); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | keeps raw URL when sendDefaultPii is enabled', async () => { + setupTestClient({ sendDefaultPii: true }); + jest.runOnlyPendingTimers(); + + setPendingDeepLink('myapp://profile/123?token=secret'); + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.contexts?.trace?.data?.['deeplink.url']).toBe('myapp://profile/123?token=secret'); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | does not attach when no deep link is pending', async () => { + setupTestClient(); + jest.runOnlyPendingTimers(); + + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + expect(data['navigation.trigger']).toBeUndefined(); + expect(data['deeplink.url']).toBeUndefined(); + }); + + test('deep-link hand-off | drops a deep link older than routeChangeTimeoutMs', async () => { + setupTestClient(); + jest.runOnlyPendingTimers(); + + // Forge an expired pending value (routeChangeTimeoutMs defaults to 1000ms). + const originalNow = Date.now; + const baseNow = originalNow(); + Date.now = (): number => baseNow; + setPendingDeepLink('myapp://stale'); + Date.now = (): number => baseNow + 5_000; + + try { + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + expect(client.event?.contexts?.trace?.data?.['navigation.trigger']).toBeUndefined(); + } finally { + Date.now = originalNow; + clearPendingDeepLink(); + } + }); + + test('deep-link hand-off | cold start: link arriving after span starts but before state change still attaches', async () => { + // Simulates cold start: the React Navigation idle span is started in + // `afterAllSetup`, then `Linking.getInitialURL()` resolves a microtask + // later, then the route state finally changes. We model this by + // emitting the dispatch (which creates the span), THEN setting the + // pending deep link, THEN letting the state change fire. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction + + mockNavigation.dispatchToNewScreen(); + setPendingDeepLink('myapp://profile/123'); + mockNavigation.completeStateChangeToNewScreen(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + expect(data['navigation.trigger']).toBe('deeplink'); + expect(data['deeplink.url']).toBe('myapp://profile/'); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | consumes the pending value exactly once', async () => { + setupTestClient(); + jest.runOnlyPendingTimers(); + + setPendingDeepLink('myapp://profile/123'); + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + expect(client.event?.contexts?.trace?.data?.['navigation.trigger']).toBe('deeplink'); + + // The next navigation must not inherit the deep-link attribution. + mockNavigation.navigateToSecondScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + expect(client.event?.contexts?.trace?.data?.['navigation.trigger']).toBeUndefined(); + expect(client.event?.contexts?.trace?.data?.['deeplink.url']).toBeUndefined(); + }); + test('drains the pending value even when the listener short-circuits (no leak onto the next nav)', async () => { // Reproduces the bug flagged by sentry-bot/cursor-bot: with // `useDispatchedActionData: true`, a dispatched action without a route diff --git a/packages/core/test/tracing/reactnavigationutils.ts b/packages/core/test/tracing/reactnavigationutils.ts index 93ee6e3dab..bf34b79984 100644 --- a/packages/core/test/tracing/reactnavigationutils.ts +++ b/packages/core/test/tracing/reactnavigationutils.ts @@ -82,6 +82,18 @@ export function createMockNavigationAndAttachTo(sut: ReturnType { mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); }, + /** Dispatches a NAVIGATE action targeting "New Screen" without changing route state. */ + dispatchToNewScreen: () => { + mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); + }, + /** Completes the route transition to "New Screen" by firing the state listener. */ + completeStateChangeToNewScreen: () => { + mockedNavigationContained.currentRoute = { + key: 'new_screen', + name: 'New Screen', + }; + mockedNavigationContained.listeners['state']({}); + }, emitWithoutStateChange: (action: UnsafeAction) => { mockedNavigationContained.listeners['__unsafe_action__'](action); }, From a2c3a6aaf7ebeca0bd0417206e4f6f096cc4a751 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 9 Jun 2026 12:16:08 +0200 Subject: [PATCH 2/4] fix(tracing): Address PR review on deep-link / navigation correlation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three findings from Cursor Bugbot and one from Sentry's review bot on #6264: * **Late deep link never tags span** (Cursor, medium). When Expo Router auto-handles `Linking.getInitialURL()` and finishes the initial navigation *before* our integration's own `getInitialURL().then(...)` chain resolves, the URL was never attributed to the resulting span. Fix: `pendingDeepLink` now supports a synchronous listener that the navigation integration registers in `afterAllSetup`. When a link arrives, the listener tags the in-flight (`latestNavigationSpan`) or most-recent still-recording (`lastIdleNavSpan`) span directly. If nothing live exists, the link falls through to the slot for the next dispatched navigation. A WeakSet guards against double-tagging. * **Same-route path skips deeplink retry** (Cursor, low). The early return in `updateLatestNavigationSpanWithCurrentRoute` for matching route keys (legitimate when deep-linking to the screen you're already on) bypassed the attribution call. Fix: consume + tag in that branch too, before the span reference is dropped. * **Early consume loses pending link** (Cursor, medium). Consuming the pending value inside `startIdleNavigationSpan` meant a later discard (noop / timeout / empty route) silently wasted the link — the real link-driven navigation that followed would not be attributed. Fix: removed the eager consume on dispatch. Pending values are now only consumed in `updateLatestNavigationSpanWithCurrentRoute` (post route mount) or by the listener (live span). Discards never touch the slot. * **`deeplink.received_at` semantically misleading** (Sentry, medium). The attribute stored a duration but its `_at` suffix conventionally denotes a timestamp. Renamed to `deeplink.dispatch_delay_ms`, which accurately reflects the measured gap between URL receipt and span annotation. Plus: changelog entry now references PR #6264 (Danger gate). --- CHANGELOG.md | 2 +- packages/core/etc/sentry-react-native.api.md | 2 +- .../core/src/js/tracing/pendingDeepLink.ts | 54 +++++++-- .../core/src/js/tracing/reactnavigation.ts | 106 ++++++++++++++---- .../core/test/tracing/pendingDeepLink.test.ts | 56 ++++++++- .../core/test/tracing/reactnavigation.test.ts | 82 +++++++++++++- 6 files changed, 263 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bea49d98df..683065b530 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Features -- Correlate deep links with the navigation transaction they trigger. The next idle navigation span started within `routeChangeTimeoutMs` of a deep link arrival is tagged with `navigation.trigger: 'deeplink'`, `deeplink.url` (sanitized, respects `sendDefaultPii`), and `deeplink.received_at` (ms gap between URL received and navigation dispatched). Covers both cold start (`Linking.getInitialURL()`) and warm open (`'url'` event) paths ([#6159](https://github.com/getsentry/sentry-react-native/issues/6159)) +- Correlate deep links with the navigation transaction they trigger. The next idle navigation span started within `routeChangeTimeoutMs` of a deep link arrival is tagged with `navigation.trigger: 'deeplink'`, `deeplink.url` (sanitized, respects `sendDefaultPii`), and `deeplink.dispatch_delay_ms` (ms gap between URL received and navigation dispatched). Covers both cold start (`Linking.getInitialURL()`) and warm open (`'url'` event) paths, including the late-arrival case where Expo Router auto-handles the link before our `getInitialURL()` chain resolves ([#6264](https://github.com/getsentry/sentry-react-native/pull/6264)) - Add memory, CPU, and frame measurements to Android profiling ([#6250](https://github.com/getsentry/sentry-react-native/pull/6250)) - Add `enableAutoConsoleLogs` option to opt out of automatic `console.*` capture while keeping `enableLogs: true` for manual `Sentry.logger.*` calls ([#6235](https://github.com/getsentry/sentry-react-native/pull/6235)) - 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)) diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index d1fcf52958..aea1233b2e 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -857,7 +857,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:90:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts -// src/js/tracing/reactnavigation.ts:222:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts +// src/js/tracing/reactnavigation.ts:224: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/pendingDeepLink.ts b/packages/core/src/js/tracing/pendingDeepLink.ts index 6d0b1e95f7..be29c948bf 100644 --- a/packages/core/src/js/tracing/pendingDeepLink.ts +++ b/packages/core/src/js/tracing/pendingDeepLink.ts @@ -2,12 +2,20 @@ * Cross-module hand-off between the {@link deeplinkIntegration} and the * {@link reactNavigationIntegration} idle navigation span. * - * When a deep link is received (either via `Linking.getInitialURL()` on cold - * start or via the `'url'` event on warm open), the integration stores the - * raw URL together with a receive timestamp here. The navigation integration - * then attaches it to the next idle navigation span started within - * `routeChangeTimeoutMs` (default 1000ms), so traces can correlate - * "deep link → navigation" timing. + * Two delivery modes are supported, both of which need to work in practice: + * + * 1. **Pre-navigation (warm open / normal cold start):** the deep link is + * received before any navigation has been dispatched. The URL is stored in + * a single slot here; the next idle navigation span consumes it inside + * `updateLatestNavigationSpanWithCurrentRoute` (within `routeChangeTimeoutMs`). + * + * 2. **Late arrival (Expo Router auto-handled cold start):** Expo Router reads + * `Linking.getInitialURL()` independently and may finish the initial + * navigation *before* our integration's own `getInitialURL().then(...)` + * chain resolves. To still attribute that span, a synchronous listener may + * be registered (by the navigation integration) and receives every link as + * it arrives. If it tags a still-recording span, it returns `true` and the + * slot is left empty — otherwise the link falls through to the slot. */ export interface PendingDeepLink { @@ -17,20 +25,36 @@ export interface PendingDeepLink { receivedAtMs: number; } +/** + * Synchronously notified for every deep link as it arrives. A `true` return + * value indicates the listener has already attributed the link to a live span, + * and the value should NOT be stored for a future navigation. + */ +export type PendingDeepLinkListener = (link: PendingDeepLink) => boolean; + let pending: PendingDeepLink | undefined; +let listener: PendingDeepLinkListener | undefined; /** * Stores the most recently received deep link URL together with the current - * timestamp. Overwrites any previous pending value — only the latest link + * timestamp. If a listener is registered and consumes the link synchronously, + * the slot is left empty. + * + * Overwrites any previous unconsumed pending value — only the latest link * matters for correlation with the next navigation. */ export function setPendingDeepLink(url: string): void { - pending = { url, receivedAtMs: Date.now() }; + const value: PendingDeepLink = { url, receivedAtMs: Date.now() }; + if (listener?.(value)) { + return; + } + pending = value; } /** * Returns and clears the pending deep link, but only if it was received - * within `maxAgeMs` of "now". Stale entries are discarded. + * within `maxAgeMs` of "now". Stale entries are discarded and the slot is + * cleared in all cases. */ export function consumePendingDeepLink(maxAgeMs: number): PendingDeepLink | undefined { const value = pending; @@ -44,7 +68,17 @@ export function consumePendingDeepLink(maxAgeMs: number): PendingDeepLink | unde return value; } -/** Test helper — clears the pending value without consuming it. */ +/** + * Registers a synchronous listener that is invoked on every {@link setPendingDeepLink} + * call. Pass `undefined` to unregister. Only a single listener is supported — + * a new registration replaces the previous one. + */ +export function setPendingDeepLinkListener(fn: PendingDeepLinkListener | undefined): void { + listener = fn; +} + +/** Test helper — clears the pending value and listener without consuming them. */ export function clearPendingDeepLink(): void { pending = undefined; + listener = undefined; } diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 1ae32c7d9a..eefd98b0d8 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -15,6 +15,7 @@ import { } from '@sentry/core'; import type { UnsafeAction } from '../vendor/react-navigation/types'; +import type { PendingDeepLink } from './pendingDeepLink'; import type { ReactNativeTracingIntegration } from './reactnativetracing'; import { getAppRegistryIntegration } from '../integrations/appRegistry'; @@ -28,7 +29,7 @@ import { markRootSpanForDiscard, } from './onSpanEndUtils'; import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin'; -import { consumePendingDeepLink } from './pendingDeepLink'; +import { consumePendingDeepLink, setPendingDeepLinkListener } from './pendingDeepLink'; import { consumePendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; import { getReactNativeTracingIntegration } from './reactnativetracing'; import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes'; @@ -232,7 +233,28 @@ export const reactNavigationIntegration = ({ let latestNavigationSpan: Span | undefined; let latestNavigationSpanNameCustomized: boolean = false; let navigationProcessingSpan: Span | undefined; - let deepLinkAppliedToLatestSpan: boolean = false; + /** + * Most recently created idle navigation span, retained even after + * `latestNavigationSpan` is cleared on state change. Used by the deep-link + * listener to attribute a link that arrives between "route mounted" and + * "idle span ended". Cleared when superseded by the next nav span. + */ + let lastIdleNavSpan: Span | undefined; + + /** + * Synchronous listener invoked the moment a deep link is recorded. If a live + * idle navigation span exists, tag it directly and tell the pendingDeepLink + * module that the link has been consumed — otherwise let the link fall through + * to the slot so the next dispatched navigation picks it up. + */ + const handleLateDeepLink = (link: PendingDeepLink): boolean => { + const span = latestNavigationSpan ?? lastIdleNavSpan; + if (!span || !isSpanRecording(span)) { + return false; + } + tagSpanWithDeepLink(span, link); + return true; + }; let initialStateHandled: boolean = false; let isSetupComplete: boolean = false; @@ -257,6 +279,14 @@ export const reactNavigationIntegration = ({ }; } + // Listen for deep links as they arrive so we can attribute a span that has + // already mounted its route but not yet ended (e.g. Expo Router auto-handled + // the link before our integration's `getInitialURL()` chain resolved). + setPendingDeepLinkListener(handleLateDeepLink); + client.on('close', () => { + setPendingDeepLinkListener(undefined); + }); + if (initialStateHandled) { // We create an initial state here to ensure a transaction gets created before the first route mounts. // This assumes that the Sentry.init() call is made before the first route mounts. @@ -467,14 +497,12 @@ export const reactNavigationIntegration = ({ latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method); } - // Try to attribute this span to a recently received deep link. Covers the - // warm-open case (link arrives → navigation dispatches). The cold-start - // case is handled again in `updateLatestNavigationSpanWithCurrentRoute` - // because `Linking.getInitialURL()` may resolve after this point. - deepLinkAppliedToLatestSpan = false; - if (latestNavigationSpan) { - deepLinkAppliedToLatestSpan = applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); - } + // Hold a reference to the freshly-created idle span so the deep-link + // listener can still annotate it after `latestNavigationSpan` is cleared + // on state change. We deliberately do NOT consume the pending deep link + // here — if this span is later discarded (noop / timeout / empty route), + // a still-fresh pending value must remain available for the next nav. + lastIdleNavSpan = latestNavigationSpan; if (ignoreEmptyBackNavigationTransactions) { ignoreEmptyBackNavigation(getClient(), latestNavigationSpan); } @@ -531,6 +559,11 @@ export const reactNavigationIntegration = ({ if (previousRoute?.key === route.key) { debug.log(`[${INTEGRATION_NAME}] Navigation state changed, but route is the same as previous.`); + // Even a same-route state change is a legitimate destination for a + // deep link (e.g. deep-linking to the screen you're already on). Make + // sure the pending link still gets attributed before we drop the span + // reference. + applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); pushRecentRouteKey(route.key); latestRoute = route; @@ -567,12 +600,10 @@ export const reactNavigationIntegration = ({ routeName = getPathFromState(navigationState) || route.name; } - // Cold-start fallback: if the deep link arrived *after* the idle navigation - // span was started (e.g. `getInitialURL()` resolved post-`afterAllSetup`), - // try again now that the route has mounted. - if (!deepLinkAppliedToLatestSpan) { - deepLinkAppliedToLatestSpan = applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); - } + // Consume any pending deep link and attach it to this span. Done here + // (after route info is known) so the link is only attributed to a span + // that actually mounted a route — not one that was later discarded. + applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); navigationProcessingSpan?.updateName(`Navigation dispatch to screen ${routeName} mounted`); navigationProcessingSpan?.setStatus({ code: SPAN_STATUS_OK }); @@ -619,7 +650,6 @@ export const reactNavigationIntegration = ({ } // Clear the latest transaction as it has been handled. latestNavigationSpan = undefined; - deepLinkAppliedToLatestSpan = false; }; /** Pushes a recent route key, and removes earlier routes when there is greater than the max length */ @@ -644,7 +674,6 @@ export const reactNavigationIntegration = ({ if (navigationProcessingSpan) { navigationProcessingSpan = undefined; } - deepLinkAppliedToLatestSpan = false; }; const clearStateChangeTimeout = (): void => { @@ -701,20 +730,49 @@ interface NavigationContainer { * `false` otherwise. Callers may invoke this multiple times against the same * span — once the pending value has been consumed it will not be re-applied. */ -function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): boolean { - const pending = consumePendingDeepLink(maxAgeMs); - if (!pending) { - return false; +/** + * Per-span guard against double-tagging deep-link attributes. Shared between + * the synchronous listener path (late arrival) and the post-state-change path. + */ +const taggedDeepLinkSpans = new WeakSet(); + +/** + * Annotates the given span with deep-link attributes if it has not already + * been annotated. Safe to call multiple times — a span is tagged at most once. + */ +function tagSpanWithDeepLink(span: Span, link: PendingDeepLink): void { + if (taggedDeepLinkSpans.has(span)) { + return; } + taggedDeepLinkSpans.add(span); const sendDefaultPii = getClient()?.getOptions()?.sendDefaultPii ?? false; - const url = sendDefaultPii ? pending.url : sanitizeDeepLinkUrl(pending.url); + const url = sendDefaultPii ? link.url : sanitizeDeepLinkUrl(link.url); span.setAttributes({ 'navigation.trigger': 'deeplink', 'deeplink.url': url, - 'deeplink.received_at': Math.max(0, Date.now() - pending.receivedAtMs), + // Duration between URL receipt and the moment the span is annotated — + // approximates the gap between "link received" and "navigation dispatched + // / handled". + 'deeplink.dispatch_delay_ms': Math.max(0, Date.now() - link.receivedAtMs), }); +} + +/** Returns true if the span is still recording (has not been ended). */ +function isSpanRecording(span: Span): boolean { + return spanToJSON(span).timestamp === undefined; +} + +function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): boolean { + if (taggedDeepLinkSpans.has(span)) { + return true; + } + const pending = consumePendingDeepLink(maxAgeMs); + if (!pending) { + return false; + } + tagSpanWithDeepLink(span, pending); return true; } diff --git a/packages/core/test/tracing/pendingDeepLink.test.ts b/packages/core/test/tracing/pendingDeepLink.test.ts index bc6e0f7bff..44f71e3beb 100644 --- a/packages/core/test/tracing/pendingDeepLink.test.ts +++ b/packages/core/test/tracing/pendingDeepLink.test.ts @@ -1,4 +1,9 @@ -import { clearPendingDeepLink, consumePendingDeepLink, setPendingDeepLink } from '../../src/js/tracing/pendingDeepLink'; +import { + clearPendingDeepLink, + consumePendingDeepLink, + setPendingDeepLink, + setPendingDeepLinkListener, +} from '../../src/js/tracing/pendingDeepLink'; describe('pendingDeepLink', () => { afterEach(() => { @@ -54,4 +59,53 @@ describe('pendingDeepLink', () => { clearPendingDeepLink(); expect(consumePendingDeepLink(1_000)).toBeUndefined(); }); + + describe('listener', () => { + it('is invoked synchronously on every set, with url + timestamp', () => { + const received: Array<{ url: string; receivedAtMs: number }> = []; + setPendingDeepLinkListener(link => { + received.push({ url: link.url, receivedAtMs: link.receivedAtMs }); + return false; + }); + + setPendingDeepLink('myapp://a'); + setPendingDeepLink('myapp://b'); + + expect(received.map(r => r.url)).toEqual(['myapp://a', 'myapp://b']); + expect(received[0]?.receivedAtMs).toBeGreaterThan(0); + }); + + it('skips storage when the listener returns true (already consumed)', () => { + setPendingDeepLinkListener(() => true); + setPendingDeepLink('myapp://consumed-by-listener'); + + expect(consumePendingDeepLink(1_000)).toBeUndefined(); + }); + + it('falls through to storage when the listener returns false', () => { + setPendingDeepLinkListener(() => false); + setPendingDeepLink('myapp://stored'); + + expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://stored'); + }); + + it('can be unregistered with undefined', () => { + const fn = jest.fn().mockReturnValue(true); + setPendingDeepLinkListener(fn); + setPendingDeepLinkListener(undefined); + + setPendingDeepLink('myapp://x'); + expect(fn).not.toHaveBeenCalled(); + expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://x'); + }); + + it('clearPendingDeepLink also removes the listener', () => { + const fn = jest.fn().mockReturnValue(true); + setPendingDeepLinkListener(fn); + clearPendingDeepLink(); + + setPendingDeepLink('myapp://x'); + expect(fn).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index 5290bd5840..3df182e58b 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -327,8 +327,8 @@ describe('ReactNavigationInstrumentation', () => { expect(data['navigation.trigger']).toBe('deeplink'); // sendDefaultPii defaults to false → query stripped, numeric id replaced. expect(data['deeplink.url']).toBe('myapp://profile/'); - expect(typeof data['deeplink.received_at']).toBe('number'); - expect(data['deeplink.received_at']).toBeGreaterThanOrEqual(0); + expect(typeof data['deeplink.dispatch_delay_ms']).toBe('number'); + expect(data['deeplink.dispatch_delay_ms']).toBeGreaterThanOrEqual(0); clearPendingDeepLink(); }); @@ -384,6 +384,84 @@ describe('ReactNavigationInstrumentation', () => { } }); + test('deep-link hand-off | late arrival: link received between state change and idle end tags the active span', async () => { + // Reproduces Cursor bugbot's "late deep link never tags span" finding. + // Expo Router may auto-handle `Linking.getInitialURL()` and finish the + // initial navigation before our integration's own `getInitialURL()` + // chain resolves. The synchronous listener must still attribute the + // link to the in-flight (or just-mounted, still-recording) span. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction + + // Dispatch + state change — the span has now "mounted its route" but is + // still recording (idle timer hasn't fired). `latestNavigationSpan` has + // been cleared but `lastIdleNavSpan` still references the live span. + mockNavigation.navigateToNewScreen(); + + // Link arrives LATE, after the state change handler has already run. + setPendingDeepLink('myapp://profile/123'); + + jest.runOnlyPendingTimers(); // Flush the navigation transaction + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + expect(data['navigation.trigger']).toBe('deeplink'); + expect(data['deeplink.url']).toBe('myapp://profile/'); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | discarded dispatch does not consume the pending link', async () => { + // Reproduces Cursor bugbot's "early consume loses pending link" finding. + // A dispatch may turn out to be a noop (no state change → timeout → + // discard). The pending link must survive that discard so the *next* + // real navigation — the one actually caused by the link — still gets + // attributed. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction + + setPendingDeepLink('myapp://profile/123'); + + // First dispatch never produces a state change. The next dispatch will + // discard it synchronously (the "a transaction was detected that turned + // out to be a noop" branch), exercising the discard path *without* + // advancing fake time past the staleness window. + mockNavigation.emitCancelledNavigation(); + + // Second dispatch is the real navigation. It must still pick up the link. + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + expect(data['navigation.trigger']).toBe('deeplink'); + expect(data['deeplink.url']).toBe('myapp://profile/'); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | same-route state change still attributes the link', async () => { + // Reproduces Cursor bugbot's "same-route path skips deeplink retry" + // finding. Deep-linking to the screen you're already on is a legitimate + // case — the dispatch still happens and an idle span still starts, so + // the link must be attached even when the resulting route key matches + // the previous one. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction + + setPendingDeepLink('myapp://initial'); + mockNavigation.navigateToInitialScreen(); // same key as the just-flushed init + jest.runOnlyPendingTimers(); + await client.flush(); + + const data = client.event?.contexts?.trace?.data ?? {}; + // We don't care which event captures it (the same-route branch ends the + // span via the idle path), but the attribute MUST be present. + expect(data['navigation.trigger']).toBe('deeplink'); + + clearPendingDeepLink(); + }); + test('deep-link hand-off | cold start: link arriving after span starts but before state change still attaches', async () => { // Simulates cold start: the React Navigation idle span is started in // `afterAllSetup`, then `Linking.getInitialURL()` resolves a microtask From d7288321cc310852a5b2c1427ab4b9ffd9b8a71b Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 9 Jun 2026 16:37:33 +0200 Subject: [PATCH 3/4] fix(tracing): Differentiate cold-start vs warm-open deep link attribution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round. Addresses two further Cursor Bugbot findings, one Warden bot finding, and three review nits from @antonis on #6264: * **Warm-open deep link can be consumed by a stale prior nav span** (Warden, medium). The previous design tagged `lastIdleNavSpan` (the most recently created idle nav span) unconditionally when a link arrived after `latestNavigationSpan` was cleared. For warm opens that meant tagging the *previous, unrelated* navigation span (still inside its idle window) with `navigation.trigger=deeplink`, while the actual link-triggered navigation that followed got nothing. Fix: `pendingDeepLink` now carries a `source: 'cold-start' | 'warm-open'` flag set by `deeplinkIntegration` based on which Linking API surfaced the URL (`getInitialURL()` vs `'url'` event). The retroactive attribution path in the navigation integration is gated on `source === 'cold-start'` and targets a single, frozen reference (`initialFinalizedNavSpan` — the first nav span that ever finished its state change) rather than a rolling "most recent" pointer. Warm opens always go through the pending slot and attach to the next dispatched navigation. * **Stale pending link not drained** (Cursor, medium). `applyPendingDeepLinkToSpan` previously early-returned without consuming the slot when the target span was already tagged (e.g. by the listener). An older link left in the slot would then attach to the next unrelated navigation. Fix: always consume the slot inside `applyPendingDeepLinkToSpan`. The function is now `void` and its only remaining responsibility is "drain pending, tag if appropriate". * **Late listener drops subsequent URLs** (Cursor, low). `handleLateDeepLink` returned `true` whenever a recording span was reachable, even if `tagSpanWithDeepLink` skipped tagging because the span was already annotated. The pendingDeepLink module then treated the URL as consumed and dropped it. Fix: `tagSpanWithDeepLink` now returns whether it actually tagged; the listener propagates that value so an already-tagged span falls through to the pending slot. Review nits: * Duplicate JSDoc above `sanitizeDeepLinkUrl` merged into a single block. * Orphaned JSDoc that had drifted above `taggedDeepLinkSpans` moved back onto `applyPendingDeepLinkToSpan` where it belongs. * Removed the redundant `dispatchToNewScreen` test helper — the existing `emitNavigationWithoutStateChange` does the same thing and is now paired with `completeStateChangeToNewScreen` in the cold-start ordering test. New tests: * `cold-start late arrival tags the initial finalized nav span while still recording` — covers the Expo Router auto-handled cold-start case. * `warm-open never tags the previous navigation span` — verifies Warden's bug is fixed. * `second link while a span is already tagged falls through to the pending slot` — verifies the listener no longer drops follow-up URLs. All 1558 SDK tests pass. --- packages/core/etc/sentry-react-native.api.md | 2 +- packages/core/src/js/integrations/deeplink.ts | 14 +-- .../core/src/js/tracing/pendingDeepLink.ts | 18 ++- .../core/src/js/tracing/reactnavigation.ts | 89 ++++++++------ .../core/test/tracing/pendingDeepLink.test.ts | 24 ++-- .../core/test/tracing/reactnavigation.test.ts | 109 ++++++++++++++---- .../core/test/tracing/reactnavigationutils.ts | 6 +- 7 files changed, 176 insertions(+), 86 deletions(-) diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index bb29a41aba..a514a5aa3c 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -900,7 +900,7 @@ export function wrapTurboModule(name: string, module: T | null // 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:90:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts -// src/js/tracing/reactnavigation.ts:224:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts +// src/js/tracing/reactnavigation.ts:223: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/integrations/deeplink.ts b/packages/core/src/js/integrations/deeplink.ts index 2bafccec8a..72ed00e46c 100644 --- a/packages/core/src/js/integrations/deeplink.ts +++ b/packages/core/src/js/integrations/deeplink.ts @@ -2,6 +2,8 @@ import type { IntegrationFn } from '@sentry/core'; import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core'; +import type { DeepLinkSource } from '../tracing/pendingDeepLink'; + import { setPendingDeepLink } from '../tracing/pendingDeepLink'; import { sanitizeUrl } from '../tracing/utils'; @@ -21,10 +23,6 @@ interface RNLinking { * to avoid capturing PII in path segments when `sendDefaultPii` is off. * * Only replaces segments that look like identifiers (all digits, UUIDs, or hex strings). - */ -/** - * Replaces dynamic path segments (UUID-like or numeric values) with a placeholder - * to avoid capturing PII in path segments when `sendDefaultPii` is off. * * Exported so the navigation integration can apply the same sanitization when * attaching a deep link URL to a navigation span. @@ -63,12 +61,12 @@ function getBreadcrumbUrl(url: string): string { return sendDefaultPii ? url : sanitizeDeepLinkUrl(url); } -function recordDeepLink(url: string): void { +function recordDeepLink(url: string, source: DeepLinkSource): void { // Hand off to the navigation integration so the next idle navigation span // can attribute itself to this deep link. Always stores the raw URL — // sanitization (if any) happens at attach time, based on the client's // `sendDefaultPii` option at that moment. - setPendingDeepLink(url); + setPendingDeepLink(url, source); const breadcrumbUrl = getBreadcrumbUrl(url); addBreadcrumb({ @@ -101,7 +99,7 @@ const _deeplinkIntegration: IntegrationFn = () => { .getInitialURL() .then((url: string | null) => { if (url) { - recordDeepLink(url); + recordDeepLink(url, 'cold-start'); } }) .catch(() => { @@ -111,7 +109,7 @@ const _deeplinkIntegration: IntegrationFn = () => { // Warm open: deep link received while app is running subscription = linking.addEventListener('url', (event: { url: string }) => { if (event?.url) { - recordDeepLink(event.url); + recordDeepLink(event.url, 'warm-open'); } }); diff --git a/packages/core/src/js/tracing/pendingDeepLink.ts b/packages/core/src/js/tracing/pendingDeepLink.ts index be29c948bf..0c4802ac0b 100644 --- a/packages/core/src/js/tracing/pendingDeepLink.ts +++ b/packages/core/src/js/tracing/pendingDeepLink.ts @@ -18,11 +18,25 @@ * slot is left empty — otherwise the link falls through to the slot. */ +/** + * How the deep link reached the integration: + * - `cold-start` — from `Linking.getInitialURL()`. The app may have been + * launched by the link; the *initial* navigation is plausibly its target, + * so retroactive attribution to an already-mounted initial span is allowed. + * - `warm-open` — from the `'url'` event while the app was running. The + * triggered navigation has not happened yet, so the URL must wait in the + * pending slot — retroactively tagging the *previous* navigation would + * attribute the link to the wrong span. + */ +export type DeepLinkSource = 'cold-start' | 'warm-open'; + export interface PendingDeepLink { /** Raw URL as received from React Native's `Linking` API. */ url: string; /** Wall-clock timestamp (ms since epoch) when the URL was received. */ receivedAtMs: number; + /** How the link arrived — governs retroactive attribution rules. */ + source: DeepLinkSource; } /** @@ -43,8 +57,8 @@ let listener: PendingDeepLinkListener | undefined; * Overwrites any previous unconsumed pending value — only the latest link * matters for correlation with the next navigation. */ -export function setPendingDeepLink(url: string): void { - const value: PendingDeepLink = { url, receivedAtMs: Date.now() }; +export function setPendingDeepLink(url: string, source: DeepLinkSource): void { + const value: PendingDeepLink = { url, receivedAtMs: Date.now(), source }; if (listener?.(value)) { return; } diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index eefd98b0d8..2838b9622d 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -234,26 +234,38 @@ export const reactNavigationIntegration = ({ let latestNavigationSpanNameCustomized: boolean = false; let navigationProcessingSpan: Span | undefined; /** - * Most recently created idle navigation span, retained even after - * `latestNavigationSpan` is cleared on state change. Used by the deep-link - * listener to attribute a link that arrives between "route mounted" and - * "idle span ended". Cleared when superseded by the next nav span. + * The first nav span that successfully completed a state change — i.e. the + * span that mounted the app's initial route. Used (and only used) by the + * deep-link listener as a retroactive attribution target for `cold-start` + * links that arrive after that state change but before the span's idle + * timeout fires. This is the Expo-Router-auto-handled-cold-start case. + * + * Never updated after the first successful state change — a `cold-start` + * link must never retroactively tag a span the user navigated to later. + * `warm-open` links bypass this entirely and always wait in the pending + * slot for the next dispatched navigation. */ - let lastIdleNavSpan: Span | undefined; + let initialFinalizedNavSpan: Span | undefined; /** - * Synchronous listener invoked the moment a deep link is recorded. If a live - * idle navigation span exists, tag it directly and tell the pendingDeepLink - * module that the link has been consumed — otherwise let the link fall through - * to the slot so the next dispatched navigation picks it up. + * Synchronous listener invoked the moment a deep link is recorded. Returns + * `true` only when the link was actually attached to a span — in that case + * the pendingDeepLink module skips storing the value. Returns `false` to let + * the link fall through to the pending slot for the next dispatched nav. */ const handleLateDeepLink = (link: PendingDeepLink): boolean => { - const span = latestNavigationSpan ?? lastIdleNavSpan; - if (!span || !isSpanRecording(span)) { - return false; + // Prefer an in-flight span (dispatch happened, state change pending). + if (latestNavigationSpan && isSpanRecording(latestNavigationSpan)) { + return tagSpanWithDeepLink(latestNavigationSpan, link); + } + // Cold-start fallback only: the initial nav span has mounted its route + // but may still be recording within its idle window. For warm opens this + // would falsely attribute the link to a previous, unrelated navigation — + // do not do it. + if (link.source === 'cold-start' && initialFinalizedNavSpan && isSpanRecording(initialFinalizedNavSpan)) { + return tagSpanWithDeepLink(initialFinalizedNavSpan, link); } - tagSpanWithDeepLink(span, link); - return true; + return false; }; let initialStateHandled: boolean = false; @@ -497,12 +509,11 @@ export const reactNavigationIntegration = ({ latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method); } - // Hold a reference to the freshly-created idle span so the deep-link - // listener can still annotate it after `latestNavigationSpan` is cleared - // on state change. We deliberately do NOT consume the pending deep link - // here — if this span is later discarded (noop / timeout / empty route), - // a still-fresh pending value must remain available for the next nav. - lastIdleNavSpan = latestNavigationSpan; + // We deliberately do NOT consume the pending deep link here — if this span + // is later discarded (noop / timeout / empty route), a still-fresh pending + // value must remain available for the next nav. The pending value is + // consumed once a span actually mounts its route (see + // `updateLatestNavigationSpanWithCurrentRoute`). if (ignoreEmptyBackNavigationTransactions) { ignoreEmptyBackNavigation(getClient(), latestNavigationSpan); } @@ -605,6 +616,13 @@ export const reactNavigationIntegration = ({ // that actually mounted a route — not one that was later discarded. applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); + // Capture the first finalized nav span for the cold-start late-arrival + // fallback. Set exactly once, then frozen — a cold-start link must never + // retroactively tag a navigation the user performed later. + if (!initialFinalizedNavSpan) { + initialFinalizedNavSpan = latestNavigationSpan; + } + navigationProcessingSpan?.updateName(`Navigation dispatch to screen ${routeName} mounted`); navigationProcessingSpan?.setStatus({ code: SPAN_STATUS_OK }); navigationProcessingSpan?.end(stateChangedTimestamp); @@ -723,13 +741,6 @@ interface NavigationContainer { getState: () => NavigationState | undefined; } -/** - * Attempts to consume the pending deep link and attach it to the given span. - * - * Returns `true` when a deep link was consumed (and the span was annotated), - * `false` otherwise. Callers may invoke this multiple times against the same - * span — once the pending value has been consumed it will not be re-applied. - */ /** * Per-span guard against double-tagging deep-link attributes. Shared between * the synchronous listener path (late arrival) and the post-state-change path. @@ -738,11 +749,13 @@ const taggedDeepLinkSpans = new WeakSet(); /** * Annotates the given span with deep-link attributes if it has not already - * been annotated. Safe to call multiple times — a span is tagged at most once. + * been annotated. Returns `true` when the span was newly tagged, `false` when + * it was already tagged (so callers can decide whether to keep the link + * around for another span). */ -function tagSpanWithDeepLink(span: Span, link: PendingDeepLink): void { +function tagSpanWithDeepLink(span: Span, link: PendingDeepLink): boolean { if (taggedDeepLinkSpans.has(span)) { - return; + return false; } taggedDeepLinkSpans.add(span); @@ -757,6 +770,7 @@ function tagSpanWithDeepLink(span: Span, link: PendingDeepLink): void { // / handled". 'deeplink.dispatch_delay_ms': Math.max(0, Date.now() - link.receivedAtMs), }); + return true; } /** Returns true if the span is still recording (has not been ended). */ @@ -764,16 +778,19 @@ function isSpanRecording(span: Span): boolean { return spanToJSON(span).timestamp === undefined; } -function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): boolean { - if (taggedDeepLinkSpans.has(span)) { - return true; - } +/** + * Attempts to consume the pending deep link and attach it to the given span. + * + * Always drains the pending slot to prevent a stale URL from leaking onto a + * later, unrelated navigation — even when the span has already been tagged + * (e.g. by `handleLateDeepLink`). + */ +function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): void { const pending = consumePendingDeepLink(maxAgeMs); if (!pending) { - return false; + return; } tagSpanWithDeepLink(span, pending); - return true; } /** diff --git a/packages/core/test/tracing/pendingDeepLink.test.ts b/packages/core/test/tracing/pendingDeepLink.test.ts index 44f71e3beb..d97f6a9d68 100644 --- a/packages/core/test/tracing/pendingDeepLink.test.ts +++ b/packages/core/test/tracing/pendingDeepLink.test.ts @@ -16,7 +16,7 @@ describe('pendingDeepLink', () => { it('returns the most recently stored URL with a receive timestamp', () => { const before = Date.now(); - setPendingDeepLink('myapp://profile/123'); + setPendingDeepLink('myapp://profile/123', 'warm-open'); const after = Date.now(); const pending = consumePendingDeepLink(1_000); @@ -26,14 +26,14 @@ describe('pendingDeepLink', () => { }); it('clears the value after a single consume', () => { - setPendingDeepLink('myapp://a'); + setPendingDeepLink('myapp://a', 'warm-open'); expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://a'); expect(consumePendingDeepLink(1_000)).toBeUndefined(); }); it('overwrites a previous pending value', () => { - setPendingDeepLink('myapp://old'); - setPendingDeepLink('myapp://new'); + setPendingDeepLink('myapp://old', 'warm-open'); + setPendingDeepLink('myapp://new', 'warm-open'); expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://new'); }); @@ -41,7 +41,7 @@ describe('pendingDeepLink', () => { const originalNow = Date.now; const baseNow = originalNow(); Date.now = (): number => baseNow; - setPendingDeepLink('myapp://stale'); + setPendingDeepLink('myapp://stale', 'warm-open'); Date.now = (): number => baseNow + 5_000; try { @@ -55,7 +55,7 @@ describe('pendingDeepLink', () => { }); it('clearPendingDeepLink removes the value without returning it', () => { - setPendingDeepLink('myapp://x'); + setPendingDeepLink('myapp://x', 'warm-open'); clearPendingDeepLink(); expect(consumePendingDeepLink(1_000)).toBeUndefined(); }); @@ -68,8 +68,8 @@ describe('pendingDeepLink', () => { return false; }); - setPendingDeepLink('myapp://a'); - setPendingDeepLink('myapp://b'); + setPendingDeepLink('myapp://a', 'warm-open'); + setPendingDeepLink('myapp://b', 'warm-open'); expect(received.map(r => r.url)).toEqual(['myapp://a', 'myapp://b']); expect(received[0]?.receivedAtMs).toBeGreaterThan(0); @@ -77,14 +77,14 @@ describe('pendingDeepLink', () => { it('skips storage when the listener returns true (already consumed)', () => { setPendingDeepLinkListener(() => true); - setPendingDeepLink('myapp://consumed-by-listener'); + setPendingDeepLink('myapp://consumed-by-listener', 'warm-open'); expect(consumePendingDeepLink(1_000)).toBeUndefined(); }); it('falls through to storage when the listener returns false', () => { setPendingDeepLinkListener(() => false); - setPendingDeepLink('myapp://stored'); + setPendingDeepLink('myapp://stored', 'warm-open'); expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://stored'); }); @@ -94,7 +94,7 @@ describe('pendingDeepLink', () => { setPendingDeepLinkListener(fn); setPendingDeepLinkListener(undefined); - setPendingDeepLink('myapp://x'); + setPendingDeepLink('myapp://x', 'warm-open'); expect(fn).not.toHaveBeenCalled(); expect(consumePendingDeepLink(1_000)?.url).toBe('myapp://x'); }); @@ -104,7 +104,7 @@ describe('pendingDeepLink', () => { setPendingDeepLinkListener(fn); clearPendingDeepLink(); - setPendingDeepLink('myapp://x'); + setPendingDeepLink('myapp://x', 'warm-open'); expect(fn).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index 3df182e58b..b802306948 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -317,7 +317,7 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient(); jest.runOnlyPendingTimers(); // Flush the init transaction - setPendingDeepLink('myapp://profile/123?token=secret'); + setPendingDeepLink('myapp://profile/123?token=secret', 'warm-open'); mockNavigation.navigateToNewScreen(); jest.runOnlyPendingTimers(); @@ -337,7 +337,7 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient({ sendDefaultPii: true }); jest.runOnlyPendingTimers(); - setPendingDeepLink('myapp://profile/123?token=secret'); + setPendingDeepLink('myapp://profile/123?token=secret', 'warm-open'); mockNavigation.navigateToNewScreen(); jest.runOnlyPendingTimers(); @@ -369,7 +369,7 @@ describe('ReactNavigationInstrumentation', () => { const originalNow = Date.now; const baseNow = originalNow(); Date.now = (): number => baseNow; - setPendingDeepLink('myapp://stale'); + setPendingDeepLink('myapp://stale', 'warm-open'); Date.now = (): number => baseNow + 5_000; try { @@ -384,24 +384,25 @@ describe('ReactNavigationInstrumentation', () => { } }); - test('deep-link hand-off | late arrival: link received between state change and idle end tags the active span', async () => { - // Reproduces Cursor bugbot's "late deep link never tags span" finding. - // Expo Router may auto-handle `Linking.getInitialURL()` and finish the - // initial navigation before our integration's own `getInitialURL()` - // chain resolves. The synchronous listener must still attribute the - // link to the in-flight (or just-mounted, still-recording) span. + test('deep-link hand-off | cold-start late arrival tags the initial finalized nav span while still recording', async () => { + // Reproduces Cursor bugbot's "late deep link never tags span" finding + // for the Expo Router cold-start case: Expo Router auto-handles + // `Linking.getInitialURL()` and the initial route mounts BEFORE our + // integration's own `getInitialURL()` chain resolves. The synchronous + // listener must attribute the link to the just-mounted-but-still- + // recording initial nav span. setupTestClient(); - jest.runOnlyPendingTimers(); // Flush the init transaction + // Do NOT flush the init transaction — we want the initial nav span to + // still be inside its idle window when the cold-start link arrives. - // Dispatch + state change — the span has now "mounted its route" but is - // still recording (idle timer hasn't fired). `latestNavigationSpan` has - // been cleared but `lastIdleNavSpan` still references the live span. - mockNavigation.navigateToNewScreen(); + // Simulate the initial route mount (Expo Router would do this on + // container mount, before our deeplink integration sees the URL). + mockNavigation.finishAppStartNavigation(); - // Link arrives LATE, after the state change handler has already run. - setPendingDeepLink('myapp://profile/123'); + // Cold-start link arrives LATE. + setPendingDeepLink('myapp://profile/123', 'cold-start'); - jest.runOnlyPendingTimers(); // Flush the navigation transaction + jest.runOnlyPendingTimers(); // Flush the transaction. await client.flush(); const data = client.event?.contexts?.trace?.data ?? {}; @@ -411,6 +412,70 @@ describe('ReactNavigationInstrumentation', () => { clearPendingDeepLink(); }); + test('deep-link hand-off | warm-open never tags the previous navigation span', async () => { + // Reproduces Warden bot's "warm-open deep link can be consumed by a + // stale prior navigation span" finding. A warm-open link must NEVER + // attach to an already-finalized previous navigation — even if that + // span is still inside its idle window. It must wait in the pending + // slot for the navigation it actually triggers. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction. + + // User navigates to screen A. State change finalizes the span, but it + // keeps recording until idle timeout. + mockNavigation.navigateToNewScreen(); + + // Warm-open link arrives WHILE spanA is still recording. + setPendingDeepLink('myapp://profile/456', 'warm-open'); + + // The next dispatch is the navigation actually triggered by the link. + // Done synchronously so the staleness-window timer does not advance + // fake `Date.now()` past `routeChangeTimeoutMs`. + mockNavigation.navigateToSecondScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + // spanB (the link-triggered navigation) gets the deeplink attributes. + const spanBEvent = client.eventQueue.find(e => e.transaction === 'Second Screen'); + expect(spanBEvent?.contexts?.trace?.data?.['navigation.trigger']).toBe('deeplink'); + expect(spanBEvent?.contexts?.trace?.data?.['deeplink.url']).toBe('myapp://profile/'); + + // spanA (the *previous* navigation, still recording when the link + // arrived) MUST NOT carry the deeplink trigger. + const spanAEvent = client.eventQueue.find(e => e.transaction === 'New Screen'); + expect(spanAEvent?.contexts?.trace?.data?.['navigation.trigger']).toBeUndefined(); + expect(spanAEvent?.contexts?.trace?.data?.['deeplink.url']).toBeUndefined(); + + clearPendingDeepLink(); + }); + + test('deep-link hand-off | second link while a span is already tagged falls through to the pending slot', async () => { + // Reproduces Cursor bugbot's "late listener drops subsequent URLs" + // finding. If `tagSpanWithDeepLink` skips tagging (span already tagged), + // the listener must return false so the URL is stored for the next nav + // rather than being silently dropped. + setupTestClient(); + // Do NOT flush — keep the initial span recording. + + mockNavigation.finishAppStartNavigation(); + setPendingDeepLink('myapp://first', 'cold-start'); + // First link tagged the initial finalized span. A second link arriving + // immediately after must NOT be lost — the listener should report + // "not consumed" so the value is stored. + setPendingDeepLink('myapp://second', 'cold-start'); + + // Synchronously dispatch the next navigation so the pending value is + // consumed before the staleness-window timer advances fake time. + mockNavigation.navigateToNewScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + const spanBEvent = client.eventQueue.find(e => e.transaction === 'New Screen'); + expect(spanBEvent?.contexts?.trace?.data?.['deeplink.url']).toBe('myapp://second'); + + clearPendingDeepLink(); + }); + test('deep-link hand-off | discarded dispatch does not consume the pending link', async () => { // Reproduces Cursor bugbot's "early consume loses pending link" finding. // A dispatch may turn out to be a noop (no state change → timeout → @@ -420,7 +485,7 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient(); jest.runOnlyPendingTimers(); // Flush the init transaction - setPendingDeepLink('myapp://profile/123'); + setPendingDeepLink('myapp://profile/123', 'warm-open'); // First dispatch never produces a state change. The next dispatch will // discard it synchronously (the "a transaction was detected that turned @@ -449,7 +514,7 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient(); jest.runOnlyPendingTimers(); // Flush the init transaction - setPendingDeepLink('myapp://initial'); + setPendingDeepLink('myapp://initial', 'warm-open'); mockNavigation.navigateToInitialScreen(); // same key as the just-flushed init jest.runOnlyPendingTimers(); await client.flush(); @@ -471,8 +536,8 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient(); jest.runOnlyPendingTimers(); // Flush the init transaction - mockNavigation.dispatchToNewScreen(); - setPendingDeepLink('myapp://profile/123'); + mockNavigation.emitNavigationWithoutStateChange(); + setPendingDeepLink('myapp://profile/123', 'cold-start'); mockNavigation.completeStateChangeToNewScreen(); jest.runOnlyPendingTimers(); @@ -489,7 +554,7 @@ describe('ReactNavigationInstrumentation', () => { setupTestClient(); jest.runOnlyPendingTimers(); - setPendingDeepLink('myapp://profile/123'); + setPendingDeepLink('myapp://profile/123', 'warm-open'); mockNavigation.navigateToNewScreen(); jest.runOnlyPendingTimers(); await client.flush(); diff --git a/packages/core/test/tracing/reactnavigationutils.ts b/packages/core/test/tracing/reactnavigationutils.ts index bf34b79984..48609d1109 100644 --- a/packages/core/test/tracing/reactnavigationutils.ts +++ b/packages/core/test/tracing/reactnavigationutils.ts @@ -82,11 +82,7 @@ export function createMockNavigationAndAttachTo(sut: ReturnType { mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); }, - /** Dispatches a NAVIGATE action targeting "New Screen" without changing route state. */ - dispatchToNewScreen: () => { - mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); - }, - /** Completes the route transition to "New Screen" by firing the state listener. */ + /** Completes the route transition to "New Screen" by firing the state listener only — pair with `emitNavigationWithoutStateChange()` to split dispatch and state change across separate steps. */ completeStateChangeToNewScreen: () => { mockedNavigationContained.currentRoute = { key: 'new_screen', From 2758a72840740548bc05510a8e1b476aacf8abec Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Wed, 10 Jun 2026 10:10:54 +0200 Subject: [PATCH 4/4] fix(tracing): Reject warm-open links that arrived before the dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the final Cursor Bugbot finding on #6264: "Warm-open tags unrelated in-flight span". Scenario: the user dispatches an unrelated navigation (state change pending, `latestNavigationSpan = spanA`). While that span is mid-flight, a warm-open deep link arrives. The previous design's listener already skipped tagging `spanA` directly (cold-start gate added last round), but the slot consumer in `updateLatestNavigationSpanWithCurrentRoute` had no way to tell that the pending warm-open link did NOT cause `spanA`. The state change would then attribute the link to `spanA`, while the actual link-triggered navigation that followed got no attributes. The fix introduces causality tracking via a shared monotonic counter: * `pendingDeepLink` now stamps every recorded link with a `seq` drawn from `nextEventSeq()`. The same counter is incremented in the nav integration every time an idle nav span is created. * Each freshly-created span is stored in a `WeakMap` with its dispatch seq. * `applyPendingDeepLinkToSpan` peeks the slot before consuming. For warm-open links, it only attaches to a span whose dispatch seq is strictly greater than the link's seq (i.e. the span was dispatched *after* the link arrived, so it can plausibly be the link's target). Rejected warm-open links are left in the slot for the next eligible dispatch. * Cold-start links keep their unconditional retroactive attribution semantics (the whole point of that source — Expo Router auto-handles the link before our `getInitialURL()` chain resolves). `peekPendingDeepLink(maxAgeMs)` is a new export that mirrors `consumePendingDeepLink` without clearing the slot, so the consumer can inspect the seq before deciding whether to take the value. A counter beats wall-clock timestamps for this purpose because Jest's fake timers (and identical-tick events in real code) can produce identical `Date.now()` values for events that are causally ordered. The seq is monotonic across the whole module, so any pair of events has a deterministic order regardless of test timing. New test: `warm-open never tags an unrelated in-flight span (dispatched but not yet finalized)` — exercises the exact scenario Cursor flagged. All 1597 SDK tests pass. --- packages/core/etc/sentry-react-native.api.md | 2 +- .../core/src/js/tracing/pendingDeepLink.ts | 44 +++++++++- .../core/src/js/tracing/reactnavigation.ts | 86 ++++++++++++++----- .../core/test/tracing/reactnavigation.test.ts | 35 ++++++++ 4 files changed, 143 insertions(+), 24 deletions(-) diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index a514a5aa3c..adccaaaa7b 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -900,7 +900,7 @@ export function wrapTurboModule(name: string, module: T | null // 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:90:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts -// src/js/tracing/reactnavigation.ts:223:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts +// src/js/tracing/reactnavigation.ts:228: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/pendingDeepLink.ts b/packages/core/src/js/tracing/pendingDeepLink.ts index 0c4802ac0b..49d16d4486 100644 --- a/packages/core/src/js/tracing/pendingDeepLink.ts +++ b/packages/core/src/js/tracing/pendingDeepLink.ts @@ -37,6 +37,14 @@ export interface PendingDeepLink { receivedAtMs: number; /** How the link arrived — governs retroactive attribution rules. */ source: DeepLinkSource; + /** + * Monotonic sequence number shared with `nextEventSeq()`. The navigation + * integration tags every dispatched nav span with a seq from the same + * sequence — a warm-open link must only consume the slot for spans whose + * seq is strictly greater than the link's, i.e. were dispatched *after* the + * link arrived. + */ + seq: number; } /** @@ -48,6 +56,16 @@ export type PendingDeepLinkListener = (link: PendingDeepLink) => boolean; let pending: PendingDeepLink | undefined; let listener: PendingDeepLinkListener | undefined; +let seqCounter = 0; + +/** + * Returns the next value in the shared monotonic sequence. Used by the + * navigation integration to stamp each dispatched nav span, so consumers can + * tell whether a pending link arrived before or after a given dispatch. + */ +export function nextEventSeq(): number { + return ++seqCounter; +} /** * Stores the most recently received deep link URL together with the current @@ -58,13 +76,34 @@ let listener: PendingDeepLinkListener | undefined; * matters for correlation with the next navigation. */ export function setPendingDeepLink(url: string, source: DeepLinkSource): void { - const value: PendingDeepLink = { url, receivedAtMs: Date.now(), source }; + const value: PendingDeepLink = { + url, + receivedAtMs: Date.now(), + source, + seq: nextEventSeq(), + }; if (listener?.(value)) { return; } pending = value; } +/** + * Returns the pending deep link without clearing the slot, applying the same + * staleness check as {@link consumePendingDeepLink}. A stale value is dropped + * (so subsequent calls do not return it) but no fresh value is returned. + */ +export function peekPendingDeepLink(maxAgeMs: number): PendingDeepLink | undefined { + if (!pending) { + return undefined; + } + if (Date.now() - pending.receivedAtMs > maxAgeMs) { + pending = undefined; + return undefined; + } + return pending; +} + /** * Returns and clears the pending deep link, but only if it was received * within `maxAgeMs` of "now". Stale entries are discarded and the slot is @@ -91,8 +130,9 @@ export function setPendingDeepLinkListener(fn: PendingDeepLinkListener | undefin listener = fn; } -/** Test helper — clears the pending value and listener without consuming them. */ +/** Test helper — clears the pending value, listener, and sequence counter. */ export function clearPendingDeepLink(): void { pending = undefined; listener = undefined; + seqCounter = 0; } diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 2838b9622d..d56f9c7262 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -29,7 +29,12 @@ import { markRootSpanForDiscard, } from './onSpanEndUtils'; import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin'; -import { consumePendingDeepLink, setPendingDeepLinkListener } from './pendingDeepLink'; +import { + consumePendingDeepLink, + nextEventSeq, + peekPendingDeepLink, + setPendingDeepLinkListener, +} from './pendingDeepLink'; import { consumePendingExpoRouterNavigation } from './pendingExpoRouterNavigation'; import { getReactNativeTracingIntegration } from './reactnativetracing'; import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes'; @@ -247,22 +252,70 @@ export const reactNavigationIntegration = ({ */ let initialFinalizedNavSpan: Span | undefined; + /** + * Monotonic dispatch sequence per span, drawn from the shared `nextEventSeq` + * counter. Used to reject warm-open links that arrived *before* a dispatch — + * such links cannot have caused that dispatch, so the span must not be + * tagged with them. + */ + const spanDispatchSeq = new WeakMap(); + + /** + * Attempts to attach the pending deep link to the given span. + * + * Warm-open links only attach to spans dispatched *after* the link was + * received. This prevents an unrelated, already-in-flight navigation from + * being tagged when a deep link arrives mid-dispatch but is actually the + * trigger of a subsequent navigation. + * + * Cold-start links attach unconditionally — retroactive attribution to the + * initial nav span is the whole point of that source. + * + * Rejected warm-open links are left in the slot to be picked up by the next + * eligible span. + */ + const applyPendingDeepLinkToSpan = (span: Span, maxAgeMs: number): void => { + const pending = peekPendingDeepLink(maxAgeMs); + if (!pending) { + return; + } + if (pending.source === 'warm-open') { + const spanSeq = spanDispatchSeq.get(span); + if (spanSeq === undefined || spanSeq <= pending.seq) { + // Span was dispatched before (or at the same tick as) the link arrived + // — it cannot be the navigation the link triggered. Leave the link in + // the slot for the next eligible span. + return; + } + } + consumePendingDeepLink(maxAgeMs); + tagSpanWithDeepLink(span, pending); + }; + /** * Synchronous listener invoked the moment a deep link is recorded. Returns * `true` only when the link was actually attached to a span — in that case * the pendingDeepLink module skips storing the value. Returns `false` to let * the link fall through to the pending slot for the next dispatched nav. + * + * Only `cold-start` links may retroactively tag an existing span. The + * realistic warm-open flow is "`'url'` event → user handler synchronously + * calls `navigation.navigate`": at listener invocation time no link-driven + * dispatch has happened yet, so any span we could reach belongs to an + * unrelated, prior navigation. */ const handleLateDeepLink = (link: PendingDeepLink): boolean => { + if (link.source !== 'cold-start') { + return false; + } // Prefer an in-flight span (dispatch happened, state change pending). if (latestNavigationSpan && isSpanRecording(latestNavigationSpan)) { return tagSpanWithDeepLink(latestNavigationSpan, link); } - // Cold-start fallback only: the initial nav span has mounted its route - // but may still be recording within its idle window. For warm opens this - // would falsely attribute the link to a previous, unrelated navigation — - // do not do it. - if (link.source === 'cold-start' && initialFinalizedNavSpan && isSpanRecording(initialFinalizedNavSpan)) { + // Fallback: the initial nav span may have already mounted its route but + // still be recording within its idle window (e.g. Expo Router auto-handled + // the link before our own `getInitialURL()` chain resolved). + if (initialFinalizedNavSpan && isSpanRecording(initialFinalizedNavSpan)) { return tagSpanWithDeepLink(initialFinalizedNavSpan, link); } return false; @@ -509,6 +562,12 @@ export const reactNavigationIntegration = ({ latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method); } + // Stamp the span with a monotonic sequence so the deep-link consumer can + // determine whether a pending link arrived before or after this dispatch. + if (latestNavigationSpan) { + spanDispatchSeq.set(latestNavigationSpan, nextEventSeq()); + } + // We deliberately do NOT consume the pending deep link here — if this span // is later discarded (noop / timeout / empty route), a still-fresh pending // value must remain available for the next nav. The pending value is @@ -778,21 +837,6 @@ function isSpanRecording(span: Span): boolean { return spanToJSON(span).timestamp === undefined; } -/** - * Attempts to consume the pending deep link and attach it to the given span. - * - * Always drains the pending slot to prevent a stale URL from leaking onto a - * later, unrelated navigation — even when the span has already been tagged - * (e.g. by `handleLateDeepLink`). - */ -function applyPendingDeepLinkToSpan(span: Span, maxAgeMs: number): void { - const pending = consumePendingDeepLink(maxAgeMs); - if (!pending) { - return; - } - tagSpanWithDeepLink(span, pending); -} - /** * Extracts the route name from a React Navigation dispatch action payload. * diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index b802306948..86bb040b7c 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -412,6 +412,41 @@ describe('ReactNavigationInstrumentation', () => { clearPendingDeepLink(); }); + test('deep-link hand-off | warm-open never tags an unrelated in-flight span (dispatched but not yet finalized)', async () => { + // Reproduces Cursor bugbot's "warm-open tags unrelated in-flight span" + // finding. If the user has just dispatched an unrelated navigation + // (state change pending) and a warm-open link arrives before the link + // handler has run, the listener must NOT attach the link to the + // in-flight span — it belongs to the (yet-to-be-dispatched) + // link-triggered navigation that follows. + setupTestClient(); + jest.runOnlyPendingTimers(); // Flush the init transaction. + + // Unrelated dispatch — `latestNavigationSpan` is now set, state change + // has not fired yet. + mockNavigation.emitNavigationWithoutStateChange(); + + // Warm-open link arrives mid-flight. + setPendingDeepLink('myapp://profile/789', 'warm-open'); + + // The unrelated dispatch finalizes (different route, not the link's). + mockNavigation.completeStateChangeToNewScreen(); + // The actual link-triggered navigation follows. + mockNavigation.navigateToSecondScreen(); + jest.runOnlyPendingTimers(); + await client.flush(); + + // The unrelated nav must NOT carry the deeplink trigger. + const newScreen = client.eventQueue.find(e => e.transaction === 'New Screen'); + expect(newScreen?.contexts?.trace?.data?.['navigation.trigger']).toBeUndefined(); + // The link-triggered nav must carry it. + const secondScreen = client.eventQueue.find(e => e.transaction === 'Second Screen'); + expect(secondScreen?.contexts?.trace?.data?.['navigation.trigger']).toBe('deeplink'); + expect(secondScreen?.contexts?.trace?.data?.['deeplink.url']).toBe('myapp://profile/'); + + clearPendingDeepLink(); + }); + test('deep-link hand-off | warm-open never tags the previous navigation span', async () => { // Reproduces Warden bot's "warm-open deep link can be consumed by a // stale prior navigation span" finding. A warm-open link must NEVER