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,54 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
   expect(backNavigationEvent.transaction).toBe('/');
   expect(backNavigationEvent.contexts?.trace?.op).toBe('navigation');
 });
+
+test('Updates 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);
+  }
+});

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

@@ -727,29 +727,88 @@ 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) {
+    // 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,
+  );
+}
+
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<string, any>, R extends React.FC<P>>(
   Routes: R,

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

@@ -140,4 +140,47 @@ 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 would require exporting it
+      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);
+    });
+  });
 });