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..502f742d4bc0 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; @@ -667,7 +660,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 +668,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,29 +725,104 @@ 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] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename); 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); } } } +/** + * 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 patchSpanEnd( + span: Span, + location: Location, + routes: RouteObject[], + basename: string | undefined, + _allRoutes: RouteObject[] | undefined, + spanType: 'pageload' | 'navigation', +): void { + const patchedPropertyName = `__sentry_${spanType}_end_patched__` as const; + const hasEndBeenPatched = (span as unknown as Record)?.[patchedPropertyName]; + + if (hasEndBeenPatched || !span.end) { + return; + } + + const originalEnd = span.end.bind(span); + + span.end = function patchedEnd(...args) { + 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, + location, + basename, + ) as unknown as RouteMatch[]; + + 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); + }; + + // Mark this span as having its end() method patched to prevent duplicate patching + addNonEnumerableProperty(span as unknown as Record, patchedPropertyName, true); +} + +function patchPageloadSpanEnd( + span: Span, + location: Location, + routes: RouteObject[], + basename: string | undefined, + _allRoutes: RouteObject[] | undefined, +): void { + patchSpanEnd(span, location, routes, basename, _allRoutes, 'pageload'); +} + +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 export function createV6CompatibleWithSentryReactRouterRouting

, R extends React.FC

>( Routes: R, 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); + }); + }); });