Do not skip updating the previous span

Author: onurtemizkan

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

@@ -12,10 +12,8 @@ import type { Client, Integration, Span } from '@sentry/core';
 import {
   addNonEnumerableProperty,
   debug,
-  getActiveSpan,
   getClient,
   getCurrentScope,
-  getRootSpan,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
@@ -41,7 +39,14 @@ import type {
   UseRoutes,
 } from '../types';
 import { checkRouteForAsyncHandler } from './lazy-routes';
-import { initializeRouterUtils, resolveRouteNameAndSource, transactionNameHasWildcard } from './utils';
+import {
+  clearNavigationContext,
+  getActiveRootSpan,
+  initializeRouterUtils,
+  resolveRouteNameAndSource,
+  setNavigationContext,
+  transactionNameHasWildcard,
+} from './utils';
 
 let _useEffect: UseEffect;
 let _useLocation: UseLocation;
@@ -230,11 +235,14 @@ function trackLazyRouteLoad(span: Span, promise: Promise<unknown>): void {
 
 /**
  * Processes resolved routes by adding them to allRoutes and checking for nested async handlers.
+ * When capturedSpan is provided, updates that specific span instead of the current active span.
+ * This prevents race conditions where a lazy handler resolves after the user has navigated away.
  */
 export function processResolvedRoutes(
   resolvedRoutes: RouteObject[],
   parentRoute?: RouteObject,
   currentLocation: Location | null = null,
+  capturedSpan?: Span,
 ): void {
   resolvedRoutes.forEach(child => {
     allRoutes.add(child);
@@ -249,56 +257,44 @@ export function processResolvedRoutes(
     addResolvedRoutesToParent(resolvedRoutes, parentRoute);
   }
 
-  // After processing lazy routes, check if we need to update an active transaction
-  const activeRootSpan = getActiveRootSpan();
-  if (activeRootSpan) {
-    const spanOp = spanToJSON(activeRootSpan).op;
+  // Use captured span if provided, otherwise fall back to current active span
+  const targetSpan = capturedSpan ?? getActiveRootSpan();
+  if (targetSpan) {
+    const spanJson = spanToJSON(targetSpan);
+
+    // Skip update if span has already ended (timestamp is set when span.end() is called)
+    if (spanJson.timestamp) {
+      DEBUG_BUILD && debug.warn('[React Router] Lazy handler resolved after span ended - skipping update');
+      return;
+    }
+
+    const spanOp = spanJson.op;
 
-    // Try to use the provided location first, then fall back to global window location if needed
+    // Use captured location for route matching (ensures we match against the correct route)
+    // Fall back to window.location only if no captured location and no captured span
+    // (i.e., this is not from an async handler)
     let location = currentLocation;
-    if (!location) {
+    if (!location && !capturedSpan) {
       if (typeof WINDOW !== 'undefined') {
         const globalLocation = WINDOW.location;
-        if (globalLocation) {
+        if (globalLocation?.pathname) {
           location = { pathname: globalLocation.pathname };
         }
       }
     }
 
-    // If the user navigated away before this lazy handler resolved, skip updating the span.
-    if (currentLocation && typeof WINDOW !== 'undefined') {
-      try {
-        const windowLocation = WINDOW.location;
-        if (windowLocation) {
-          const capturedKey = computeLocationKey(currentLocation);
-          const currentKey = computeLocationKey({
-            pathname: windowLocation.pathname,
-            search: windowLocation.search || '',
-            hash: windowLocation.hash || '',
-          });
-
-          if (currentKey !== capturedKey) {
-            return;
-          }
-        }
-      } catch {
-        DEBUG_BUILD && debug.warn('[React Router] Could not access window.location');
-        return;
-      }
-    }
-
     if (location) {
       if (spanOp === 'pageload') {
         // Re-run the pageload transaction update with the newly loaded routes
         updatePageloadTransaction({
-          activeRootSpan,
+          activeRootSpan: targetSpan,
           location: { pathname: location.pathname },
           routes: Array.from(allRoutes),
           allRoutes: Array.from(allRoutes),
         });
       } else if (spanOp === 'navigation') {
         // For navigation spans, update the name with the newly loaded routes
-        updateNavigationSpan(activeRootSpan, location, Array.from(allRoutes), false, _matchRoutes);
+        updateNavigationSpan(targetSpan, location, Array.from(allRoutes), false, _matchRoutes);
       }
     }
   }
@@ -750,7 +746,14 @@ function wrapPatchRoutesOnNavigation(
       }
 
       const lazyLoadPromise = (async () => {
-        const result = await originalPatchRoutes(args);
+        // Set context so async handlers can access correct targetPath and span
+        setNavigationContext(targetPath, activeRootSpan);
+        let result;
+        try {
+          result = await originalPatchRoutes(args);
+        } finally {
+          clearNavigationContext();
+        }
 
         const currentActiveRootSpan = getActiveRootSpan();
         if (currentActiveRootSpan && (spanToJSON(currentActiveRootSpan) as { op?: string }).op === 'navigation') {
@@ -1206,17 +1209,3 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
   // will break advanced type inference done by react router params
   return SentryRoutes;
 }
-
-function getActiveRootSpan(): Span | undefined {
-  const span = getActiveSpan();
-  const rootSpan = span ? getRootSpan(span) : undefined;
-
-  if (!rootSpan) {
-    return undefined;
-  }
-
-  const op = spanToJSON(rootSpan).op;
-
-  // Only use this root span if it is a pageload or navigation span
-  return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
-}

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

@@ -1,12 +1,26 @@
 import { WINDOW } from '@sentry/browser';
+import type { Span } from '@sentry/core';
 import { addNonEnumerableProperty, debug, isThenable } from '@sentry/core';
 import { DEBUG_BUILD } from '../debug-build';
 import type { Location, RouteObject } from '../types';
+import { getActiveRootSpan, getNavigationContext } from './utils';
 
 /**
- * Captures the current location at the time of invocation.
+ * Captures location at invocation time. Prefers navigation context over window.location
+ * since window.location hasn't updated yet when async handlers are invoked.
  */
 function captureCurrentLocation(): Location | null {
+  const navContext = getNavigationContext();
+  if (navContext) {
+    return {
+      pathname: navContext.targetPath,
+      search: '',
+      hash: '',
+      state: null,
+      key: 'default',
+    };
+  }
+
   if (typeof WINDOW !== 'undefined') {
     try {
       const windowLocation = WINDOW.location;
@@ -26,20 +40,47 @@ function captureCurrentLocation(): Location | null {
   return null;
 }
 
+/**
+ * Captures the active span at invocation time. Prefers navigation context span
+ * to ensure we update the correct span even if another navigation starts.
+ */
+function captureActiveSpan(): Span | undefined {
+  const navContext = getNavigationContext();
+  if (navContext) {
+    return navContext.span;
+  }
+  return getActiveRootSpan();
+}
+
 /**
  * Creates a proxy wrapper for an async handler function.
+ * Captures both the location and the active span at invocation time to ensure
+ * the correct span is updated when the handler resolves.
  */
 export function createAsyncHandlerProxy(
   originalFunction: (...args: unknown[]) => unknown,
   route: RouteObject,
   handlerKey: string,
-  processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void,
+  processResolvedRoutes: (
+    resolvedRoutes: RouteObject[],
+    parentRoute?: RouteObject,
+    currentLocation?: Location,
+    capturedSpan?: Span,
+  ) => void,
 ): (...args: unknown[]) => unknown {
   const proxy = new Proxy(originalFunction, {
     apply(target: (...args: unknown[]) => unknown, thisArg, argArray) {
       const locationAtInvocation = captureCurrentLocation();
+      const spanAtInvocation = captureActiveSpan();
       const result = target.apply(thisArg, argArray);
-      handleAsyncHandlerResult(result, route, handlerKey, processResolvedRoutes, locationAtInvocation);
+      handleAsyncHandlerResult(
+        result,
+        route,
+        handlerKey,
+        processResolvedRoutes,
+        locationAtInvocation,
+        spanAtInvocation,
+      );
       return result;
     },
   });
@@ -51,26 +92,33 @@ export function createAsyncHandlerProxy(
 
 /**
  * Handles the result of an async handler function call.
+ * Passes the captured span through to ensure the correct span is updated.
  */
 export function handleAsyncHandlerResult(
   result: unknown,
   route: RouteObject,
   handlerKey: string,
-  processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void,
+  processResolvedRoutes: (
+    resolvedRoutes: RouteObject[],
+    parentRoute?: RouteObject,
+    currentLocation?: Location,
+    capturedSpan?: Span,
+  ) => void,
   currentLocation: Location | null,
+  capturedSpan: Span | undefined,
 ): void {
   if (isThenable(result)) {
     (result as Promise<unknown>)
       .then((resolvedRoutes: unknown) => {
         if (Array.isArray(resolvedRoutes)) {
-          processResolvedRoutes(resolvedRoutes, route, currentLocation ?? undefined);
+          processResolvedRoutes(resolvedRoutes, route, currentLocation ?? undefined, capturedSpan);
         }
       })
       .catch((e: unknown) => {
         DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e);
       });
   } else if (Array.isArray(result)) {
-    processResolvedRoutes(result, route, currentLocation ?? undefined);
+    processResolvedRoutes(result, route, currentLocation ?? undefined, capturedSpan);
   }
 }
 
@@ -79,7 +127,12 @@ export function handleAsyncHandlerResult(
  */
 export function checkRouteForAsyncHandler(
   route: RouteObject,
-  processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void,
+  processResolvedRoutes: (
+    resolvedRoutes: RouteObject[],
+    parentRoute?: RouteObject,
+    currentLocation?: Location,
+    capturedSpan?: Span,
+  ) => void,
 ): void {
   // Set up proxies for any functions in the route's handle
   if (route.handle && typeof route.handle === 'object') {

packages/react/src/reactrouter-compat-utils/utils.ts

@@ -1,10 +1,37 @@
-import type { TransactionSource } from '@sentry/core';
+import type { Span, TransactionSource } from '@sentry/core';
+import { getActiveSpan, getRootSpan, spanToJSON } from '@sentry/core';
 import type { Location, MatchRoutes, RouteMatch, RouteObject } from '../types';
 
 // Global variables that these utilities depend on
 let _matchRoutes: MatchRoutes;
 let _stripBasename: boolean = false;
 
+// Navigation context stack for nested/concurrent patchRoutesOnNavigation calls.
+// Required because window.location hasn't updated yet when handlers are invoked.
+// Uses a stack to handle overlapping navigations correctly (LIFO semantics).
+interface NavigationContext {
+  targetPath: string;
+  span: Span | undefined;
+}
+
+const _navigationContextStack: NavigationContext[] = [];
+
+/** Pushes a navigation context before invoking patchRoutesOnNavigation. */
+export function setNavigationContext(targetPath: string, span: Span | undefined): void {
+  _navigationContextStack.push({ targetPath, span });
+}
+
+/** Pops the most recent navigation context after patchRoutesOnNavigation completes. */
+export function clearNavigationContext(): void {
+  _navigationContextStack.pop();
+}
+
+/** Gets the current (most recent) navigation context if inside a patchRoutesOnNavigation call. */
+export function getNavigationContext(): NavigationContext | null {
+  const length = _navigationContextStack.length;
+  return length > 0 ? (_navigationContextStack[length - 1] ?? null) : null;
+}
+
 /**
  * Initialize function to set dependencies that the router utilities need.
  * Must be called before using any of the exported utility functions.
@@ -273,3 +300,20 @@ export function resolveRouteNameAndSource(
 
   return [name || location.pathname, source];
 }
+
+/**
+ * Gets the active root span if it's a pageload or navigation span.
+ */
+export function getActiveRootSpan(): Span | undefined {
+  const span = getActiveSpan();
+  const rootSpan = span ? getRootSpan(span) : undefined;
+
+  if (!rootSpan) {
+    return undefined;
+  }
+
+  const op = spanToJSON(rootSpan).op;
+
+  // Only use this root span if it is a pageload or navigation span
+  return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
+}

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

@@ -59,6 +59,7 @@ vi.mock('../../src/reactrouter-compat-utils/utils', () => ({
   transactionNameHasWildcard: vi.fn((name: string) => {
     return name.includes('/*') || name === '*' || name.endsWith('*');
   }),
+  getActiveRootSpan: vi.fn(() => undefined),
 }));
 
 vi.mock('../../src/reactrouter-compat-utils/lazy-routes', () => ({