fix(react): Prevent navigation span leaks for consecutive navigations

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

@@ -523,6 +523,7 @@ test('Creates separate transactions for rapid consecutive navigations', async ({
   expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
   expect(firstEvent.contexts?.trace?.op).toBe('navigation');
   expect(firstEvent.contexts?.trace?.status).toBe('ok');
+
   const firstTraceId = firstEvent.contexts?.trace?.trace_id;
   const firstSpanId = firstEvent.contexts?.trace?.span_id;
 
@@ -545,6 +546,7 @@ test('Creates separate transactions for rapid consecutive navigations', async ({
   expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
   expect(secondEvent.contexts?.trace?.op).toBe('navigation');
   expect(secondEvent.contexts?.trace?.status).toBe('ok');
+
   const secondTraceId = secondEvent.contexts?.trace?.trace_id;
   const secondSpanId = secondEvent.contexts?.trace?.span_id;
 
@@ -569,6 +571,7 @@ test('Creates separate transactions for rapid consecutive navigations', async ({
   expect(thirdEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
   expect(thirdEvent.contexts?.trace?.op).toBe('navigation');
   expect(thirdEvent.contexts?.trace?.status).toBe('ok');
+
   const thirdTraceId = thirdEvent.contexts?.trace?.trace_id;
   const thirdSpanId = thirdEvent.contexts?.trace?.span_id;
 

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

@@ -279,7 +279,10 @@ export function createV6CompatibleWrapCreateBrowserRouter<
         state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete);
 
       if (shouldHandleNavigation) {
-        const navigationHandler = (): void => {
+        // Only handle navigation when it's complete (state is idle).
+        // During 'loading' or 'submitting', state.location may still have the old pathname,
+        // which would cause us to create a span for the wrong route.
+        if (state.navigation.state === 'idle') {
           handleNavigation({
             location: state.location,
             routes,
@@ -288,13 +291,6 @@ export function createV6CompatibleWrapCreateBrowserRouter<
             basename,
             allRoutes: Array.from(allRoutes),
           });
-        };
-
-        // Wait for the next render if loading an unsettled route
-        if (state.navigation.state !== 'idle') {
-          requestAnimationFrame(navigationHandler);
-        } else {
-          navigationHandler();
         }
       }
     });
@@ -632,7 +628,8 @@ export function handleNavigation(opts: {
   allRoutes?: RouteObject[];
 }): void {
   const { location, routes, navigationType, version, matches, basename, allRoutes } = opts;
-  const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location, basename);
+  // Use allRoutes for matching to include lazy-loaded routes
+  const branches = Array.isArray(matches) ? matches : _matchRoutes(allRoutes || routes, location, basename);
 
   const client = getClient();
   if (!client || !CLIENTS_WITH_INSTRUMENT_NAVIGATION.has(client)) {
@@ -649,7 +646,7 @@ export function handleNavigation(opts: {
   if ((navigationType === 'PUSH' || navigationType === 'POP') && branches) {
     const [name, source] = resolveRouteNameAndSource(
       location,
-      routes,
+      allRoutes || routes,
       allRoutes || routes,
       branches as RouteMatch[],
       basename,
@@ -659,8 +656,11 @@ export function handleNavigation(opts: {
     const spanJson = activeSpan && spanToJSON(activeSpan);
     const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';
 
-    // Cross usage can result in multiple navigation spans being created without this check
-    if (!isAlreadyInNavigationSpan) {
+    // Only skip creating a new span if we're already in a navigation span AND the route name matches.
+    // This handles cross-usage (multiple wrappers for same navigation) while allowing consecutive navigations.
+    const isSpanForSameRoute = isAlreadyInNavigationSpan && spanJson?.description === name;
+
+    if (!isSpanForSameRoute) {
       const navigationSpan = startBrowserTracingNavigationSpan(client, {
         name,
         attributes: {
@@ -727,7 +727,13 @@ function updatePageloadTransaction({
     : (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]);
 
   if (branches) {
-    const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename);
+    const [name, source] = resolveRouteNameAndSource(
+      location,
+      allRoutes || routes,
+      allRoutes || routes,
+      branches,
+      basename,
+    );
 
     getCurrentScope().setTransactionName(name || '/');
 
@@ -780,7 +786,7 @@ function patchSpanEnd(
         if (branches) {
           const [name, source] = resolveRouteNameAndSource(
             location,
-            routes,
+            currentAllRoutes.length > 0 ? currentAllRoutes : routes,
             currentAllRoutes.length > 0 ? currentAllRoutes : routes,
             branches,
             basename,

packages/react/test/reactrouter-cross-usage.test.tsx

@@ -9,7 +9,7 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   setCurrentClient,
 } from '@sentry/core';
-import { render } from '@testing-library/react';
+import { act, render, waitFor } from '@testing-library/react';
 import * as React from 'react';
 import {
   createMemoryRouter,
@@ -607,4 +607,276 @@ describe('React Router cross usage of wrappers', () => {
       });
     });
   });
+
+  describe('consecutive navigations to different routes', () => {
+    it('should create separate transactions for consecutive navigations to different routes', async () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+
+      const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter);
+
+      const router = createSentryMemoryRouter(
+        [
+          {
+            children: [
+              { path: '/users', element: <div>Users</div> },
+              { path: '/settings', element: <div>Settings</div> },
+              { path: '/profile', element: <div>Profile</div> },
+            ],
+          },
+        ],
+        { initialEntries: ['/users'] },
+      );
+
+      render(
+        <React.StrictMode>
+          <RouterProvider router={router} />
+        </React.StrictMode>,
+      );
+
+      expect(mockStartBrowserTracingNavigationSpan).not.toHaveBeenCalled();
+
+      await act(async () => {
+        router.navigate('/settings');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
+        name: '/settings',
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
+        },
+      });
+
+      await act(async () => {
+        router.navigate('/profile');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
+
+      const calls = mockStartBrowserTracingNavigationSpan.mock.calls;
+      expect(calls[0]![1].name).toBe('/settings');
+      expect(calls[1]![1].name).toBe('/profile');
+      expect(calls[0]![1].attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('navigation');
+      expect(calls[1]![1].attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('navigation');
+    });
+
+    it('should create separate transactions for rapid consecutive navigations', async () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+
+      const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter);
+
+      const router = createSentryMemoryRouter(
+        [
+          {
+            children: [
+              { path: '/a', element: <div>A</div> },
+              { path: '/b', element: <div>B</div> },
+              { path: '/c', element: <div>C</div> },
+            ],
+          },
+        ],
+        { initialEntries: ['/a'] },
+      );
+
+      render(
+        <React.StrictMode>
+          <RouterProvider router={router} />
+        </React.StrictMode>,
+      );
+
+      await act(async () => {
+        router.navigate('/b');
+        router.navigate('/c');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
+
+      const calls = mockStartBrowserTracingNavigationSpan.mock.calls;
+      expect(calls[0]![1].name).toBe('/b');
+      expect(calls[1]![1].name).toBe('/c');
+    });
+
+    it('should NOT create duplicate spans for same route name (even with different params)', async () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+
+      const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter);
+
+      const router = createSentryMemoryRouter(
+        [
+          {
+            children: [{ path: '/user/:id', element: <div>User</div> }],
+          },
+        ],
+        { initialEntries: ['/user/1'] },
+      );
+
+      render(
+        <React.StrictMode>
+          <RouterProvider router={router} />
+        </React.StrictMode>,
+      );
+
+      await act(async () => {
+        router.navigate('/user/2');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledWith(expect.any(BrowserClient), {
+        name: '/user/:id',
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
+        },
+      });
+
+      await act(async () => {
+        router.navigate('/user/3');
+        await new Promise(resolve => setTimeout(resolve, 100));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
+    });
+
+    it('should handle mixed cross-usage and consecutive navigations correctly', async () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+
+      const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter);
+      const sentryUseRoutes = wrapUseRoutesV6(useRoutes);
+
+      const UsersRoute: React.FC = () => sentryUseRoutes([{ path: '/', element: <div>Users</div> }]);
+
+      const SettingsRoute: React.FC = () => sentryUseRoutes([{ path: '/', element: <div>Settings</div> }]);
+
+      const router = createSentryMemoryRouter(
+        [
+          {
+            children: [
+              { path: '/users/*', element: <UsersRoute /> },
+              { path: '/settings/*', element: <SettingsRoute /> },
+            ],
+          },
+        ],
+        { initialEntries: ['/users'] },
+      );
+
+      render(
+        <React.StrictMode>
+          <RouterProvider router={router} />
+        </React.StrictMode>,
+      );
+
+      expect(mockStartBrowserTracingNavigationSpan).not.toHaveBeenCalled();
+
+      await act(async () => {
+        router.navigate('/settings');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1));
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
+        name: '/settings/*',
+        attributes: expect.objectContaining({
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+        }),
+      });
+    });
+
+    it('should not create duplicate spans for cross-usage on same route', async () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+
+      const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter);
+      const sentryUseRoutes = wrapUseRoutesV6(useRoutes);
+
+      const NestedRoute: React.FC = () => sentryUseRoutes([{ path: '/', element: <div>Details</div> }]);
+
+      const router = createSentryMemoryRouter(
+        [
+          {
+            children: [{ path: '/details/*', element: <NestedRoute /> }],
+          },
+        ],
+        { initialEntries: ['/home'] },
+      );
+
+      render(
+        <React.StrictMode>
+          <RouterProvider router={router} />
+        </React.StrictMode>,
+      );
+
+      await act(async () => {
+        router.navigate('/details');
+        await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalled());
+      });
+
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
+      expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledWith(expect.any(BrowserClient), {
+        name: '/details/*',
+        attributes: expect.objectContaining({
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+        }),
+      });
+    });
+  });
 });