Skip to content

Commit 9a57f57

Browse files
committed
feat(react-router): Include middleware function names and indices
1 parent 5dfc086 commit 9a57f57

File tree

13 files changed

+934
-96
lines changed

13 files changed

+934
-96
lines changed

dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ export default [
1010
route('server-loader', 'routes/performance/server-loader.tsx'),
1111
route('server-action', 'routes/performance/server-action.tsx'),
1212
route('with-middleware', 'routes/performance/with-middleware.tsx'),
13+
route('multi-middleware', 'routes/performance/multi-middleware.tsx'),
14+
route('other-middleware', 'routes/performance/other-middleware.tsx'),
1315
route('error-loader', 'routes/performance/error-loader.tsx'),
1416
route('error-action', 'routes/performance/error-action.tsx'),
1517
route('error-middleware', 'routes/performance/error-middleware.tsx'),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { Route } from './+types/multi-middleware';
2+
3+
// Multiple middleware functions to test index tracking
4+
// Using unique names to avoid bundler renaming due to collisions with other routes
5+
export const middleware: Route.MiddlewareFunction[] = [
6+
async function multiAuthMiddleware({ context }, next) {
7+
(context as any).auth = true;
8+
const response = await next();
9+
return response;
10+
},
11+
async function multiLoggingMiddleware({ context }, next) {
12+
(context as any).logged = true;
13+
const response = await next();
14+
return response;
15+
},
16+
async function multiValidationMiddleware({ context }, next) {
17+
(context as any).validated = true;
18+
const response = await next();
19+
return response;
20+
},
21+
];
22+
23+
export function loader() {
24+
return { message: 'Multi-middleware route loaded' };
25+
}
26+
27+
export default function MultiMiddlewarePage() {
28+
return (
29+
<div>
30+
<h1 id="multi-middleware-title">Multi Middleware Route</h1>
31+
<p id="multi-middleware-content">This route has 3 middlewares</p>
32+
</div>
33+
);
34+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { Route } from './+types/other-middleware';
2+
3+
// Different middleware to test isolation between routes
4+
export const middleware: Route.MiddlewareFunction[] = [
5+
async function rateLimitMiddleware({ context }, next) {
6+
(context as any).rateLimited = false;
7+
const response = await next();
8+
return response;
9+
},
10+
];
11+
12+
export function loader() {
13+
return { message: 'Other middleware route loaded' };
14+
}
15+
16+
export default function OtherMiddlewarePage() {
17+
return (
18+
<div>
19+
<h1 id="other-middleware-title">Other Middleware Route</h1>
20+
<p id="other-middleware-content">This route has a different middleware</p>
21+
</div>
22+
);
23+
}

dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts

Lines changed: 181 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,27 @@ import { expect, test } from '@playwright/test';
22
import { waitForTransaction } from '@sentry-internal/test-utils';
33
import { APP_NAME } from '../constants';
44

5+
interface SpanData {
6+
'sentry.op'?: string;
7+
'sentry.origin'?: string;
8+
'react_router.route.id'?: string;
9+
'react_router.route.pattern'?: string;
10+
'react_router.middleware.name'?: string;
11+
'react_router.middleware.index'?: number;
12+
}
13+
14+
interface Span {
15+
span_id?: string;
16+
trace_id?: string;
17+
data?: SpanData;
18+
description?: string;
19+
parent_span_id?: string;
20+
start_timestamp?: number;
21+
timestamp?: number;
22+
op?: string;
23+
origin?: string;
24+
}
25+
526
// Note: React Router middleware instrumentation now works in Framework Mode.
627
// Previously this was a known limitation (see: https://github.com/remix-run/react-router/discussions/12950)
728
test.describe('server - instrumentation API middleware', () => {
@@ -40,7 +61,7 @@ test.describe('server - instrumentation API middleware', () => {
4061

4162
// Find the middleware span
4263
const middlewareSpan = transaction?.spans?.find(
43-
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
64+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
4465
);
4566

4667
expect(middlewareSpan).toMatchObject({
@@ -49,8 +70,12 @@ test.describe('server - instrumentation API middleware', () => {
4970
data: {
5071
'sentry.origin': 'auto.function.react_router.instrumentation_api',
5172
'sentry.op': 'function.react_router.middleware',
73+
'react_router.route.id': 'routes/performance/with-middleware',
74+
'react_router.route.pattern': '/performance/with-middleware',
75+
'react_router.middleware.name': 'authMiddleware',
76+
'react_router.middleware.index': 0,
5277
},
53-
description: '/performance/with-middleware',
78+
description: 'middleware authMiddleware',
5479
parent_span_id: expect.any(String),
5580
start_timestamp: expect.any(Number),
5681
timestamp: expect.any(Number),
@@ -69,17 +94,168 @@ test.describe('server - instrumentation API middleware', () => {
6994
const transaction = await txPromise;
7095

7196
const middlewareSpan = transaction?.spans?.find(
72-
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
97+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
7398
);
7499

75100
const loaderSpan = transaction?.spans?.find(
76-
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader',
101+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.loader',
77102
);
78103

79104
expect(middlewareSpan).toBeDefined();
80105
expect(loaderSpan).toBeDefined();
81106

82107
// Middleware should start before loader
83-
expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp);
108+
expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp!);
109+
});
110+
111+
test('should track multiple middlewares with correct indices and names', async ({ page }) => {
112+
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
113+
return transactionEvent.transaction === 'GET /performance/multi-middleware';
114+
});
115+
116+
await page.goto(`/performance/multi-middleware`);
117+
118+
const transaction = await txPromise;
119+
120+
// Verify the page rendered
121+
await expect(page.locator('#multi-middleware-title')).toBeVisible();
122+
await expect(page.locator('#multi-middleware-content')).toHaveText('This route has 3 middlewares');
123+
124+
// Find all middleware spans
125+
const middlewareSpans = transaction?.spans?.filter(
126+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
127+
);
128+
129+
expect(middlewareSpans).toHaveLength(3);
130+
131+
// Sort by index to ensure correct order
132+
const sortedSpans = [...middlewareSpans!].sort(
133+
(a: Span, b: Span) =>
134+
(a.data?.['react_router.middleware.index'] ?? 0) - (b.data?.['react_router.middleware.index'] ?? 0),
135+
);
136+
137+
// First middleware: multiAuthMiddleware (index 0)
138+
expect(sortedSpans[0]).toMatchObject({
139+
data: expect.objectContaining({
140+
'sentry.op': 'function.react_router.middleware',
141+
'react_router.route.id': 'routes/performance/multi-middleware',
142+
'react_router.route.pattern': '/performance/multi-middleware',
143+
'react_router.middleware.name': 'multiAuthMiddleware',
144+
'react_router.middleware.index': 0,
145+
}),
146+
description: 'middleware multiAuthMiddleware',
147+
});
148+
149+
// Second middleware: multiLoggingMiddleware (index 1)
150+
expect(sortedSpans[1]).toMatchObject({
151+
data: expect.objectContaining({
152+
'sentry.op': 'function.react_router.middleware',
153+
'react_router.route.id': 'routes/performance/multi-middleware',
154+
'react_router.route.pattern': '/performance/multi-middleware',
155+
'react_router.middleware.name': 'multiLoggingMiddleware',
156+
'react_router.middleware.index': 1,
157+
}),
158+
description: 'middleware multiLoggingMiddleware',
159+
});
160+
161+
// Third middleware: multiValidationMiddleware (index 2)
162+
expect(sortedSpans[2]).toMatchObject({
163+
data: expect.objectContaining({
164+
'sentry.op': 'function.react_router.middleware',
165+
'react_router.route.id': 'routes/performance/multi-middleware',
166+
'react_router.route.pattern': '/performance/multi-middleware',
167+
'react_router.middleware.name': 'multiValidationMiddleware',
168+
'react_router.middleware.index': 2,
169+
}),
170+
description: 'middleware multiValidationMiddleware',
171+
});
172+
173+
// Verify execution order: middleware spans should be sequential
174+
expect(sortedSpans[0]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[1]!.start_timestamp!);
175+
expect(sortedSpans[1]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[2]!.start_timestamp!);
176+
});
177+
178+
test('should isolate middleware indices between different routes', async ({ page }) => {
179+
// First visit the route with different middleware
180+
const txPromise1 = waitForTransaction(APP_NAME, async transactionEvent => {
181+
return transactionEvent.transaction === 'GET /performance/other-middleware';
182+
});
183+
184+
await page.goto(`/performance/other-middleware`);
185+
186+
const transaction1 = await txPromise1;
187+
188+
// Verify the page rendered
189+
await expect(page.locator('#other-middleware-title')).toBeVisible();
190+
191+
// Find the middleware span
192+
const middlewareSpan1 = transaction1?.spans?.find(
193+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
194+
);
195+
196+
// The other route should have its own middleware with index 0
197+
expect(middlewareSpan1).toMatchObject({
198+
data: expect.objectContaining({
199+
'sentry.op': 'function.react_router.middleware',
200+
'react_router.route.id': 'routes/performance/other-middleware',
201+
'react_router.route.pattern': '/performance/other-middleware',
202+
'react_router.middleware.name': 'rateLimitMiddleware',
203+
'react_router.middleware.index': 0,
204+
}),
205+
description: 'middleware rateLimitMiddleware',
206+
});
207+
208+
// Now visit the multi-middleware route
209+
const txPromise2 = waitForTransaction(APP_NAME, async transactionEvent => {
210+
return transactionEvent.transaction === 'GET /performance/multi-middleware';
211+
});
212+
213+
await page.goto(`/performance/multi-middleware`);
214+
215+
const transaction2 = await txPromise2;
216+
217+
// Find all middleware spans
218+
const middlewareSpans2 = transaction2?.spans?.filter(
219+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
220+
);
221+
222+
// Should have 3 middleware spans with indices 0, 1, 2 (isolated from previous route)
223+
expect(middlewareSpans2).toHaveLength(3);
224+
225+
const indices = middlewareSpans2!.map((span: Span) => span.data?.['react_router.middleware.index']).sort();
226+
expect(indices).toEqual([0, 1, 2]);
227+
});
228+
229+
test('should handle visiting same multi-middleware route twice with fresh indices', async ({ page }) => {
230+
// First visit
231+
const txPromise1 = waitForTransaction(APP_NAME, async transactionEvent => {
232+
return transactionEvent.transaction === 'GET /performance/multi-middleware';
233+
});
234+
235+
await page.goto(`/performance/multi-middleware`);
236+
await txPromise1;
237+
238+
// Second visit - indices should reset
239+
const txPromise2 = waitForTransaction(APP_NAME, async transactionEvent => {
240+
return transactionEvent.transaction === 'GET /performance/multi-middleware';
241+
});
242+
243+
await page.goto(`/performance/multi-middleware`);
244+
245+
const transaction2 = await txPromise2;
246+
247+
const middlewareSpans = transaction2?.spans?.filter(
248+
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
249+
);
250+
251+
expect(middlewareSpans).toHaveLength(3);
252+
253+
// Indices should be 0, 1, 2 (reset for new request)
254+
const indices = middlewareSpans!.map((span: Span) => span.data?.['react_router.middleware.index']).sort();
255+
expect(indices).toEqual([0, 1, 2]);
256+
257+
// Names should still be correct
258+
const names = middlewareSpans!.map((span: Span) => span.data?.['react_router.middleware.name']).sort();
259+
expect(names).toEqual(['multiAuthMiddleware', 'multiLoggingMiddleware', 'multiValidationMiddleware']);
84260
});
85261
});

packages/react-router/src/client/createClientInstrumentation.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;
1919
// Tracks active numeric navigation span to prevent duplicate spans when popstate fires
2020
let currentNumericNavigationSpan: Span | undefined;
2121

22+
// Tracks middleware execution index per route, keyed by Request object.
23+
// Uses WeakMap to isolate counters per navigation and allow GC of cancelled navigations.
24+
const middlewareCountersMap = new WeakMap<object, Record<string, number>>();
25+
2226
const SENTRY_CLIENT_INSTRUMENTATION_FLAG = '__sentryReactRouterClientInstrumentationUsed';
2327
// Intentionally never reset - once set, instrumentation API handles all navigations for the session.
2428
const SENTRY_NAVIGATE_HOOK_INVOKED_FLAG = '__sentryReactRouterNavigateHookInvoked';
@@ -214,6 +218,8 @@ export function createSentryClientInstrumentation(
214218
},
215219

216220
route(route: InstrumentableRoute) {
221+
const routeId = route.id;
222+
217223
route.instrument({
218224
async loader(callLoader, info) {
219225
const urlPath = getPathFromRequest(info.request);
@@ -267,12 +273,33 @@ export function createSentryClientInstrumentation(
267273
const urlPath = getPathFromRequest(info.request);
268274
const routePattern = normalizeRoutePath(getPattern(info)) || urlPath;
269275

276+
// Get or create counters for this navigation's Request
277+
let counters = middlewareCountersMap.get(info.request);
278+
if (!counters) {
279+
counters = {};
280+
middlewareCountersMap.set(info.request, counters);
281+
}
282+
283+
// Get middleware index and increment for next middleware
284+
const middlewareIndex = counters[routeId] ?? 0;
285+
counters[routeId] = middlewareIndex + 1;
286+
287+
// Try to get the actual middleware function name
288+
const middlewareName = getClientMiddlewareName(routeId, middlewareIndex);
289+
290+
// Build display name: prefer function name, fallback to routeId
291+
const displayName = middlewareName || routeId;
292+
270293
await startSpan(
271294
{
272-
name: routePattern,
295+
name: `middleware ${displayName}`,
273296
attributes: {
274297
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react_router.client_middleware',
275298
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
299+
'react_router.route.id': routeId,
300+
'react_router.route.pattern': routePattern,
301+
...(middlewareName && { 'react_router.middleware.name': middlewareName }),
302+
'react_router.middleware.index': middlewareIndex,
276303
},
277304
},
278305
async span => {
@@ -325,3 +352,30 @@ export function isClientInstrumentationApiUsed(): boolean {
325352
export function isNavigateHookInvoked(): boolean {
326353
return !!GLOBAL_WITH_FLAGS[SENTRY_NAVIGATE_HOOK_INVOKED_FLAG];
327354
}
355+
356+
interface RouteModule {
357+
[key: string]: unknown;
358+
clientMiddleware?: Array<{ name?: string }>;
359+
}
360+
361+
interface GlobalObjWithRouteModules {
362+
__reactRouterRouteModules?: Record<string, RouteModule>;
363+
}
364+
365+
/**
366+
* Get client middleware function name from __reactRouterRouteModules.
367+
* @internal
368+
*/
369+
function getClientMiddlewareName(routeId: string, index: number): string | undefined {
370+
const globalWithModules = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalObjWithRouteModules;
371+
const routeModules = globalWithModules.__reactRouterRouteModules;
372+
if (!routeModules) return undefined;
373+
374+
const routeModule = routeModules[routeId];
375+
// Client middleware is exposed as clientMiddleware in route modules
376+
const clientMiddleware = routeModule?.clientMiddleware;
377+
if (!Array.isArray(clientMiddleware)) return undefined;
378+
379+
const middlewareFn = clientMiddleware[index];
380+
return middlewareFn?.name || undefined;
381+
}

0 commit comments

Comments
 (0)