From fa46b4a778a0b6e37922ec6bcbd67c73efe0f5d4 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 17 Oct 2025 10:43:14 +0100 Subject: [PATCH 1/6] fix(react): Patch `spanEnd` for potentially cancelled lazy-route `pageload`s --- .../tests/transactions.test.ts | 121 ++++++++++++++ .../instrumentation.tsx | 147 ++++++++++++++++-- .../instrumentation.test.tsx | 37 +++++ 3 files changed, 291 insertions(+), 14 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts index 59d43c14ae95..3901b0938ca5 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -377,3 +377,124 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes', expect(backNavigationEvent.transaction).toBe('/'); expect(backNavigationEvent.contexts?.trace?.op).toBe('navigation'); }); + +test('Updates pageload transaction name correctly when span is cancelled early (document.hidden simulation)', async ({ + page, +}) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + // Set up the page to simulate document.hidden before navigation + await page.addInitScript(() => { + // Wait a bit for Sentry to initialize and start the pageload span + setTimeout(() => { + // Override document.hidden to simulate tab switching + Object.defineProperty(document, 'hidden', { + configurable: true, + get: function () { + return true; + }, + }); + + // Dispatch visibilitychange event to trigger the idle span cancellation logic + document.dispatchEvent(new Event('visibilitychange')); + }, 100); // Small delay to ensure the span has started + }); + + // Navigate to the lazy route URL + await page.goto('/lazy/inner/1/2/3'); + + const event = await transactionPromise; + + // Verify the lazy route content eventually loads (even though span was cancelled early) + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + await expect(lazyRouteContent).toBeVisible(); + + // Validate that the transaction event has the correct parameterized route name + // even though the span was cancelled early due to document.hidden + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); + + // Check if the span was indeed cancelled (should have idle_span_finish_reason attribute) + const idleSpanFinishReason = event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']; + if (idleSpanFinishReason) { + // If the span was cancelled due to visibility change, verify it still got the right name + expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason); + } +}); + +test('Updates navigation transaction name correctly when span is cancelled early (document.hidden simulation)', async ({ + page, +}) => { + // First go to home page + await page.goto('/'); + + const navigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + // Set up a listener to simulate document.hidden after clicking the navigation link + await page.evaluate(() => { + // Override document.hidden to simulate tab switching + let hiddenValue = false; + Object.defineProperty(document, 'hidden', { + configurable: true, + get: function () { + return hiddenValue; + }, + }); + + // Listen for clicks on the navigation link and simulate document.hidden shortly after + document.addEventListener( + 'click', + () => { + setTimeout(() => { + hiddenValue = true; + // Dispatch visibilitychange event to trigger the idle span cancellation logic + document.dispatchEvent(new Event('visibilitychange')); + }, 50); // Small delay to ensure the navigation span has started + }, + { once: true }, + ); + }); + + // Click the navigation link to navigate to the lazy route + const navigationLink = page.locator('id=navigation'); + await expect(navigationLink).toBeVisible(); + await navigationLink.click(); + + const event = await navigationPromise; + + // Verify the lazy route content eventually loads (even though span was cancelled early) + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + await expect(lazyRouteContent).toBeVisible(); + + // Validate that the transaction event has the correct parameterized route name + // even though the span was cancelled early due to document.hidden + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('navigation'); + + // Check if the span was indeed cancelled (should have cancellation_reason attribute or idle_span_finish_reason) + const cancellationReason = event.contexts?.trace?.data?.['sentry.cancellation_reason']; + const idleSpanFinishReason = event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']; + + // Verify that the span was cancelled due to document.hidden + if (cancellationReason) { + expect(cancellationReason).toBe('document.hidden'); + } + + if (idleSpanFinishReason) { + expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason); + } +}); diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index bf57fdbd74dc..70d8601c8a82 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -667,7 +667,7 @@ export function handleNavigation(opts: { // Cross usage can result in multiple navigation spans being created without this check if (!isAlreadyInNavigationSpan) { - startBrowserTracingNavigationSpan(client, { + const navigationSpan = startBrowserTracingNavigationSpan(client, { name, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, @@ -675,6 +675,11 @@ export function handleNavigation(opts: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`, }, }); + + // Patch navigation span to handle early cancellation (e.g., document.hidden) + if (navigationSpan) { + patchNavigationSpanEnd(navigationSpan, location, routes, basename, allRoutes); + } } } } @@ -727,27 +732,141 @@ function updatePageloadTransaction({ : (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]); if (branches) { - let name, - source: TransactionSource = 'url'; - - const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes); - - if (isInDescendantRoute) { - name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location)); - source = 'route'; - } - - if (!isInDescendantRoute || !name) { - [name, source] = getNormalizedName(routes, location, branches, basename); - } + const [name, source] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes); getCurrentScope().setTransactionName(name || '/'); if (activeRootSpan) { activeRootSpan.updateName(name); activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + + // Patch span.end() to ensure we update the name one last time before the span is sent + patchPageloadSpanEnd(activeRootSpan, location, routes, basename, allRoutes); + } + } +} + +/** + * Extracts the transaction name and source from the route information. + */ +function getTransactionNameAndSource( + location: Location, + routes: RouteObject[], + branches: RouteMatch[], + basename: string | undefined, + allRoutes: RouteObject[] | undefined, +): [string, TransactionSource] { + let name: string | undefined; + let source: TransactionSource = 'url'; + + const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes); + + if (isInDescendantRoute) { + name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location)); + source = 'route'; + } + + if (!isInDescendantRoute || !name) { + [name, source] = getNormalizedName(routes, location, branches, basename); + } + + return [name || '/', source]; +} + +/** + * Patches the span.end() method to update the transaction name one last time before the span is sent. + * This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading. + */ +function patchPageloadSpanEnd( + span: Span, + location: Location, + routes: RouteObject[], + basename: string | undefined, + allRoutes: RouteObject[] | undefined, +): void { + const hasEndBeenPatched = (span as { __sentry_pageload_end_patched__?: boolean })?.__sentry_pageload_end_patched__; + + if (hasEndBeenPatched || !span.end) { + return; + } + + const originalEnd = span.end.bind(span); + + span.end = function patchedEnd(...args) { + // Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet) + const spanJson = spanToJSON(span); + const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; + if (currentSource !== 'route') { + // Last chance to update the transaction name with the latest route info + const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]; + + if (branches) { + const [latestName, latestSource] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes); + + span.updateName(latestName); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, latestSource); + } } + + return originalEnd(...args); + }; + + // Mark this span as having its end() method patched to prevent duplicate patching + addNonEnumerableProperty( + span as { __sentry_pageload_end_patched__?: boolean }, + '__sentry_pageload_end_patched__', + true, + ); +} + +/** + * Patches the navigation span.end() method to update the transaction name one last time before the span is sent. + * This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading. + */ +function patchNavigationSpanEnd( + span: Span, + location: Location, + routes: RouteObject[], + basename: string | undefined, + allRoutes: RouteObject[] | undefined, +): void { + const hasEndBeenPatched = (span as { __sentry_navigation_end_patched__?: boolean }) + ?.__sentry_navigation_end_patched__; + + if (hasEndBeenPatched || !span.end) { + return; } + + const originalEnd = span.end.bind(span); + + span.end = function patchedEnd(...args) { + // Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet) + const spanJson = spanToJSON(span); + const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; + if (currentSource !== 'route') { + // Last chance to update the transaction name with the latest route info + const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]; + + if (branches) { + const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename); + + // Only update if we have a valid name and the span hasn't finished + if (name && !spanJson.timestamp) { + span.updateName(name); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + } + } + } + + return originalEnd(...args); + }; + + // Mark this span as having its end() method patched to prevent duplicate patching + addNonEnumerableProperty( + span as { __sentry_navigation_end_patched__?: boolean }, + '__sentry_navigation_end_patched__', + true, + ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx index 0eeeeb342287..4e741be7df9d 100644 --- a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -140,4 +140,41 @@ describe('reactrouter-compat-utils/instrumentation', () => { expect(typeof integration.afterAllSetup).toBe('function'); }); }); + + describe('span.end() patching for early cancellation', () => { + it('should update transaction name when span.end() is called during cancellation', () => { + const mockEnd = vi.fn(); + let patchedEnd: ((...args: any[]) => any) | null = null; + + const updateNameMock = vi.fn(); + const setAttributeMock = vi.fn(); + + const testSpan = { + updateName: updateNameMock, + setAttribute: setAttributeMock, + get end() { + return patchedEnd || mockEnd; + }, + set end(fn: (...args: any[]) => any) { + patchedEnd = fn; + }, + } as unknown as Span; + + // Simulate the patching behavior + const originalEnd = testSpan.end.bind(testSpan); + (testSpan as any).end = function patchedEndFn(...args: any[]) { + // This simulates what happens in the actual implementation + updateNameMock('Updated Route'); + setAttributeMock('sentry.source', 'route'); + return originalEnd(...args); + }; + + // Call the patched end + testSpan.end(12345); + + expect(updateNameMock).toHaveBeenCalledWith('Updated Route'); + expect(setAttributeMock).toHaveBeenCalledWith('sentry.source', 'route'); + expect(mockEnd).toHaveBeenCalledWith(12345); + }); + }); }); From bd6da3310cfceaaa4a5733a0c1025041c65abf10 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 20 Oct 2025 14:06:08 +0100 Subject: [PATCH 2/6] Update patch functions to use live global allRoutes Set for lazy route handling --- .../instrumentation.tsx | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 70d8601c8a82..a2a60068ff7a 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -782,7 +782,7 @@ function patchPageloadSpanEnd( location: Location, routes: RouteObject[], basename: string | undefined, - allRoutes: RouteObject[] | undefined, + _allRoutes: RouteObject[] | undefined, ): void { const hasEndBeenPatched = (span as { __sentry_pageload_end_patched__?: boolean })?.__sentry_pageload_end_patched__; @@ -798,10 +798,18 @@ function patchPageloadSpanEnd( const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; if (currentSource !== 'route') { // Last chance to update the transaction name with the latest route info - const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]; + // Use the live global allRoutes Set to include any lazy routes loaded after patching + const currentAllRoutes = Array.from(allRoutes); + const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; if (branches) { - const [latestName, latestSource] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes); + const [latestName, latestSource] = getTransactionNameAndSource( + location, + routes, + branches, + basename, + currentAllRoutes, + ); span.updateName(latestName); span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, latestSource); @@ -828,7 +836,7 @@ function patchNavigationSpanEnd( location: Location, routes: RouteObject[], basename: string | undefined, - allRoutes: RouteObject[] | undefined, + _allRoutes: RouteObject[] | undefined, ): void { const hasEndBeenPatched = (span as { __sentry_navigation_end_patched__?: boolean }) ?.__sentry_navigation_end_patched__; @@ -845,10 +853,12 @@ function patchNavigationSpanEnd( const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; if (currentSource !== 'route') { // Last chance to update the transaction name with the latest route info - const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]; + // Use the live global allRoutes Set to include any lazy routes loaded after patching + const currentAllRoutes = Array.from(allRoutes); + const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; if (branches) { - const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename); + const [name, source] = resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); // Only update if we have a valid name and the span hasn't finished if (name && !spanJson.timestamp) { From 657256e2cd4b3b359476c0f460ca4d28cea7cd14 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 20 Oct 2025 15:12:54 +0100 Subject: [PATCH 3/6] Deduplicate span end logic --- .../instrumentation.tsx | 84 ++++++------------- 1 file changed, 25 insertions(+), 59 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index a2a60068ff7a..b27de77d806c 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -777,14 +777,16 @@ function getTransactionNameAndSource( * Patches the span.end() method to update the transaction name one last time before the span is sent. * This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading. */ -function patchPageloadSpanEnd( +function patchSpanEnd( span: Span, location: Location, routes: RouteObject[], basename: string | undefined, _allRoutes: RouteObject[] | undefined, + spanType: 'pageload' | 'navigation', ): void { - const hasEndBeenPatched = (span as { __sentry_pageload_end_patched__?: boolean })?.__sentry_pageload_end_patched__; + const patchedPropertyName = `__sentry_${spanType}_end_patched__` as const; + const hasEndBeenPatched = (span as unknown as Record)?.[patchedPropertyName]; if (hasEndBeenPatched || !span.end) { return; @@ -803,16 +805,16 @@ function patchPageloadSpanEnd( const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; if (branches) { - const [latestName, latestSource] = getTransactionNameAndSource( - location, - routes, - branches, - basename, - currentAllRoutes, - ); + const [name, source] = + spanType === 'pageload' + ? getTransactionNameAndSource(location, routes, branches, basename, currentAllRoutes) + : resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); - span.updateName(latestName); - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, latestSource); + // Only update if we have a valid name + if (name && (spanType === 'pageload' || !spanJson.timestamp)) { + span.updateName(name); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + } } } @@ -820,63 +822,27 @@ function patchPageloadSpanEnd( }; // Mark this span as having its end() method patched to prevent duplicate patching - addNonEnumerableProperty( - span as { __sentry_pageload_end_patched__?: boolean }, - '__sentry_pageload_end_patched__', - true, - ); + addNonEnumerableProperty(span as unknown as Record, patchedPropertyName, true); } -/** - * Patches the navigation span.end() method to update the transaction name one last time before the span is sent. - * This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading. - */ -function patchNavigationSpanEnd( +function patchPageloadSpanEnd( span: Span, location: Location, routes: RouteObject[], basename: string | undefined, _allRoutes: RouteObject[] | undefined, ): void { - const hasEndBeenPatched = (span as { __sentry_navigation_end_patched__?: boolean }) - ?.__sentry_navigation_end_patched__; - - if (hasEndBeenPatched || !span.end) { - return; - } - - const originalEnd = span.end.bind(span); - - span.end = function patchedEnd(...args) { - // Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet) - const spanJson = spanToJSON(span); - const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; - if (currentSource !== 'route') { - // Last chance to update the transaction name with the latest route info - // Use the live global allRoutes Set to include any lazy routes loaded after patching - const currentAllRoutes = Array.from(allRoutes); - const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; - - if (branches) { - const [name, source] = resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); - - // Only update if we have a valid name and the span hasn't finished - if (name && !spanJson.timestamp) { - span.updateName(name); - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); - } - } - } - - return originalEnd(...args); - }; + patchSpanEnd(span, location, routes, basename, _allRoutes, 'pageload'); +} - // Mark this span as having its end() method patched to prevent duplicate patching - addNonEnumerableProperty( - span as { __sentry_navigation_end_patched__?: boolean }, - '__sentry_navigation_end_patched__', - true, - ); +function patchNavigationSpanEnd( + span: Span, + location: Location, + routes: RouteObject[], + basename: string | undefined, + _allRoutes: RouteObject[] | undefined, +): void { + patchSpanEnd(span, location, routes, basename, _allRoutes, 'navigation'); } // eslint-disable-next-line @typescript-eslint/no-explicit-any From e7270bc6e7ca05fcf14eb4f7bfe1e79bfd5e2fbd Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 20 Oct 2025 15:53:26 +0100 Subject: [PATCH 4/6] Deduplicate transaction name extractors --- .../instrumentation.tsx | 45 ++----------------- 1 file changed, 4 insertions(+), 41 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index b27de77d806c..4a47f6b91330 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -8,7 +8,7 @@ import { startBrowserTracingPageLoadSpan, WINDOW, } from '@sentry/browser'; -import type { Client, Integration, Span, TransactionSource } from '@sentry/core'; +import type { Client, Integration, Span } from '@sentry/core'; import { addNonEnumerableProperty, debug, @@ -41,14 +41,7 @@ import type { UseRoutes, } from '../types'; import { checkRouteForAsyncHandler } from './lazy-routes'; -import { - getNormalizedName, - initializeRouterUtils, - locationIsInsideDescendantRoute, - prefixWithSlash, - rebuildRoutePathFromAllRoutes, - resolveRouteNameAndSource, -} from './utils'; +import { initializeRouterUtils, resolveRouteNameAndSource } from './utils'; let _useEffect: UseEffect; let _useLocation: UseLocation; @@ -732,7 +725,7 @@ function updatePageloadTransaction({ : (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]); if (branches) { - const [name, source] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes); + const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename); getCurrentScope().setTransactionName(name || '/'); @@ -746,33 +739,6 @@ function updatePageloadTransaction({ } } -/** - * Extracts the transaction name and source from the route information. - */ -function getTransactionNameAndSource( - location: Location, - routes: RouteObject[], - branches: RouteMatch[], - basename: string | undefined, - allRoutes: RouteObject[] | undefined, -): [string, TransactionSource] { - let name: string | undefined; - let source: TransactionSource = 'url'; - - const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes); - - if (isInDescendantRoute) { - name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location)); - source = 'route'; - } - - if (!isInDescendantRoute || !name) { - [name, source] = getNormalizedName(routes, location, branches, basename); - } - - return [name || '/', source]; -} - /** * Patches the span.end() method to update the transaction name one last time before the span is sent. * This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading. @@ -805,10 +771,7 @@ function patchSpanEnd( const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; if (branches) { - const [name, source] = - spanType === 'pageload' - ? getTransactionNameAndSource(location, routes, branches, basename, currentAllRoutes) - : resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); + const [name, source] = resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); // Only update if we have a valid name if (name && (spanType === 'pageload' || !spanJson.timestamp)) { From bf7633ac3340953b22613a7dca8e7a498041656a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 22 Oct 2025 15:24:59 +0100 Subject: [PATCH 5/6] Address seer's review --- .../reactrouter-compat-utils/instrumentation.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 4a47f6b91330..bce2d7a4fde5 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -768,10 +768,20 @@ function patchSpanEnd( // Last chance to update the transaction name with the latest route info // Use the live global allRoutes Set to include any lazy routes loaded after patching const currentAllRoutes = Array.from(allRoutes); - const branches = _matchRoutes(currentAllRoutes || routes, location, basename) as unknown as RouteMatch[]; + const branches = _matchRoutes( + currentAllRoutes.length > 0 ? currentAllRoutes : routes, + location, + basename, + ) as unknown as RouteMatch[]; if (branches) { - const [name, source] = resolveRouteNameAndSource(location, routes, currentAllRoutes, branches, basename); + const [name, source] = resolveRouteNameAndSource( + location, + routes, + currentAllRoutes.length > 0 ? currentAllRoutes : routes, + branches, + basename, + ); // Only update if we have a valid name if (name && (spanType === 'pageload' || !spanJson.timestamp)) { From 30860c8c05657606eb9a56257878d8e3b4e1d9af Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 22 Oct 2025 16:09:44 +0100 Subject: [PATCH 6/6] Error handle patched logic of `span.end` --- .../instrumentation.tsx | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index bce2d7a4fde5..502f742d4bc0 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -761,34 +761,39 @@ function patchSpanEnd( const originalEnd = span.end.bind(span); span.end = function patchedEnd(...args) { - // Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet) - const spanJson = spanToJSON(span); - const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; - if (currentSource !== 'route') { - // Last chance to update the transaction name with the latest route info - // Use the live global allRoutes Set to include any lazy routes loaded after patching - const currentAllRoutes = Array.from(allRoutes); - const branches = _matchRoutes( - currentAllRoutes.length > 0 ? currentAllRoutes : routes, - location, - basename, - ) as unknown as RouteMatch[]; - - if (branches) { - const [name, source] = resolveRouteNameAndSource( - location, - routes, + try { + // Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet) + const spanJson = spanToJSON(span); + const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; + if (currentSource !== 'route') { + // Last chance to update the transaction name with the latest route info + // Use the live global allRoutes Set to include any lazy routes loaded after patching + const currentAllRoutes = Array.from(allRoutes); + const branches = _matchRoutes( currentAllRoutes.length > 0 ? currentAllRoutes : routes, - branches, + location, basename, - ); + ) as unknown as RouteMatch[]; - // Only update if we have a valid name - if (name && (spanType === 'pageload' || !spanJson.timestamp)) { - span.updateName(name); - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + if (branches) { + const [name, source] = resolveRouteNameAndSource( + location, + routes, + currentAllRoutes.length > 0 ? currentAllRoutes : routes, + branches, + basename, + ); + + // Only update if we have a valid name + if (name && (spanType === 'pageload' || !spanJson.timestamp)) { + span.updateName(name); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + } } } + } catch (error) { + // Silently catch errors to ensure span.end() is always called + DEBUG_BUILD && debug.warn(`Error updating span details before ending: ${error}`); } return originalEnd(...args);