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

Following up on #18346

Fixes a remaining race condition where the `args.patch` callback and
completion handler in `wrapPatchRoutesOnNavigation` were still calling
`getActiveRootSpan()` at resolution time instead of using the captured
span. This caused wrong span updates when users navigated away during
lazy route loading.

The fix uses the captured `activeRootSpan` consistently, adds a
`!spanJson.timestamp` check to skip updates on ended spans, and removes
the `WINDOW.location` fallback. In `captureCurrentLocation`, returns
`null` when navigation context exists but `targetPath` is `undefined`,
instead of falling back to stale `WINDOW.location`.

Also added E2E tests that simulate rapid navigation scenarios where a
slow lazy route handler completes after the user navigated elsewhere.

Author: onurtemizkan

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

@@ -1308,3 +1308,128 @@ test('Slow lazy route navigation with early span end still gets parameterized ro
   expect(event.contexts?.trace?.op).toBe('navigation');
   expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route');
 });
+
+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

@@ -772,33 +772,6 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
     return <SentryRoutes routes={routes} locationArg={locationArg} />;
   };
 }
-
-/**
- * Helper to update the current span (navigation or pageload) with lazy-loaded route information.
- * Reduces code duplication in patchRoutesOnNavigation wrapper.
- */
-function updateSpanWithLazyRoutes(pathname: string, forceUpdate: boolean): void {
-  const currentActiveRootSpan = getActiveRootSpan();
-  if (!currentActiveRootSpan) {
-    return;
-  }
-
-  const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op;
-  const location = { pathname, search: '', hash: '', state: null, key: 'default' };
-  const routesArray = Array.from(allRoutes);
-
-  if (spanOp === 'navigation') {
-    updateNavigationSpan(currentActiveRootSpan, location, routesArray, forceUpdate, _matchRoutes);
-  } else if (spanOp === 'pageload') {
-    updatePageloadTransaction({
-      activeRootSpan: currentActiveRootSpan,
-      location,
-      routes: routesArray,
-      allRoutes: routesArray,
-    });
-  }
-}
-
 function wrapPatchRoutesOnNavigation(
   opts: Record<string, unknown> | undefined,
   isMemoryRouter = false,
@@ -853,9 +826,25 @@ function wrapPatchRoutesOnNavigation(
               }
             }
 
-            // Only update if we have a valid targetPath (patchRoutesOnNavigation can be called without path)
-            if (targetPath) {
-              updateSpanWithLazyRoutes(targetPath, true);
+            // 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 &&
+              activeRootSpan &&
+              spanJson &&
+              !spanJson.timestamp && // Span hasn't ended yet
+              spanJson.op === 'navigation'
+            ) {
+              updateNavigationSpan(
+                activeRootSpan,
+                { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
+                Array.from(allRoutes),
+                true,
+                _matchRoutes,
+              );
             }
             return originalPatch(routeId, children);
           };
@@ -877,9 +866,28 @@ function wrapPatchRoutesOnNavigation(
           }
         }
 
-        const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname;
-        if (pathname) {
-          updateSpanWithLazyRoutes(pathname, false);
+        // 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(
+              activeRootSpan,
+              { pathname, search: '', hash: '', state: null, key: 'default' },
+              Array.from(allRoutes),
+              false,
+              _matchRoutes,
+            );
+          }
         }
 
         return result;

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

@@ -1397,4 +1397,56 @@ describe('tryUpdateSpanNameBeforeEnd - source upgrade logic', () => {
       expect(allRoutesArray).toContain(lazyLoadedRoutes[0]);
     });
   });
+
+  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();
+    });
+  });
 });