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

onurtemizkan · onurtemizkan · commit d720cad1b8dc · 2025-11-05T12:32:53.000Z
diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts
@@ -498,3 +498,87 @@ test('Updates navigation transaction name correctly when span is cancelled early
     expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason);
   }
 });
+
+test('Creates separate transactions for rapid consecutive navigations', async ({ page }) => {
+  await page.goto('/');
+
+  // First navigation: / -> /lazy/inner/:id/:anotherId/:someAnotherId
+  const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
+    );
+  });
+
+  const navigationToInner = page.locator('id=navigation');
+  await expect(navigationToInner).toBeVisible();
+  await navigationToInner.click();
+
+  const firstEvent = await firstTransactionPromise;
+
+  // Verify first transaction
+  expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
+  expect(firstEvent.contexts?.trace?.op).toBe('navigation');
+  const firstTraceId = firstEvent.contexts?.trace?.trace_id;
+  const firstSpanId = firstEvent.contexts?.trace?.span_id;
+
+  // Second navigation: /lazy/inner -> /another-lazy/sub/:id/:subId
+  const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/another-lazy/sub/:id/:subId'
+    );
+  });
+
+  const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
+  await expect(navigationToAnother).toBeVisible();
+  await navigationToAnother.click();
+
+  const secondEvent = await secondTransactionPromise;
+
+  // Verify second transaction
+  expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
+  expect(secondEvent.contexts?.trace?.op).toBe('navigation');
+  const secondTraceId = secondEvent.contexts?.trace?.trace_id;
+  const secondSpanId = secondEvent.contexts?.trace?.span_id;
+
+  // Third navigation: /another-lazy -> /lazy/inner/:id/:anotherId/:someAnotherId (back to same route as first)
+  const thirdTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' &&
+      // Ensure we're not matching the first transaction again
+      transactionEvent.contexts?.trace?.trace_id !== firstTraceId
+    );
+  });
+
+  const navigationBackToInner = page.locator('id=navigate-to-inner-from-deep');
+  await expect(navigationBackToInner).toBeVisible();
+  await navigationBackToInner.click();
+
+  const thirdEvent = await thirdTransactionPromise;
+
+  // Verify third transaction
+  expect(thirdEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
+  expect(thirdEvent.contexts?.trace?.op).toBe('navigation');
+  const thirdTraceId = thirdEvent.contexts?.trace?.trace_id;
+  const thirdSpanId = thirdEvent.contexts?.trace?.span_id;
+
+  // Verify each navigation created a separate transaction with unique trace and span IDs
+  expect(firstTraceId).toBeDefined();
+  expect(secondTraceId).toBeDefined();
+  expect(thirdTraceId).toBeDefined();
+
+  // All trace IDs should be unique
+  expect(firstTraceId).not.toBe(secondTraceId);
+  expect(secondTraceId).not.toBe(thirdTraceId);
+  expect(firstTraceId).not.toBe(thirdTraceId);
+
+  // All span IDs should be unique
+  expect(firstSpanId).not.toBe(secondSpanId);
+  expect(secondSpanId).not.toBe(thirdSpanId);
+  expect(firstSpanId).not.toBe(thirdSpanId);
+});
diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx
@@ -659,8 +659,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: {
diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/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',
+        }),
+      });
+    });
+  });
 });
