From e14f27cbc75c7d716a28ae7b31b683033a60d8d4 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 13:39:15 +0200 Subject: [PATCH 1/6] fix(tracing): Name transaction when re-navigating to focused route with new params Navigating to the already-focused route with only params changed keeps the same route.key. The state-change handler treated this as a no-op and dropped the navigation span, which then self-finalized as an unnamed "Route Change". Distinguish same-key/params-changed re-navigation (name and finalize) from a true no-op same-route change (keep the idle path) via a shallow param compare. Fixes #6593 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 4 + .../core/src/js/tracing/reactnavigation.ts | 44 ++++++++--- .../core/test/tracing/reactnavigation.test.ts | 75 +++++++++++++++++++ 3 files changed, 114 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a858168646..bf8f2d3388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ - Add `enableNdkAppHangTracking` and `ndkAppHangTimeoutIntervalMillis` options to enable Android NDK app hang tracking ([#6548](https://github.com/getsentry/sentry-react-native/pull/6548)) +### Fixes + +- Ship a named navigation transaction when re-navigating to the focused route with new params, instead of an unnamed "Route Change" ([#6593](https://github.com/getsentry/sentry-react-native/issues/6593)) + ### Dependencies - Bump Android SDK from v8.52.0 to v8.53.0 ([#6586](https://github.com/getsentry/sentry-react-native/pull/6586)) diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 550aad6236..e4ea6c6166 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -719,13 +719,9 @@ 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. + // Attribute a pending deep link even to a same-route change (e.g. a deep + // link to the screen you're already on) before deciding on the span. applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs); - pushRecentRouteKey(route.key); - latestRoute = route; // A `POP_TO` span that landed on the route we were already on (same // `route.key`) was a params-only bookkeeping dispatch — e.g. Expo @@ -740,14 +736,27 @@ export const reactNavigationIntegration = ({ // never affects a genuine navigation that actually changed the route. if (!taggedDeepLinkSpans.has(latestNavigationSpan) && latestNavigationActionType === 'POP_TO') { debug.log(`[${INTEGRATION_NAME}] Discarding POP_TO navigation span that did not change the route.`); + pushRecentRouteKey(route.key); + latestRoute = route; clearStateChangeTimeout(); _discardLatestTransaction(); return undefined; } - // Clear the latest transaction as it has been handled. - latestNavigationSpan = undefined; - return undefined; + // Same key and unchanged params is a no-op (drawer toggle, empty + // SET_PARAMS, deep link to the current screen): keep the idle-path + // behaviour and let the span end via the idle timeout. + if (!haveRouteParamsChanged(previousRoute?.params, route.params)) { + pushRecentRouteKey(route.key); + latestRoute = route; + latestNavigationSpan = undefined; + return undefined; + } + + // Same key but changed params is a genuine navigation onto the focused + // route (e.g. navigate('Chat', { id: 2 }) from Chat/id:1). Fall through + // to name and finalize the transaction instead of leaking an unnamed + // "Route Change" (#6593). } const routeHasBeenSeen = recentRouteKeys.includes(route.key); @@ -963,6 +972,23 @@ function getRouteNameFromAction(event: UnsafeAction | undefined): string | undef return undefined; } +/** + * Shallow-compares two route param bags to tell a genuine re-navigation onto + * the already-focused route (same `route.key`, changed params) apart from a + * no-op same-route state change (#6593). Missing and empty bags are equal. + */ +function haveRouteParamsChanged( + a: Record | undefined, + b: Record | undefined, +): boolean { + const aKeys = a ? Object.keys(a) : []; + const bKeys = b ? Object.keys(b) : []; + if (aKeys.length !== bKeys.length) { + return true; + } + return aKeys.some(key => !Object.is((a as Record)[key], (b as Record)[key])); +} + /** * Returns React Navigation integration of the given client. */ diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index 5e1c2e93d0..f05be6cd89 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -1451,6 +1451,81 @@ describe('ReactNavigationInstrumentation', () => { }); }); + describe('NAVIGATE onto the already-focused route with new params (#6593)', () => { + // navigate('Chat', { id: 2 }) while Chat/id:1 is focused keeps the same + // route.key (only params change), but is a genuine screen entry and must + // ship a named transaction, not an unnamed "Route Change". + const navigateToChat: UnsafeAction = { + data: { + action: { type: 'NAVIGATE', payload: { name: 'Chat' } }, + noop: false, + stack: undefined, + }, + }; + + function countTransactionsNamed(name: string): number { + return client.eventQueue.filter(e => e.type === 'transaction' && e.transaction === name).length; + } + + test('ships a named transaction instead of an unnamed "Route Change"', async () => { + // ignoreEmptyBackNavigationTransactions: false isolates the naming + // behaviour from the empty-back-navigation discard. + const instrumentation = reactNavigationIntegration({ + routeChangeTimeoutMs: 200, + ignoreEmptyBackNavigationTransactions: false, + useDispatchedActionData: true, + }); + const options = getDefaultTestClientOptions({ + enableNativeFramesTracking: false, + enableStallTracking: false, + tracesSampleRate: 1.0, + integrations: [instrumentation], + enableAppStartTracking: false, + }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + mockNavigation = createMockNavigationAndAttachTo(instrumentation); + await jest.runOnlyPendingTimersAsync(); // Flush the initial span + + // Enter Chat for conversation 1. + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } }); + await jest.runOnlyPendingTimersAsync(); + client.eventQueue = []; + + // Tap a notification for conversation 2 — same route key, new params. + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } }); + await jest.runOnlyPendingTimersAsync(); + await client.flush(); + + expect(countTransactionsNamed('Chat')).toBe(1); + expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0); + const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat'); + expect(chat?.contexts?.trace?.op).toBe('navigation'); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('Chat'); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBe('chat'); + }); + + test('does not leak an unnamed transaction when the re-navigation is empty (default config)', async () => { + // Under the default config an empty same-key re-nav is discarded as an + // empty back navigation, not leaked as an unnamed "Route Change". + setupTestClient({ useDispatchedActionData: true }); + await jest.runOnlyPendingTimersAsync(); // Flush the initial span + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } }); + await jest.runOnlyPendingTimersAsync(); + client.eventQueue = []; + + // Re-navigate onto the already-focused "Chat" with new params (same key). + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } }); + await jest.runOnlyPendingTimersAsync(); + await client.flush(); + + expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0); + }); + }); + test('noop does not remove the previous navigation span from scope', async () => { setupTestClient({ useDispatchedActionData: true }); await jest.runOnlyPendingTimers(); // Flushes the initial navigation span From b8b30529683033807c1b0dc9d5a1a7da5e1b670f Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 13:40:38 +0200 Subject: [PATCH 2/6] docs: Reference PR in changelog entry Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf8f2d3388..51bf23d543 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Fixes -- Ship a named navigation transaction when re-navigating to the focused route with new params, instead of an unnamed "Route Change" ([#6593](https://github.com/getsentry/sentry-react-native/issues/6593)) +- Ship a named navigation transaction when re-navigating to the focused route with new params, instead of an unnamed "Route Change" ([#6606](https://github.com/getsentry/sentry-react-native/pull/6606)) ### Dependencies From d7615c44b7ae1fe729ba5d5d37b25257df872ac7 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 14:07:41 +0200 Subject: [PATCH 3/6] fix(tracing): Clear discard timer on no-op same-route change; tighten test Address Warden review on #6606: - The unchanged-params same-key branch now clears the route-change timeout and ends the processing span, so a leftover timer cannot later discard the next navigation's span. - Assert the empty default-config re-nav ships no named "Chat" transaction, so the intentional empty-back-navigation discard is pinned explicitly. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/js/tracing/reactnavigation.ts | 8 +++++++- packages/core/test/tracing/reactnavigation.test.ts | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index e4ea6c6166..f7376476d1 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -745,10 +745,16 @@ export const reactNavigationIntegration = ({ // Same key and unchanged params is a no-op (drawer toggle, empty // SET_PARAMS, deep link to the current screen): keep the idle-path - // behaviour and let the span end via the idle timeout. + // behaviour and let the span end via the idle timeout. Clear the pending + // discard timeout and end the processing span here so a leftover timer + // cannot fire later and discard the next navigation's span. if (!haveRouteParamsChanged(previousRoute?.params, route.params)) { pushRecentRouteKey(route.key); latestRoute = route; + clearStateChangeTimeout(); + navigationProcessingSpan?.setStatus({ code: SPAN_STATUS_OK }); + navigationProcessingSpan?.end(stateChangedTimestamp); + navigationProcessingSpan = undefined; latestNavigationSpan = undefined; return undefined; } diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index f05be6cd89..aeb25b418c 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -1522,7 +1522,11 @@ describe('ReactNavigationInstrumentation', () => { await jest.runOnlyPendingTimersAsync(); await client.flush(); + // Under the default config the re-nav is intentionally discarded as an + // empty back navigation, so neither an unnamed nor a named "Chat" + // transaction ships. expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0); + expect(countTransactionsNamed('Chat')).toBe(0); }); }); From 0ae559d2a377ff8dc26bf7d2d3ab8a210b1c1049 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 14:41:14 +0200 Subject: [PATCH 4/6] fix(tracing): Detect disjoint undefined-valued param keys in same-route compare haveRouteParamsChanged walked only a's keys, so equal-count bags with disjoint keys whose values are undefined (e.g. { a: undefined } vs { b: undefined }) compared equal. Require each key to exist on b too. Addresses find-bugs review. Co-Authored-By: Claude Opus 4.8 --- .../core/src/js/tracing/reactnavigation.ts | 8 +++- .../core/test/tracing/reactnavigation.test.ts | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index f7376476d1..2574103711 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -992,7 +992,13 @@ function haveRouteParamsChanged( if (aKeys.length !== bKeys.length) { return true; } - return aKeys.some(key => !Object.is((a as Record)[key], (b as Record)[key])); + // Counts are equal, so `a` non-empty implies `b` non-empty. Require every key + // to exist on `b` too — otherwise disjoint bags with matching `undefined` + // values (e.g. `{ x: undefined }` vs `{ y: undefined }`) would compare equal. + const bBag = (b ?? {}) as Record; + return aKeys.some( + key => !Object.prototype.hasOwnProperty.call(bBag, key) || !Object.is((a as Record)[key], bBag[key]), + ); } /** diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index aeb25b418c..ab28b97000 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -1507,6 +1507,43 @@ describe('ReactNavigationInstrumentation', () => { expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBe('chat'); }); + test('detects a param change when only disjoint undefined-valued keys differ', async () => { + // { a: undefined } -> { b: undefined }: same key count, disjoint keys. + // haveRouteParamsChanged must still report a change, so the span goes + // through the naming path (which sets route.key/route.name) rather than + // the no-op idle path (which does not). + const instrumentation = reactNavigationIntegration({ + routeChangeTimeoutMs: 200, + ignoreEmptyBackNavigationTransactions: false, + useDispatchedActionData: true, + }); + const options = getDefaultTestClientOptions({ + enableNativeFramesTracking: false, + enableStallTracking: false, + tracesSampleRate: 1.0, + integrations: [instrumentation], + enableAppStartTracking: false, + }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + mockNavigation = createMockNavigationAndAttachTo(instrumentation); + await jest.runOnlyPendingTimersAsync(); // Flush the initial span + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { a: undefined } }); + await jest.runOnlyPendingTimersAsync(); + client.eventQueue = []; + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { b: undefined } }); + await jest.runOnlyPendingTimersAsync(); + await client.flush(); + + const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat'); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBe('chat'); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('Chat'); + }); + test('does not leak an unnamed transaction when the re-navigation is empty (default config)', async () => { // Under the default config an empty same-key re-nav is discarded as an // empty back navigation, not leaked as an unnamed "Route Change". From 482104dee0277075158c3a1623335e7f08633d57 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 14:49:15 +0200 Subject: [PATCH 5/6] fix(tracing): Drop unnecessary type assertions in haveRouteParamsChanged oxlint no-unnecessary-type-assertion flagged the casts; b ?? {} already yields Record. Bind aBag/bBag via nullish coalescing instead. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/js/tracing/reactnavigation.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 2574103711..507773a914 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -995,10 +995,9 @@ function haveRouteParamsChanged( // Counts are equal, so `a` non-empty implies `b` non-empty. Require every key // to exist on `b` too — otherwise disjoint bags with matching `undefined` // values (e.g. `{ x: undefined }` vs `{ y: undefined }`) would compare equal. - const bBag = (b ?? {}) as Record; - return aKeys.some( - key => !Object.prototype.hasOwnProperty.call(bBag, key) || !Object.is((a as Record)[key], bBag[key]), - ); + const aBag = a ?? {}; + const bBag = b ?? {}; + return aKeys.some(key => !Object.prototype.hasOwnProperty.call(bBag, key) || !Object.is(aBag[key], bBag[key])); } /** From d9da00dfd9c24034cc8ca268a13787d45d53326d Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 24 Aug 2026 16:06:54 +0200 Subject: [PATCH 6/6] test(tracing): Cover default-config repro and no-op same-route path (#6593) Add two cases flagged by review: - useDispatchedActionData off, same-key changed params: proves the fix names the transaction instead of leaking an unnamed "Route Change" (the actual bug mode; verified to fail on the pre-fix branch). - same-key unchanged params: locks the no-op idle path (no route.key/route.name attributes set), guarding the timeout-clear + processing-span-end branch. Co-Authored-By: Claude Opus 4.8 --- .../core/test/tracing/reactnavigation.test.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index ab28b97000..c04743968d 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -1565,6 +1565,83 @@ describe('ReactNavigationInstrumentation', () => { expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0); expect(countTransactionsNamed('Chat')).toBe(0); }); + + test('names the transaction with useDispatchedActionData off (true #6593 repro)', async () => { + // With useDispatchedActionData off the idle span starts unnamed + // (DEFAULT_NAVIGATION_SPAN_NAME). Before the fix, the same-key/changed- + // params branch dropped the span reference without naming it, so it + // self-finalized as an unnamed "Route Change". This is the actual bug + // mode; the fix must derive the name from the mounted route instead. + const instrumentation = reactNavigationIntegration({ + routeChangeTimeoutMs: 200, + ignoreEmptyBackNavigationTransactions: false, + useDispatchedActionData: false, + }); + const options = getDefaultTestClientOptions({ + enableNativeFramesTracking: false, + enableStallTracking: false, + tracesSampleRate: 1.0, + integrations: [instrumentation], + enableAppStartTracking: false, + }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + mockNavigation = createMockNavigationAndAttachTo(instrumentation); + await jest.runOnlyPendingTimersAsync(); // Flush the initial span + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } }); + await jest.runOnlyPendingTimersAsync(); + client.eventQueue = []; + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } }); + await jest.runOnlyPendingTimersAsync(); + await client.flush(); + + expect(countTransactionsNamed('Chat')).toBe(1); + expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0); + }); + + test('takes the no-op idle path when the same-key re-nav does not change params', async () => { + // Same key and unchanged params ({ id: 1 } -> { id: 1 }) is a no-op: the + // span must end via the idle timeout without going through the naming + // path. useDispatchedActionData keeps it named "Chat" (from dispatch), so + // the discriminator is the absence of the route.key/route.name attributes + // that only the naming path sets. + const instrumentation = reactNavigationIntegration({ + routeChangeTimeoutMs: 200, + ignoreEmptyBackNavigationTransactions: false, + useDispatchedActionData: true, + }); + const options = getDefaultTestClientOptions({ + enableNativeFramesTracking: false, + enableStallTracking: false, + tracesSampleRate: 1.0, + integrations: [instrumentation], + enableAppStartTracking: false, + }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + mockNavigation = createMockNavigationAndAttachTo(instrumentation); + await jest.runOnlyPendingTimersAsync(); // Flush the initial span + + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } }); + await jest.runOnlyPendingTimersAsync(); + client.eventQueue = []; + + // Re-dispatch onto the focused route with identical params. + mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } }); + await jest.runOnlyPendingTimersAsync(); + await client.flush(); + + const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat'); + expect(chat).toBeDefined(); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBeUndefined(); + expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBeUndefined(); + }); }); test('noop does not remove the previous navigation span from scope', async () => {