Skip to content

Commit cd5ad7e

Browse files
onurtemizkans1gr1d
andauthored
ref(react): Add more guarding against wildcards in lazy route transactions (#18155)
Building on top of #17962 Added a few more checks to make sure non-resolved (wildcard) routes are not reported in lazy route pageloads / navigations. - Improved `patchSpanEnd` with a user-configurable wait timeout for potentially slow route resolution. Named this option as `lazyRouteTimeout` and it's defaulted as `idleTimeout` * 3. It may conditionally delay reporting (if the route resolution is still not done by the end of the timeout), but will prevent prematurely sent lazy-route transactions inside that window. - Added extra checks on `updateNavigationSpan` and `handleNavigation` for whether any wildcard still exists in a lazy-route, so they are still marked as open to full resolution. We keep track of pending lazy-route resolutions inside `pendingLazyRouteLoads` - Added a final attempt to update the transaction name with fully-resolved route when the pending resolution is done. Any of these should not affect the behaviour of non-lazy route usage --------- Co-authored-by: Sigrid <32902192+s1gr1d@users.noreply.github.com>
1 parent 5e7cd06 commit cd5ad7e

File tree

14 files changed

+2114
-312
lines changed

14 files changed

+2114
-312
lines changed

dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as Sentry from '@sentry/react';
22
import React from 'react';
33
import ReactDOM from 'react-dom/client';
44
import {
5-
Navigate,
65
PatchRoutesOnNavigationFunction,
76
RouterProvider,
87
createBrowserRouter,
@@ -12,6 +11,49 @@ import {
1211
useNavigationType,
1312
} from 'react-router-dom';
1413
import Index from './pages/Index';
14+
import Deep from './pages/Deep';
15+
16+
function getRuntimeConfig(): { lazyRouteTimeout?: number; idleTimeout?: number } {
17+
if (typeof window === 'undefined') {
18+
return {};
19+
}
20+
21+
try {
22+
const url = new URL(window.location.href);
23+
const timeoutParam = url.searchParams.get('timeout');
24+
const idleTimeoutParam = url.searchParams.get('idleTimeout');
25+
26+
let lazyRouteTimeout: number | undefined = undefined;
27+
if (timeoutParam) {
28+
if (timeoutParam === 'Infinity') {
29+
lazyRouteTimeout = Infinity;
30+
} else {
31+
const parsed = parseInt(timeoutParam, 10);
32+
if (!isNaN(parsed)) {
33+
lazyRouteTimeout = parsed;
34+
}
35+
}
36+
}
37+
38+
let idleTimeout: number | undefined = undefined;
39+
if (idleTimeoutParam) {
40+
const parsed = parseInt(idleTimeoutParam, 10);
41+
if (!isNaN(parsed)) {
42+
idleTimeout = parsed;
43+
}
44+
}
45+
46+
return {
47+
lazyRouteTimeout,
48+
idleTimeout,
49+
};
50+
} catch (error) {
51+
console.warn('Failed to read runtime config, falling back to defaults', error);
52+
return {};
53+
}
54+
}
55+
56+
const runtimeConfig = getRuntimeConfig();
1557

1658
Sentry.init({
1759
environment: 'qa', // dynamic sampling bias to keep transactions
@@ -25,6 +67,8 @@ Sentry.init({
2567
matchRoutes,
2668
trackFetchStreamPerformance: true,
2769
enableAsyncRouteHandlers: true,
70+
lazyRouteTimeout: runtimeConfig.lazyRouteTimeout,
71+
idleTimeout: runtimeConfig.idleTimeout,
2872
}),
2973
],
3074
// We recommend adjusting this value in production, or using tracesSampler
@@ -66,8 +110,21 @@ const router = sentryCreateBrowserRouter(
66110
element: <>Hello World</>,
67111
},
68112
{
69-
path: '*',
70-
element: <Navigate to="/" replace />,
113+
path: '/delayed-lazy/:id',
114+
lazy: async () => {
115+
// Simulate slow lazy route loading (400ms delay)
116+
await new Promise(resolve => setTimeout(resolve, 400));
117+
return {
118+
Component: (await import('./pages/DelayedLazyRoute')).default,
119+
};
120+
},
121+
},
122+
{
123+
path: '/deep',
124+
element: <Deep />,
125+
handle: {
126+
lazyChildren: () => import('./pages/deep/Level1Routes').then(module => module.level2Routes),
127+
},
71128
},
72129
],
73130
{
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import React from 'react';
2+
import { Outlet } from 'react-router-dom';
3+
4+
export default function Deep() {
5+
return (
6+
<div>
7+
<h1>Deep Route Root</h1>
8+
<p id="deep-root">You are at the deep route root</p>
9+
<Outlet />
10+
</div>
11+
);
12+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import React from 'react';
2+
import { Link, useParams, useLocation, useSearchParams } from 'react-router-dom';
3+
4+
const DelayedLazyRoute = () => {
5+
const { id } = useParams<{ id: string }>();
6+
const location = useLocation();
7+
const [searchParams] = useSearchParams();
8+
const view = searchParams.get('view') || 'none';
9+
const source = searchParams.get('source') || 'none';
10+
11+
return (
12+
<div id="delayed-lazy-ready">
13+
<h1>Delayed Lazy Route</h1>
14+
<p id="delayed-lazy-id">ID: {id}</p>
15+
<p id="delayed-lazy-path">{location.pathname}</p>
16+
<p id="delayed-lazy-search">{location.search}</p>
17+
<p id="delayed-lazy-hash">{location.hash}</p>
18+
<p id="delayed-lazy-view">View: {view}</p>
19+
<p id="delayed-lazy-source">Source: {source}</p>
20+
21+
<div id="navigation-links">
22+
<Link to="/" id="delayed-lazy-home-link">
23+
Back Home
24+
</Link>
25+
<br />
26+
<Link to={`/delayed-lazy/${id}?view=detailed`} id="link-to-query-view-detailed">
27+
View: Detailed (query param)
28+
</Link>
29+
<br />
30+
<Link to={`/delayed-lazy/${id}?view=list`} id="link-to-query-view-list">
31+
View: List (query param)
32+
</Link>
33+
<br />
34+
<Link to={`/delayed-lazy/${id}#section1`} id="link-to-hash-section1">
35+
Section 1 (hash only)
36+
</Link>
37+
<br />
38+
<Link to={`/delayed-lazy/${id}#section2`} id="link-to-hash-section2">
39+
Section 2 (hash only)
40+
</Link>
41+
<br />
42+
<Link to={`/delayed-lazy/${id}?view=grid#results`} id="link-to-query-and-hash">
43+
Grid View + Results (query + hash)
44+
</Link>
45+
</div>
46+
</div>
47+
);
48+
};
49+
50+
export default DelayedLazyRoute;

dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,18 @@ const Index = () => {
1919
<Link to="/long-running/slow/12345" id="navigation-to-long-running">
2020
Navigate to Long Running Lazy Route
2121
</Link>
22+
<br />
23+
<Link to="/delayed-lazy/123" id="navigation-to-delayed-lazy">
24+
Navigate to Delayed Lazy Parameterized Route
25+
</Link>
26+
<br />
27+
<Link to="/delayed-lazy/123?source=homepage" id="navigation-to-delayed-lazy-with-query">
28+
Navigate to Delayed Lazy with Query Param
29+
</Link>
30+
<br />
31+
<Link to="/deep/level2/level3/123" id="navigation-to-deep">
32+
Navigate to Deep Nested Route (3 levels, 900ms total)
33+
</Link>
2234
</>
2335
);
2436
};
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Delay: 300ms before module loads
2+
await new Promise(resolve => setTimeout(resolve, 300));
3+
4+
export const level2Routes = [
5+
{
6+
path: 'level2',
7+
handle: {
8+
lazyChildren: () => import('./Level2Routes').then(module => module.level3Routes),
9+
},
10+
},
11+
];
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Delay: 300ms before module loads
2+
await new Promise(resolve => setTimeout(resolve, 300));
3+
4+
export const level3Routes = [
5+
{
6+
path: 'level3/:id',
7+
lazy: async () => {
8+
await new Promise(resolve => setTimeout(resolve, 300));
9+
return {
10+
Component: (await import('./Level3')).default,
11+
};
12+
},
13+
},
14+
];
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import React from 'react';
2+
import { useParams } from 'react-router-dom';
3+
4+
export default function Level3() {
5+
const { id } = useParams();
6+
return (
7+
<div>
8+
<h1>Level 3 Deep Route</h1>
9+
<p id="deep-level3">Deeply nested route loaded!</p>
10+
<p id="deep-level3-id">ID: {id}</p>
11+
</div>
12+
);
13+
}
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForTransaction } from '@sentry-internal/test-utils';
3+
4+
test('lazyRouteTimeout: Routes load within timeout window', async ({ page }) => {
5+
const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
6+
return (
7+
!!transactionEvent?.transaction &&
8+
transactionEvent.contexts?.trace?.op === 'navigation' &&
9+
transactionEvent.transaction.includes('deep')
10+
);
11+
});
12+
13+
// Route takes ~900ms, timeout allows 1050ms (50 + 1000)
14+
// Routes will load in time → parameterized name
15+
await page.goto('/?idleTimeout=50&timeout=1000');
16+
17+
const navigationLink = page.locator('id=navigation-to-deep');
18+
await expect(navigationLink).toBeVisible();
19+
await navigationLink.click();
20+
21+
const event = await transactionPromise;
22+
23+
// Should get full parameterized route
24+
expect(event.transaction).toBe('/deep/level2/level3/:id');
25+
expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route');
26+
expect(event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']).toBe('idleTimeout');
27+
});
28+
29+
test('lazyRouteTimeout: Infinity timeout always waits for routes', async ({ page }) => {
30+
const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
31+
return (
32+
!!transactionEvent?.transaction &&
33+
transactionEvent.contexts?.trace?.op === 'navigation' &&
34+
transactionEvent.transaction.includes('deep')
35+
);
36+
});
37+
38+
// Infinity timeout → waits as long as possible (capped at finalTimeout to prevent indefinite hangs)
39+
await page.goto('/?idleTimeout=50&timeout=Infinity');
40+
41+
const navigationLink = page.locator('id=navigation-to-deep');
42+
await expect(navigationLink).toBeVisible();
43+
await navigationLink.click();
44+
45+
const event = await transactionPromise;
46+
47+
// Should wait for routes to load (up to finalTimeout) and get full route
48+
expect(event.transaction).toBe('/deep/level2/level3/:id');
49+
expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route');
50+
expect(event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']).toBe('idleTimeout');
51+
});
52+
53+
test('idleTimeout: Captures all activity with increased timeout', async ({ page }) => {
54+
const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
55+
return (
56+
!!transactionEvent?.transaction &&
57+
transactionEvent.contexts?.trace?.op === 'navigation' &&
58+
transactionEvent.transaction.includes('deep')
59+
);
60+
});
61+
62+
// High idleTimeout (5000ms) ensures transaction captures all lazy loading activity
63+
await page.goto('/?idleTimeout=5000');
64+
65+
const navigationLink = page.locator('id=navigation-to-deep');
66+
await expect(navigationLink).toBeVisible();
67+
await navigationLink.click();
68+
69+
const event = await transactionPromise;
70+
71+
expect(event.transaction).toBe('/deep/level2/level3/:id');
72+
expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route');
73+
expect(event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']).toBe('idleTimeout');
74+
75+
// Transaction should wait for full idle timeout (5+ seconds)
76+
const duration = event.timestamp! - event.start_timestamp;
77+
expect(duration).toBeGreaterThan(5.0);
78+
expect(duration).toBeLessThan(7.0);
79+
});
80+
81+
test('idleTimeout: Finishes prematurely with low timeout', async ({ page }) => {
82+
const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
83+
return (
84+
!!transactionEvent?.transaction &&
85+
transactionEvent.contexts?.trace?.op === 'navigation' &&
86+
transactionEvent.transaction.includes('deep')
87+
);
88+
});
89+
90+
// Very low idleTimeout (50ms) and lazyRouteTimeout (100ms)
91+
// Transaction finishes quickly, but still gets parameterized route name
92+
await page.goto('/?idleTimeout=50&timeout=100');
93+
94+
const navigationLink = page.locator('id=navigation-to-deep');
95+
await expect(navigationLink).toBeVisible();
96+
await navigationLink.click();
97+
98+
const event = await transactionPromise;
99+
100+
expect(event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']).toBe('idleTimeout');
101+
expect(event.transaction).toBe('/deep/level2/level3/:id');
102+
expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route');
103+
104+
// Transaction should finish quickly (< 200ms)
105+
const duration = event.timestamp! - event.start_timestamp;
106+
expect(duration).toBeLessThan(0.2);
107+
});
108+
109+
test('idleTimeout: Pageload on deeply nested route', async ({ page }) => {
110+
const pageloadPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
111+
return (
112+
!!transactionEvent?.transaction &&
113+
transactionEvent.contexts?.trace?.op === 'pageload' &&
114+
transactionEvent.transaction.includes('deep')
115+
);
116+
});
117+
118+
// Direct pageload to deeply nested route (not navigation)
119+
await page.goto('/deep/level2/level3/12345');
120+
121+
const pageloadEvent = await pageloadPromise;
122+
123+
expect(pageloadEvent.transaction).toBe('/deep/level2/level3/:id');
124+
expect(pageloadEvent.contexts?.trace?.data?.['sentry.source']).toBe('route');
125+
expect(pageloadEvent.contexts?.trace?.data?.['sentry.idle_span_finish_reason']).toBe('idleTimeout');
126+
});

0 commit comments

Comments
 (0)