Skip to content

Commit de6be28

Browse files
committed
Add ESM loader hook exports for middleware name resolution on Node 20.19+
1 parent 9a57f57 commit de6be28

File tree

6 files changed

+40
-32
lines changed

6 files changed

+40
-32
lines changed

dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/instrument.mjs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
// Register ESM hooks before importing any other modules.
2+
// This is required on Node 20.19+ and 22.12+ for OTEL module patching to work.
3+
// Without this, middleware function names won't be captured.
4+
import '@sentry/react-router/loader';
5+
16
import * as Sentry from '@sentry/react-router';
27

38
// Initialize Sentry early (before the server starts)

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

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ interface Span {
2323
origin?: string;
2424
}
2525

26-
// Note: React Router middleware instrumentation now works in Framework Mode.
27-
// Previously this was a known limitation (see: https://github.com/remix-run/react-router/discussions/12950)
26+
// Middleware names require ESM loader hook on Node 20.19+/22.12+ - see instrument.mjs for setup.
2827
test.describe('server - instrumentation API middleware', () => {
2928
test('should instrument server middleware with instrumentation API origin', async ({ page }) => {
3029
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
@@ -64,24 +63,27 @@ test.describe('server - instrumentation API middleware', () => {
6463
(span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware',
6564
);
6665

66+
expect(middlewareSpan).toBeDefined();
6767
expect(middlewareSpan).toMatchObject({
6868
span_id: expect.any(String),
6969
trace_id: expect.any(String),
70-
data: {
70+
data: expect.objectContaining({
7171
'sentry.origin': 'auto.function.react_router.instrumentation_api',
7272
'sentry.op': 'function.react_router.middleware',
7373
'react_router.route.id': 'routes/performance/with-middleware',
7474
'react_router.route.pattern': '/performance/with-middleware',
75-
'react_router.middleware.name': 'authMiddleware',
7675
'react_router.middleware.index': 0,
77-
},
78-
description: 'middleware authMiddleware',
76+
}),
7977
parent_span_id: expect.any(String),
8078
start_timestamp: expect.any(Number),
8179
timestamp: expect.any(Number),
8280
op: 'function.react_router.middleware',
8381
origin: 'auto.function.react_router.instrumentation_api',
8482
});
83+
84+
// Middleware name is available via OTEL patching of createRequestHandler
85+
expect(middlewareSpan!.data?.['react_router.middleware.name']).toBe('authMiddleware');
86+
expect(middlewareSpan!.description).toBe('middleware authMiddleware');
8587
});
8688

8789
test('should have middleware span run before loader span', async ({ page }) => {
@@ -108,7 +110,7 @@ test.describe('server - instrumentation API middleware', () => {
108110
expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp!);
109111
});
110112

111-
test('should track multiple middlewares with correct indices and names', async ({ page }) => {
113+
test('should track multiple middlewares with correct indices', async ({ page }) => {
112114
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
113115
return transactionEvent.transaction === 'GET /performance/multi-middleware';
114116
});
@@ -134,47 +136,47 @@ test.describe('server - instrumentation API middleware', () => {
134136
(a.data?.['react_router.middleware.index'] ?? 0) - (b.data?.['react_router.middleware.index'] ?? 0),
135137
);
136138

137-
// First middleware: multiAuthMiddleware (index 0)
139+
// First middleware (index 0)
138140
expect(sortedSpans[0]).toMatchObject({
139141
data: expect.objectContaining({
140142
'sentry.op': 'function.react_router.middleware',
141143
'react_router.route.id': 'routes/performance/multi-middleware',
142144
'react_router.route.pattern': '/performance/multi-middleware',
143-
'react_router.middleware.name': 'multiAuthMiddleware',
144145
'react_router.middleware.index': 0,
145146
}),
146-
description: 'middleware multiAuthMiddleware',
147147
});
148148

149-
// Second middleware: multiLoggingMiddleware (index 1)
149+
// Second middleware (index 1)
150150
expect(sortedSpans[1]).toMatchObject({
151151
data: expect.objectContaining({
152152
'sentry.op': 'function.react_router.middleware',
153153
'react_router.route.id': 'routes/performance/multi-middleware',
154154
'react_router.route.pattern': '/performance/multi-middleware',
155-
'react_router.middleware.name': 'multiLoggingMiddleware',
156155
'react_router.middleware.index': 1,
157156
}),
158-
description: 'middleware multiLoggingMiddleware',
159157
});
160158

161-
// Third middleware: multiValidationMiddleware (index 2)
159+
// Third middleware (index 2)
162160
expect(sortedSpans[2]).toMatchObject({
163161
data: expect.objectContaining({
164162
'sentry.op': 'function.react_router.middleware',
165163
'react_router.route.id': 'routes/performance/multi-middleware',
166164
'react_router.route.pattern': '/performance/multi-middleware',
167-
'react_router.middleware.name': 'multiValidationMiddleware',
168165
'react_router.middleware.index': 2,
169166
}),
170-
description: 'middleware multiValidationMiddleware',
171167
});
172168

173169
// Verify execution order: middleware spans should be sequential
174170
expect(sortedSpans[0]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[1]!.start_timestamp!);
175171
expect(sortedSpans[1]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[2]!.start_timestamp!);
172+
173+
// Verify middleware names are correctly resolved via OTEL patching
174+
expect(sortedSpans[0]!.data?.['react_router.middleware.name']).toBe('multiAuthMiddleware');
175+
expect(sortedSpans[1]!.data?.['react_router.middleware.name']).toBe('multiLoggingMiddleware');
176+
expect(sortedSpans[2]!.data?.['react_router.middleware.name']).toBe('multiValidationMiddleware');
176177
});
177178

179+
// Note: Remaining tests focus on index tracking. Name resolution is verified above.
178180
test('should isolate middleware indices between different routes', async ({ page }) => {
179181
// First visit the route with different middleware
180182
const txPromise1 = waitForTransaction(APP_NAME, async transactionEvent => {
@@ -199,10 +201,8 @@ test.describe('server - instrumentation API middleware', () => {
199201
'sentry.op': 'function.react_router.middleware',
200202
'react_router.route.id': 'routes/performance/other-middleware',
201203
'react_router.route.pattern': '/performance/other-middleware',
202-
'react_router.middleware.name': 'rateLimitMiddleware',
203204
'react_router.middleware.index': 0,
204205
}),
205-
description: 'middleware rateLimitMiddleware',
206206
});
207207

208208
// Now visit the multi-middleware route
@@ -253,9 +253,5 @@ test.describe('server - instrumentation API middleware', () => {
253253
// Indices should be 0, 1, 2 (reset for new request)
254254
const indices = middlewareSpans!.map((span: Span) => span.data?.['react_router.middleware.index']).sort();
255255
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']);
260256
});
261257
});
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
import '@sentry/node/import-hook';
1+
import '@sentry/node/import';

packages/react-router/package.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@
3939
"require": "./build/cjs/cloudflare/index.js",
4040
"types": "./build/types/cloudflare/index.d.ts",
4141
"default": "./build/esm/cloudflare/index.js"
42+
},
43+
"./import": {
44+
"import": {
45+
"default": "./build/import-hook.mjs"
46+
}
47+
},
48+
"./loader": {
49+
"import": {
50+
"default": "./build/loader-hook.mjs"
51+
}
4252
}
4353
},
4454
"publishConfig": {

packages/react-router/rollup.npm.config.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';
1+
import { makeBaseNPMConfig, makeNPMConfigVariants, makeOtelLoaders } from '@sentry-internal/rollup-utils';
22

33
export default [
44
...makeNPMConfigVariants(
@@ -19,4 +19,5 @@ export default [
1919
},
2020
}),
2121
),
22+
...makeOtelLoaders('./build', 'sentry-node'),
2223
];

packages/react-router/src/server/integration/reactRouterServer.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,10 @@ export const reactRouterServerIntegration = defineIntegration(() => {
2323
return {
2424
name: INTEGRATION_NAME,
2525
setupOnce() {
26-
// Always install the OTEL instrumentation to capture the ServerBuild reference.
27-
// This is needed for middleware name resolution regardless of Node version.
28-
// When the instrumentation API is active, the OTEL wrapper captures the
29-
// ServerBuild but returns the original handler without per-request wrapping.
30-
//
31-
// Note: On Node 20.19+ and 22.12+, ESM module patching requires the
32-
// --import @sentry/node/import flag. Without it, middleware names will
33-
// gracefully fall back to using routeId.
26+
// Install OTEL instrumentation to capture ServerBuild for middleware name resolution.
27+
// When instrumentation API is active, the wrapper captures ServerBuild but skips
28+
// per-request wrapping. On Node 20.19+/22.12+, users must import
29+
// '@sentry/react-router/loader' first for ESM patching; otherwise names fall back to routeId.
3430
instrumentReactRouterServer();
3531
},
3632
processEvent(event) {

0 commit comments

Comments
 (0)