fix(react): Patch spanEnd for potentially cancelled lazy-route pageloads

Author: onurtemizkan

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);
+  }
+});

packages/react/src/reactrouter-compat-utils/instrumentation.tsx

@@ -667,14 +667,19 @@ 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,
           [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
           [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,146 @@ 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

packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx

@@ -140,4 +140,49 @@ describe('reactrouter-compat-utils/instrumentation', () => {
       expect(typeof integration.afterAllSetup).toBe('function');
     });
   });
+
+  describe('span.end() patching for early cancellation', () => {
+    it('should verify addNonEnumerableProperty is called to mark span as patched', () => {
+      // This test verifies the deduplication mechanism exists
+      // Direct unit testing of patchPageloadSpanEnd and patchNavigationSpanEnd would require exporting them
+      // Both pageload and navigation spans have their end() methods patched to handle early cancellation
+      // (e.g., document.hidden) before lazy routes have finished loading
+      expect(addNonEnumerableProperty).toBeDefined();
+    });
+
+    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);
+    });
+  });
 });