From 16d5d6801c0b0a0915b3e9021a90649e9f7d2c26 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 11 Sep 2026 13:52:45 -0400 Subject: [PATCH 1/3] fix(react): Don't rename a navigation span with another location's route React Router resolves the routes for a location around the time the SDK starts that navigation's span, in either order, so a late resolution can hold the previous navigation's span while carrying the next location's path. Renaming it there shipped a navigation span under a route that was never visited on it. Each navigation span now records the pathname it was started for, and a rename is refused when the location being resolved does not match it. Keying this on the span rather than on whichever navigation is currently tracked matters, since by the time a slow lazy handler resolves, the tracker has usually moved on to the next span. --- .../instrumentation.tsx | 19 +++++++------ .../instrumentation.test.tsx | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f6019445a2a2..8f7bd5b09bd6 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -374,16 +374,14 @@ export function updateNavigationSpan( ): void { const { name: currentName, end_timestamp, attributes } = spanToJSON(activeRootSpan); - // React Router can start resolving the routes for the next location before the SDK has started - // that navigation's span, so the span captured for the resolution is still the previous - // navigation's. Renaming it would ship it under a route the user never visited on it, so a span - // the SDK is still tracking for a different location is left alone. - const client = getClient(); - const trackedNav = client ? activeNavigationSpans.get(client) : undefined; - if (trackedNav?.span === activeRootSpan && trackedNav?.pathname !== location.pathname) { + // React Router resolves the routes for a location around the time the SDK starts that + // navigation's span, in either order, so the span captured for a resolution can be the previous + // navigation's. Renaming it would ship it under a route the user never visited on it. + const spanPathname = (activeRootSpan as { __sentry_navigation_pathname__?: string })?.__sentry_navigation_pathname__; + if (spanPathname !== undefined && spanPathname !== location.pathname) { DEBUG_BUILD && debug.log( - `[React Router] Not renaming the navigation span for "${trackedNav.pathname}" with the route of "${location.pathname}"`, + `[React Router] Not renaming the navigation span for "${spanPathname}" with the route of "${location.pathname}"`, ); return; } @@ -411,6 +409,7 @@ export function updateNavigationSpan( (currentSource === 'route' && source === 'route' && currentNameHasWildcard)); // Route → better route (only if current has wildcard) if (isImprovement) { // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. + const client = getClient(); const isUnparameterizedStreamedNavigation = source !== 'route' && !!client && hasSpanStreamingEnabled(client); activeRootSpan.updateName(isUnparameterizedStreamedNavigation ? NAVIGATION_SPAN_NAME_FALLBACK : name); activeRootSpan.setAttribute(SENTRY_SEGMENT_NAME_SOURCE, source); @@ -1091,6 +1090,10 @@ export function handleNavigation(opts: { } if (navigationSpan) { + // Recorded on the span itself so a late route resolution can tell whether the span it + // captured is the one that navigation belongs to, however far the tracked navigation has + // moved on by then. + addNonEnumerableProperty(navigationSpan, '__sentry_navigation_pathname__', location.pathname); // Update the map with the real span (isPlaceholder omitted, defaults to false) activeNavigationSpans.set(client, { span: navigationSpan, diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx index efe7228d778b..fd6f00d13690 100644 --- a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -134,6 +134,34 @@ describe('reactrouter-compat-utils/instrumentation', () => { expect(mockUpdateName).not.toHaveBeenCalled(); }); + + it('should not rename a span that was started for another location', () => { + const spanOfPreviousNavigation = { ...mockSpan, __sentry_navigation_pathname__: '/previous' }; + + updateNavigationSpan( + spanOfPreviousNavigation as any, + sampleLocation, + sampleRoutes, + true, + makeMockConfig({ matchRoutes: mockMatchRoutes }), + ); + + expect(mockUpdateName).not.toHaveBeenCalled(); + }); + + it('should rename a span that was started for this location', () => { + const spanOfThisNavigation = { ...mockSpan, __sentry_navigation_pathname__: sampleLocation.pathname }; + + updateNavigationSpan( + spanOfThisNavigation as any, + sampleLocation, + sampleRoutes, + false, + makeMockConfig({ matchRoutes: mockMatchRoutes }), + ); + + expect(mockUpdateName).toHaveBeenCalledWith('Test Route'); + }); }); describe('addResolvedRoutesToParent', () => { From 698017db8b8b5fd9f17dac5941eacb5b926d4af0 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 11 Sep 2026 14:17:06 -0400 Subject: [PATCH 2/3] ref(react): Trim the navigation span rename comments --- .../src/reactrouter-compat-utils/instrumentation.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 8f7bd5b09bd6..ff860013dd59 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -374,9 +374,8 @@ export function updateNavigationSpan( ): void { const { name: currentName, end_timestamp, attributes } = spanToJSON(activeRootSpan); - // React Router resolves the routes for a location around the time the SDK starts that - // navigation's span, in either order, so the span captured for a resolution can be the previous - // navigation's. Renaming it would ship it under a route the user never visited on it. + // React Router resolves a location's routes either side of the SDK starting that navigation's + // span, so a resolution can arrive holding the previous navigation's span. const spanPathname = (activeRootSpan as { __sentry_navigation_pathname__?: string })?.__sentry_navigation_pathname__; if (spanPathname !== undefined && spanPathname !== location.pathname) { DEBUG_BUILD && @@ -1090,9 +1089,8 @@ export function handleNavigation(opts: { } if (navigationSpan) { - // Recorded on the span itself so a late route resolution can tell whether the span it - // captured is the one that navigation belongs to, however far the tracked navigation has - // moved on by then. + // On the span rather than only in the tracked entry, which a late resolution finds already + // moved on to the next navigation. addNonEnumerableProperty(navigationSpan, '__sentry_navigation_pathname__', location.pathname); // Update the map with the real span (isPlaceholder omitted, defaults to false) activeNavigationSpans.set(client, { From b69dc88845fd1c07d49ebf77692dddcb69924236 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 14 Sep 2026 09:07:06 -0400 Subject: [PATCH 3/3] test(e2e): Cover the stale navigation span rename Navigating between two lazy route trees without waiting leaves the first navigation's span open while React Router resolves the second location's routes, which is the window where the rename used to land on the wrong span. The test asserts each navigation is reported under its own route. --- .../tests/spans.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts index d0dc72192dca..e5aed8fda13b 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts @@ -1507,3 +1507,33 @@ test('GQL fetch spans are attributed to correct navigation segments when navigat expect(secondNavSegment.trace_id).toBeDefined(); expect(firstNavSegment.trace_id).not.toBe(secondNavSegment.trace_id); }); + +test('Does not rename the previous navigation span with the route of the next navigation', async ({ page }) => { + const navigationNames: string[] = []; + + const navigationsPromise = waitForStreamedSpan('react-router-7-lazy-routes', span => { + if (getSpanOp(span) === 'navigation' && span.is_segment) { + navigationNames.push(span.name); + } + + return navigationNames.length >= 2; + }); + + await page.goto('/'); + + const toAnotherLazy = page.locator('id=navigation-to-another-deep'); + await expect(toAnotherLazy).toBeVisible(); + await toAnotherLazy.click(); + + // No wait in between: the second navigation has to start while the first navigation's span is + // still open, which is when React Router hands it the second location's routes. + const toInnerLazy = page.locator('id=navigate-to-inner-from-deep'); + await expect(toInnerLazy).toBeVisible(); + await toInnerLazy.click(); + + await expect(page.locator('id=innermost-lazy-route')).toBeVisible(); + + await navigationsPromise; + + expect(navigationNames).toEqual(['/another-lazy/sub/:id/:subId', '/lazy/inner/:id/:anotherId/:someAnotherId']); +});