Skip to content

Commit 9ce440c

Browse files
authored
fix(react): Prevent navigation span leaks for consecutive navigations (#18098)
Fixes an issue where consecutive navigations to different routes fail to create separate navigation spans, causing span leaks and missing transaction data. This came up in a React Router v6/v7 application where the pageload / navigation transactions take longer and there is a high `finalTimeout` set in config. When users navigate between different routes (e.g., `/users/:id` → `/projects/:projectId` → `/settings`). The SDK was incorrectly preventing new navigation spans from being created whenever an ongoing navigation span was active, regardless of whether the navigation was to a different route. This resulted in only the first navigation being tracked, with subsequent navigations being silently ignored. Also, the spans that needed to be a part of the subsequent navigation were recorded as a part of the previous one. The root cause was the `if (!isAlreadyInNavigationSpan)` check that we used to prevent cross-usage scenarios (multiple wrappers instrumenting the same navigation), which incorrectly blocked legitimate consecutive navigations to different routes. So, this fix changes the logic to check both navigation span state and the route name: `isSpanForSameRoute = isAlreadyInNavigationSpan && spanJson?.description === name`. This allows consecutive navigations to different routes while preventing duplicate spans for the same route. Also added tracking using `LAST_NAVIGATION_PER_CLIENT`. When multiple wrappers (e.g., `wrapCreateBrowserRouter` + `wrapUseRoutes`) instrument the same application, they may each trigger span creation for the same navigation event. We store the navigation key `${location.pathname}${location.search}${location.hash}` while the span is active and clear it when that span ends. If the same navigation key shows up again before the original span finishes, the second wrapper updates that span’s name if it has better parameterization instead of creating a duplicate, which keeps cross-usage covered.
1 parent ccfda39 commit 9ce440c

File tree

6 files changed

+538
-185
lines changed

6 files changed

+538
-185
lines changed

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

Lines changed: 55 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ test('Creates a navigation transaction inside a lazy route', async ({ page }) =>
8787
});
8888

8989
test('Creates navigation transactions between two different lazy routes', async ({ page }) => {
90-
// First, navigate to the "another-lazy" route
90+
// Set up transaction listeners for both navigations
9191
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
9292
return (
9393
!!transactionEvent?.transaction &&
@@ -96,6 +96,14 @@ test('Creates navigation transactions between two different lazy routes', async
9696
);
9797
});
9898

99+
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
100+
return (
101+
!!transactionEvent?.transaction &&
102+
transactionEvent.contexts?.trace?.op === 'navigation' &&
103+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
104+
);
105+
});
106+
99107
await page.goto('/');
100108

101109
// Navigate to another lazy route first
@@ -115,14 +123,6 @@ test('Creates navigation transactions between two different lazy routes', async
115123
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
116124

117125
// Now navigate from the first lazy route to the second lazy route
118-
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
119-
return (
120-
!!transactionEvent?.transaction &&
121-
transactionEvent.contexts?.trace?.op === 'navigation' &&
122-
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
123-
);
124-
});
125-
126126
// Click the navigation link from within the first lazy route to the second lazy route
127127
const navigationToInnerFromDeep = page.locator('id=navigate-to-inner-from-deep');
128128
await expect(navigationToInnerFromDeep).toBeVisible();
@@ -255,7 +255,7 @@ test('Does not send any duplicate navigation transaction names browsing between
255255

256256
// Go to root page
257257
await page.goto('/');
258-
page.waitForTimeout(1000);
258+
await page.waitForTimeout(1000);
259259

260260
// Navigate to inner lazy route
261261
const navigationToInner = page.locator('id=navigation');
@@ -339,6 +339,7 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
339339
const navigationToLongRunning = page.locator('id=navigation-to-long-running');
340340
await expect(navigationToLongRunning).toBeVisible();
341341

342+
// Set up transaction listeners for both navigations
342343
const firstNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
343344
return (
344345
!!transactionEvent?.transaction &&
@@ -347,6 +348,14 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
347348
);
348349
});
349350

351+
const backNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
352+
return (
353+
!!transactionEvent?.transaction &&
354+
transactionEvent.contexts?.trace?.op === 'navigation' &&
355+
transactionEvent.transaction === '/'
356+
);
357+
});
358+
350359
await navigationToLongRunning.click();
351360

352361
const slowLoadingContent = page.locator('id=slow-loading-content');
@@ -359,14 +368,6 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
359368

360369
// Now navigate back using browser back button (POP event)
361370
// This should create a navigation transaction since pageload is complete
362-
const backNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
363-
return (
364-
!!transactionEvent?.transaction &&
365-
transactionEvent.contexts?.trace?.op === 'navigation' &&
366-
transactionEvent.transaction === '/'
367-
);
368-
});
369-
370371
await page.goBack();
371372

372373
// Verify we're back at home
@@ -504,7 +505,7 @@ test('Updates navigation transaction name correctly when span is cancelled early
504505
test('Creates separate transactions for rapid consecutive navigations', async ({ page }) => {
505506
await page.goto('/');
506507

507-
// First navigation: / -> /lazy/inner/:id/:anotherId/:someAnotherId
508+
// Set up transaction listeners
508509
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
509510
return (
510511
!!transactionEvent?.transaction &&
@@ -513,20 +514,6 @@ test('Creates separate transactions for rapid consecutive navigations', async ({
513514
);
514515
});
515516

516-
const navigationToInner = page.locator('id=navigation');
517-
await expect(navigationToInner).toBeVisible();
518-
await navigationToInner.click();
519-
520-
const firstEvent = await firstTransactionPromise;
521-
522-
// Verify first transaction
523-
expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
524-
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
525-
expect(firstEvent.contexts?.trace?.status).toBe('ok');
526-
const firstTraceId = firstEvent.contexts?.trace?.trace_id;
527-
const firstSpanId = firstEvent.contexts?.trace?.span_id;
528-
529-
// Second navigation: /lazy/inner -> /another-lazy/sub/:id/:subId
530517
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
531518
return (
532519
!!transactionEvent?.transaction &&
@@ -535,40 +522,54 @@ test('Creates separate transactions for rapid consecutive navigations', async ({
535522
);
536523
});
537524

538-
const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
539-
await expect(navigationToAnother).toBeVisible();
540-
await navigationToAnother.click();
525+
// Third navigation promise - using counter to match second occurrence of same route
526+
let innerRouteMatchCount = 0;
527+
const thirdTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
528+
if (
529+
transactionEvent?.transaction &&
530+
transactionEvent.contexts?.trace?.op === 'navigation' &&
531+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
532+
) {
533+
innerRouteMatchCount++;
534+
return innerRouteMatchCount === 2; // Match the second occurrence
535+
}
536+
return false;
537+
});
538+
539+
// Perform navigations
540+
// First navigation: / -> /lazy/inner/:id/:anotherId/:someAnotherId
541+
await page.locator('id=navigation').click();
542+
543+
const firstEvent = await firstTransactionPromise;
544+
545+
// Second navigation: /lazy/inner -> /another-lazy/sub/:id/:subId
546+
await page.locator('id=navigate-to-another-from-inner').click();
541547

542548
const secondEvent = await secondTransactionPromise;
543549

544-
// Verify second transaction
550+
// Third navigation: /another-lazy -> /lazy/inner/:id/:anotherId/:someAnotherId (back to same route as first)
551+
await page.locator('id=navigate-to-inner-from-deep').click();
552+
553+
const thirdEvent = await thirdTransactionPromise;
554+
555+
// Verify transactions
556+
expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
557+
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
558+
const firstTraceId = firstEvent.contexts?.trace?.trace_id;
559+
const firstSpanId = firstEvent.contexts?.trace?.span_id;
560+
545561
expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
546562
expect(secondEvent.contexts?.trace?.op).toBe('navigation');
547563
expect(secondEvent.contexts?.trace?.status).toBe('ok');
564+
548565
const secondTraceId = secondEvent.contexts?.trace?.trace_id;
549566
const secondSpanId = secondEvent.contexts?.trace?.span_id;
550567

551-
// Third navigation: /another-lazy -> /lazy/inner/:id/:anotherId/:someAnotherId (back to same route as first)
552-
const thirdTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
553-
return (
554-
!!transactionEvent?.transaction &&
555-
transactionEvent.contexts?.trace?.op === 'navigation' &&
556-
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' &&
557-
// Ensure we're not matching the first transaction again
558-
transactionEvent.contexts?.trace?.trace_id !== firstTraceId
559-
);
560-
});
561-
562-
const navigationBackToInner = page.locator('id=navigate-to-inner-from-deep');
563-
await expect(navigationBackToInner).toBeVisible();
564-
await navigationBackToInner.click();
565-
566-
const thirdEvent = await thirdTransactionPromise;
567-
568568
// Verify third transaction
569569
expect(thirdEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
570570
expect(thirdEvent.contexts?.trace?.op).toBe('navigation');
571571
expect(thirdEvent.contexts?.trace?.status).toBe('ok');
572+
572573
const thirdTraceId = thirdEvent.contexts?.trace?.trace_id;
573574
const thirdSpanId = thirdEvent.contexts?.trace?.span_id;
574575

0 commit comments

Comments
 (0)