fix(react): Prevent lazy route handlers from updating wrong navigation span

Author: onurtemizkan

dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

@@ -1228,3 +1228,134 @@ test('Query/hash navigation does not corrupt transaction name', async ({ page })
   const corruptedToRoot = navigationTransactions.filter(t => t.name === '/');
   expect(corruptedToRoot.length).toBe(0);
 });
+
+test('Captured navigation context is used instead of stale window.location during rapid navigation', async ({
+  page,
+}) => {
+  // Validates fix for race condition where captureCurrentLocation would use stale WINDOW.location.
+  // Navigate to slow route, then quickly to another route before lazy handler resolves.
+  await page.goto('/');
+
+  const allNavigationTransactions: Array<{ name: string; traceId: string }> = [];
+
+  const collectorPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    if (transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation') {
+      allNavigationTransactions.push({
+        name: transactionEvent.transaction,
+        traceId: transactionEvent.contexts.trace.trace_id || '',
+      });
+    }
+    return allNavigationTransactions.length >= 2;
+  });
+
+  const slowFetchLink = page.locator('id=navigation-to-slow-fetch');
+  await expect(slowFetchLink).toBeVisible();
+  await slowFetchLink.click();
+
+  // Navigate away quickly before slow-fetch's async handler resolves
+  await page.waitForTimeout(50);
+
+  const anotherLink = page.locator('id=navigation-to-another');
+  await anotherLink.click();
+
+  await expect(page.locator('id=another-lazy-route')).toBeVisible({ timeout: 10000 });
+
+  await page.waitForTimeout(2000);
+
+  await Promise.race([
+    collectorPromise,
+    new Promise<'timeout'>(resolve => setTimeout(() => resolve('timeout'), 3000)),
+  ]).catch(() => {});
+
+  expect(allNavigationTransactions.length).toBeGreaterThanOrEqual(1);
+
+  // /another-lazy transaction must have correct name (not corrupted by slow-fetch handler)
+  const anotherLazyTransaction = allNavigationTransactions.find(
+    t =>
+      t.name.startsWith('/another-lazy/sub'),
+  );
+  expect(anotherLazyTransaction).toBeDefined();
+
+  const corruptedToRoot = allNavigationTransactions.filter(t => t.name === '/');
+  expect(corruptedToRoot.length).toBe(0);
+
+  if (allNavigationTransactions.length >= 2) {
+    const uniqueNames = new Set(allNavigationTransactions.map(t => t.name));
+    expect(uniqueNames.size).toBe(allNavigationTransactions.length);
+  }
+});
+
+test('Second navigation span is not corrupted by first slow lazy handler completing late', async ({ page }) => {
+  // Validates fix for race condition where slow lazy handler would update the wrong span.
+  // Navigate to slow route (which fetches /api/slow-data), then quickly to fast route.
+  // Without fix: second transaction gets wrong name and/or contains leaked spans.
+
+  await page.goto('/');
+
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const allNavigationTransactions: Array<{ name: string; traceId: string; spans: any[] }> = [];
+
+  const collectorPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    if (transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation') {
+      allNavigationTransactions.push({
+        name: transactionEvent.transaction,
+        traceId: transactionEvent.contexts.trace.trace_id || '',
+        spans: transactionEvent.spans || [],
+      });
+    }
+    return false;
+  });
+
+  // Navigate to slow-fetch (500ms lazy delay, fetches /api/slow-data)
+  const slowFetchLink = page.locator('id=navigation-to-slow-fetch');
+  await expect(slowFetchLink).toBeVisible();
+  await slowFetchLink.click();
+
+  // Wait 150ms (before 500ms lazy loading completes), then navigate away
+  await page.waitForTimeout(150);
+
+  const anotherLink = page.locator('id=navigation-to-another');
+  await anotherLink.click();
+
+  await expect(page.locator('id=another-lazy-route')).toBeVisible({ timeout: 10000 });
+
+  // Wait for slow-fetch lazy handler to complete and transactions to be sent
+  await page.waitForTimeout(2000);
+
+  await Promise.race([
+    collectorPromise,
+    new Promise<'timeout'>(resolve => setTimeout(() => resolve('timeout'), 3000)),
+  ]).catch(() => {});
+
+  expect(allNavigationTransactions.length).toBeGreaterThanOrEqual(1);
+
+  // /another-lazy transaction must have correct name, not "/slow-fetch/:id"
+  const anotherLazyTransaction = allNavigationTransactions.find(
+    t =>
+      t.name.startsWith('/another-lazy/sub'),
+  );
+  expect(anotherLazyTransaction).toBeDefined();
+
+  // Key assertion 2: /another-lazy transaction must NOT contain spans from /slow-fetch route
+  // The /api/slow-data fetch is triggered by the slow-fetch route's lazy loading
+  if (anotherLazyTransaction) {
+    const leakedSpans = anotherLazyTransaction.spans.filter(
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (span: any) => span.description?.includes('slow-data') || span.data?.url?.includes('slow-data'),
+    );
+    expect(leakedSpans.length).toBe(0);
+  }
+
+  // Key assertion 3: If slow-fetch transaction exists, verify it has the correct name
+  // (not corrupted to /another-lazy)
+  const slowFetchTransaction = allNavigationTransactions.find(t => t.name.includes('slow-fetch'));
+  if (slowFetchTransaction) {
+    expect(slowFetchTransaction.name).toMatch(/\/slow-fetch/);
+    // Verify slow-fetch transaction doesn't contain spans that belong to /another-lazy
+    const wrongSpans = slowFetchTransaction.spans.filter(
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (span: any) => span.description?.includes('another-lazy') || span.data?.url?.includes('another-lazy'),
+    );
+    expect(wrongSpans.length).toBe(0);
+  }
+});

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

@@ -730,15 +730,20 @@ function wrapPatchRoutesOnNavigation(
           // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
           (args as any).patch = (routeId: string, children: RouteObject[]) => {
             addRoutesToAllRoutes(children);
-            const currentActiveRootSpan = getActiveRootSpan();
-            // Only update if we have a valid targetPath (patchRoutesOnNavigation can be called without path)
+            // Use the captured activeRootSpan instead of getActiveRootSpan() to avoid race conditions
+            // where user navigates away during lazy route loading and we'd update the wrong span
+            const spanJson = activeRootSpan ? spanToJSON(activeRootSpan) : undefined;
+            // Only update if we have a valid targetPath (patchRoutesOnNavigation can be called without path),
+            // the captured span exists, hasn't ended, and is a navigation span
             if (
               targetPath &&
-              currentActiveRootSpan &&
-              (spanToJSON(currentActiveRootSpan) as { op?: string }).op === 'navigation'
+              activeRootSpan &&
+              spanJson &&
+              !spanJson.timestamp && // Span hasn't ended yet
+              spanJson.op === 'navigation'
             ) {
               updateNavigationSpan(
-                currentActiveRootSpan,
+                activeRootSpan,
                 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
                 Array.from(allRoutes),
                 true,
@@ -760,13 +765,22 @@ function wrapPatchRoutesOnNavigation(
           clearNavigationContext(contextToken);
         }
 
-        const currentActiveRootSpan = getActiveRootSpan();
-        if (currentActiveRootSpan && (spanToJSON(currentActiveRootSpan) as { op?: string }).op === 'navigation') {
-          const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname;
+        // Use the captured activeRootSpan instead of getActiveRootSpan() to avoid race conditions
+        // where user navigates away during lazy route loading and we'd update the wrong span
+        const spanJson = activeRootSpan ? spanToJSON(activeRootSpan) : undefined;
+        if (
+          activeRootSpan &&
+          spanJson &&
+          !spanJson.timestamp && // Span hasn't ended yet
+          spanJson.op === 'navigation'
+        ) {
+          // Use targetPath consistently - don't fall back to WINDOW.location which may have changed
+          // if the user navigated away during async loading
+          const pathname = targetPath;
 
           if (pathname) {
             updateNavigationSpan(
-              currentActiveRootSpan,
+              activeRootSpan,
               { pathname, search: '', hash: '', state: null, key: 'default' },
               Array.from(allRoutes),
               false,

packages/react/src/reactrouter-compat-utils/lazy-routes.tsx

@@ -8,19 +8,28 @@ import { getActiveRootSpan, getNavigationContext } from './utils';
 /**
  * Captures location at invocation time. Prefers navigation context over window.location
  * since window.location hasn't updated yet when async handlers are invoked.
+ *
+ * When inside a patchRoutesOnNavigation call, uses the captured targetPath. If targetPath
+ * is undefined (patchRoutesOnNavigation can be invoked without a path argument), returns
+ * null rather than falling back to WINDOW.location which could be stale/wrong after the
+ * user navigated away during async loading. Returning null causes the span name update
+ * to be skipped, which is safer than using incorrect location data.
  */
 function captureCurrentLocation(): Location | null {
   const navContext = getNavigationContext();
-  // Only use navigation context if targetPath is defined (it can be undefined
-  // if patchRoutesOnNavigation was invoked without a path argument)
-  if (navContext?.targetPath) {
-    return {
-      pathname: navContext.targetPath,
-      search: '',
-      hash: '',
-      state: null,
-      key: 'default',
-    };
+
+  if (navContext) {
+    if (navContext.targetPath) {
+      return {
+        pathname: navContext.targetPath,
+        search: '',
+        hash: '',
+        state: null,
+        key: 'default',
+      };
+    }
+    // Don't fall back to potentially stale WINDOW.location
+    return null;
   }
 
   if (typeof WINDOW !== 'undefined') {

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

@@ -1309,4 +1309,56 @@ describe('tryUpdateSpanNameBeforeEnd - source upgrade logic', () => {
       }
     });
   });
+
+  describe('wrapPatchRoutesOnNavigation race condition fix', () => {
+    it('should use captured span instead of current active span in args.patch callback', () => {
+      const endedSpanJson = {
+        op: 'navigation',
+        timestamp: 1234567890, // Span has ended
+      };
+
+      vi.mocked(spanToJSON).mockReturnValue(endedSpanJson as any);
+
+      const endedSpan = {
+        updateName: vi.fn(),
+        setAttribute: vi.fn(),
+      } as unknown as Span;
+
+      updateNavigationSpan(
+        endedSpan,
+        { pathname: '/test', search: '', hash: '', state: null, key: 'test' },
+        [],
+        false,
+        vi.fn(() => []),
+      );
+
+      // eslint-disable-next-line @typescript-eslint/unbound-method
+      expect(endedSpan.updateName).not.toHaveBeenCalled();
+    });
+
+    it('should not fall back to WINDOW.location.pathname after async operations', () => {
+      const validSpanJson = {
+        op: 'navigation',
+        timestamp: undefined, // Span hasn't ended
+      };
+
+      vi.mocked(spanToJSON).mockReturnValue(validSpanJson as any);
+
+      const validSpan = {
+        updateName: vi.fn(),
+        setAttribute: vi.fn(),
+      } as unknown as Span;
+
+      updateNavigationSpan(
+        validSpan,
+        { pathname: '/captured/path', search: '', hash: '', state: null, key: 'test' },
+        [],
+        false,
+        vi.fn(() => []),
+      );
+
+      // eslint-disable-next-line @typescript-eslint/unbound-method
+      expect(validSpan.updateName).toHaveBeenCalled();
+    });
+  });
 });