diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts index 6bd5b27264eb..477336455452 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts @@ -10,6 +10,8 @@ export default [ route('server-loader', 'routes/performance/server-loader.tsx'), route('server-action', 'routes/performance/server-action.tsx'), route('with-middleware', 'routes/performance/with-middleware.tsx'), + route('multi-middleware', 'routes/performance/multi-middleware.tsx'), + route('other-middleware', 'routes/performance/other-middleware.tsx'), route('error-loader', 'routes/performance/error-loader.tsx'), route('error-action', 'routes/performance/error-action.tsx'), route('error-middleware', 'routes/performance/error-middleware.tsx'), diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/multi-middleware.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/multi-middleware.tsx new file mode 100644 index 000000000000..4b43ad619901 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/multi-middleware.tsx @@ -0,0 +1,34 @@ +import type { Route } from './+types/multi-middleware'; + +// Multiple middleware functions to test index tracking +// Using unique names to avoid bundler renaming due to collisions with other routes +export const middleware: Route.MiddlewareFunction[] = [ + async function multiAuthMiddleware({ context }, next) { + (context as any).auth = true; + const response = await next(); + return response; + }, + async function multiLoggingMiddleware({ context }, next) { + (context as any).logged = true; + const response = await next(); + return response; + }, + async function multiValidationMiddleware({ context }, next) { + (context as any).validated = true; + const response = await next(); + return response; + }, +]; + +export function loader() { + return { message: 'Multi-middleware route loaded' }; +} + +export default function MultiMiddlewarePage() { + return ( +
+

Multi Middleware Route

+

This route has 3 middlewares

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/other-middleware.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/other-middleware.tsx new file mode 100644 index 000000000000..7f68cd35e314 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/other-middleware.tsx @@ -0,0 +1,23 @@ +import type { Route } from './+types/other-middleware'; + +// Different middleware to test isolation between routes +export const middleware: Route.MiddlewareFunction[] = [ + async function rateLimitMiddleware({ context }, next) { + (context as any).rateLimited = false; + const response = await next(); + return response; + }, +]; + +export function loader() { + return { message: 'Other middleware route loaded' }; +} + +export default function OtherMiddlewarePage() { + return ( +
+

Other Middleware Route

+

This route has a different middleware

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts index e99a58a7f57c..ff9bec3bedd9 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts @@ -2,6 +2,27 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from '../constants'; +interface SpanData { + 'sentry.op'?: string; + 'sentry.origin'?: string; + 'react_router.route.id'?: string; + 'react_router.route.pattern'?: string; + 'react_router.middleware.name'?: string; + 'react_router.middleware.index'?: number; +} + +interface Span { + span_id?: string; + trace_id?: string; + data?: SpanData; + description?: string; + parent_span_id?: string; + start_timestamp?: number; + timestamp?: number; + op?: string; + origin?: string; +} + // Note: React Router middleware instrumentation now works in Framework Mode. // Previously this was a known limitation (see: https://github.com/remix-run/react-router/discussions/12950) test.describe('server - instrumentation API middleware', () => { @@ -40,7 +61,7 @@ test.describe('server - instrumentation API middleware', () => { // Find the middleware span const middlewareSpan = transaction?.spans?.find( - (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware', + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', ); expect(middlewareSpan).toMatchObject({ @@ -49,8 +70,12 @@ test.describe('server - instrumentation API middleware', () => { data: { 'sentry.origin': 'auto.function.react_router.instrumentation_api', 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/performance/with-middleware', + 'react_router.route.pattern': '/performance/with-middleware', + 'react_router.middleware.name': 'authMiddleware', + 'react_router.middleware.index': 0, }, - description: '/performance/with-middleware', + description: 'middleware authMiddleware', parent_span_id: expect.any(String), start_timestamp: expect.any(Number), timestamp: expect.any(Number), @@ -69,17 +94,168 @@ test.describe('server - instrumentation API middleware', () => { const transaction = await txPromise; const middlewareSpan = transaction?.spans?.find( - (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware', + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', ); const loaderSpan = transaction?.spans?.find( - (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader', + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.loader', ); expect(middlewareSpan).toBeDefined(); expect(loaderSpan).toBeDefined(); // Middleware should start before loader - expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp); + expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp!); + }); + + test('should track multiple middlewares with correct indices and names', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/multi-middleware'; + }); + + await page.goto(`/performance/multi-middleware`); + + const transaction = await txPromise; + + // Verify the page rendered + await expect(page.locator('#multi-middleware-title')).toBeVisible(); + await expect(page.locator('#multi-middleware-content')).toHaveText('This route has 3 middlewares'); + + // Find all middleware spans + const middlewareSpans = transaction?.spans?.filter( + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', + ); + + expect(middlewareSpans).toHaveLength(3); + + // Sort by index to ensure correct order + const sortedSpans = [...middlewareSpans!].sort( + (a: Span, b: Span) => + (a.data?.['react_router.middleware.index'] ?? 0) - (b.data?.['react_router.middleware.index'] ?? 0), + ); + + // First middleware: multiAuthMiddleware (index 0) + expect(sortedSpans[0]).toMatchObject({ + data: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/performance/multi-middleware', + 'react_router.route.pattern': '/performance/multi-middleware', + 'react_router.middleware.name': 'multiAuthMiddleware', + 'react_router.middleware.index': 0, + }), + description: 'middleware multiAuthMiddleware', + }); + + // Second middleware: multiLoggingMiddleware (index 1) + expect(sortedSpans[1]).toMatchObject({ + data: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/performance/multi-middleware', + 'react_router.route.pattern': '/performance/multi-middleware', + 'react_router.middleware.name': 'multiLoggingMiddleware', + 'react_router.middleware.index': 1, + }), + description: 'middleware multiLoggingMiddleware', + }); + + // Third middleware: multiValidationMiddleware (index 2) + expect(sortedSpans[2]).toMatchObject({ + data: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/performance/multi-middleware', + 'react_router.route.pattern': '/performance/multi-middleware', + 'react_router.middleware.name': 'multiValidationMiddleware', + 'react_router.middleware.index': 2, + }), + description: 'middleware multiValidationMiddleware', + }); + + // Verify execution order: middleware spans should be sequential + expect(sortedSpans[0]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[1]!.start_timestamp!); + expect(sortedSpans[1]!.start_timestamp).toBeLessThanOrEqual(sortedSpans[2]!.start_timestamp!); + }); + + test('should isolate middleware indices between different routes', async ({ page }) => { + // First visit the route with different middleware + const txPromise1 = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/other-middleware'; + }); + + await page.goto(`/performance/other-middleware`); + + const transaction1 = await txPromise1; + + // Verify the page rendered + await expect(page.locator('#other-middleware-title')).toBeVisible(); + + // Find the middleware span + const middlewareSpan1 = transaction1?.spans?.find( + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', + ); + + // The other route should have its own middleware with index 0 + expect(middlewareSpan1).toMatchObject({ + data: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/performance/other-middleware', + 'react_router.route.pattern': '/performance/other-middleware', + 'react_router.middleware.name': 'rateLimitMiddleware', + 'react_router.middleware.index': 0, + }), + description: 'middleware rateLimitMiddleware', + }); + + // Now visit the multi-middleware route + const txPromise2 = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/multi-middleware'; + }); + + await page.goto(`/performance/multi-middleware`); + + const transaction2 = await txPromise2; + + // Find all middleware spans + const middlewareSpans2 = transaction2?.spans?.filter( + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', + ); + + // Should have 3 middleware spans with indices 0, 1, 2 (isolated from previous route) + expect(middlewareSpans2).toHaveLength(3); + + const indices = middlewareSpans2!.map((span: Span) => span.data?.['react_router.middleware.index']).sort(); + expect(indices).toEqual([0, 1, 2]); + }); + + test('should handle visiting same multi-middleware route twice with fresh indices', async ({ page }) => { + // First visit + const txPromise1 = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/multi-middleware'; + }); + + await page.goto(`/performance/multi-middleware`); + await txPromise1; + + // Second visit - indices should reset + const txPromise2 = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/multi-middleware'; + }); + + await page.goto(`/performance/multi-middleware`); + + const transaction2 = await txPromise2; + + const middlewareSpans = transaction2?.spans?.filter( + (span: Span) => span.data?.['sentry.op'] === 'function.react_router.middleware', + ); + + expect(middlewareSpans).toHaveLength(3); + + // Indices should be 0, 1, 2 (reset for new request) + const indices = middlewareSpans!.map((span: Span) => span.data?.['react_router.middleware.index']).sort(); + expect(indices).toEqual([0, 1, 2]); + + // Names should still be correct + const names = middlewareSpans!.map((span: Span) => span.data?.['react_router.middleware.name']).sort(); + expect(names).toEqual(['multiAuthMiddleware', 'multiLoggingMiddleware', 'multiValidationMiddleware']); }); }); diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index c465a25dd662..888f9e879416 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -19,6 +19,10 @@ const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; // Tracks active numeric navigation span to prevent duplicate spans when popstate fires let currentNumericNavigationSpan: Span | undefined; +// Tracks middleware execution index per route, keyed by Request object. +// Uses WeakMap to isolate counters per navigation and allow GC of cancelled navigations. +const middlewareCountersMap = new WeakMap>(); + const SENTRY_CLIENT_INSTRUMENTATION_FLAG = '__sentryReactRouterClientInstrumentationUsed'; // Intentionally never reset - once set, instrumentation API handles all navigations for the session. const SENTRY_NAVIGATE_HOOK_INVOKED_FLAG = '__sentryReactRouterNavigateHookInvoked'; @@ -214,6 +218,8 @@ export function createSentryClientInstrumentation( }, route(route: InstrumentableRoute) { + const routeId = route.id; + route.instrument({ async loader(callLoader, info) { const urlPath = getPathFromRequest(info.request); @@ -267,12 +273,33 @@ export function createSentryClientInstrumentation( const urlPath = getPathFromRequest(info.request); const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + // Get or create counters for this navigation's Request + let counters = middlewareCountersMap.get(info.request); + if (!counters) { + counters = {}; + middlewareCountersMap.set(info.request, counters); + } + + // Get middleware index and increment for next middleware + const middlewareIndex = counters[routeId] ?? 0; + counters[routeId] = middlewareIndex + 1; + + // Try to get the actual middleware function name + const middlewareName = getClientMiddlewareName(routeId, middlewareIndex); + + // Build display name: prefer function name, fallback to routeId + const displayName = middlewareName || routeId; + await startSpan( { - name: routePattern, + name: `middleware ${displayName}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react_router.client_middleware', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + 'react_router.route.id': routeId, + 'react_router.route.pattern': routePattern, + ...(middlewareName && { 'react_router.middleware.name': middlewareName }), + 'react_router.middleware.index': middlewareIndex, }, }, async span => { @@ -325,3 +352,30 @@ export function isClientInstrumentationApiUsed(): boolean { export function isNavigateHookInvoked(): boolean { return !!GLOBAL_WITH_FLAGS[SENTRY_NAVIGATE_HOOK_INVOKED_FLAG]; } + +interface RouteModule { + [key: string]: unknown; + clientMiddleware?: Array<{ name?: string }>; +} + +interface GlobalObjWithRouteModules { + __reactRouterRouteModules?: Record; +} + +/** + * Get client middleware function name from __reactRouterRouteModules. + * @internal + */ +function getClientMiddlewareName(routeId: string, index: number): string | undefined { + const globalWithModules = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalObjWithRouteModules; + const routeModules = globalWithModules.__reactRouterRouteModules; + if (!routeModules) return undefined; + + const routeModule = routeModules[routeId]; + // Client middleware is exposed as clientMiddleware in route modules + const clientMiddleware = routeModule?.clientMiddleware; + if (!Array.isArray(clientMiddleware)) return undefined; + + const middlewareFn = clientMiddleware[index]; + return middlewareFn?.name || undefined; +} diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts index 3fceca6a4ff7..4f1e430d2988 100644 --- a/packages/react-router/src/server/createServerInstrumentation.ts +++ b/packages/react-router/src/server/createServerInstrumentation.ts @@ -1,4 +1,4 @@ -import { context } from '@opentelemetry/api'; +import { context, createContextKey } from '@opentelemetry/api'; import { getRPCMetadata, RPCType } from '@opentelemetry/core'; import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; import { @@ -17,8 +17,17 @@ import { import { DEBUG_BUILD } from '../common/debug-build'; import type { InstrumentableRequestHandler, InstrumentableRoute, ServerInstrumentation } from '../common/types'; import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils'; +import { getMiddlewareName } from './serverBuild'; import { markInstrumentationApiUsed } from './serverGlobals'; +// OTel context key for tracking middleware execution index per route +const MIDDLEWARE_COUNTER_KEY = createContextKey('sentry_react_router_middleware_counter'); + +interface MiddlewareCounterStore { + // Maps routeId to current middleware index + counters: Record; +} + // Re-export for backward compatibility and external use export { isInstrumentationApiUsed } from './serverGlobals'; @@ -53,6 +62,10 @@ export function createSentryServerInstrumentation( const activeSpan = getActiveSpan(); const existingRootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; + // Initialize middleware counter for this request + const counterStore: MiddlewareCounterStore = { counters: {} }; + const ctx = context.active().setValue(MIDDLEWARE_COUNTER_KEY, counterStore); + if (existingRootSpan) { updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); existingRootSpan.setAttributes({ @@ -61,53 +74,59 @@ export function createSentryServerInstrumentation( [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }); - try { - const result = await handleRequest(); - if (result.status === 'error' && result.error instanceof Error) { - existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { - 'http.method': info.request.method, - 'http.url': pathname, - }); + await context.with(ctx, async () => { + try { + const result = await handleRequest(); + if (result.status === 'error' && result.error instanceof Error) { + existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + 'http.url': pathname, + }); + } + } finally { + await flushIfServerless(); } - } finally { - await flushIfServerless(); - } + }); } else { - await startSpan( - { - name: `${info.request.method} ${pathname}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - 'http.request.method': info.request.method, - 'url.path': pathname, - 'url.full': info.request.url, + await context.with(ctx, async () => { + await startSpan( + { + name: `${info.request.method} ${pathname}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + 'http.request.method': info.request.method, + 'url.path': pathname, + 'url.full': info.request.url, + }, }, - }, - async span => { - try { - const result = await handleRequest(); - if (result.status === 'error' && result.error instanceof Error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { - 'http.method': info.request.method, - 'http.url': pathname, - }); + async span => { + try { + const result = await handleRequest(); + if (result.status === 'error' && result.error instanceof Error) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + 'http.url': pathname, + }); + } + } finally { + await flushIfServerless(); } - } finally { - await flushIfServerless(); - } - }, - ); + }, + ); + }); } }, }); }, route(route: InstrumentableRoute) { + const routeId = route.id; + route.instrument({ async loader(callLoader, info) { const urlPath = getPathFromRequest(info.request); @@ -168,15 +187,28 @@ export function createSentryServerInstrumentation( const pattern = getPattern(info); const routePattern = normalizeRoutePath(pattern) || urlPath; - // Update root span with parameterized route (same as loader/action) updateRootSpanWithRoute(info.request.method, pattern, urlPath); + const counterStore = context.active().getValue(MIDDLEWARE_COUNTER_KEY) as MiddlewareCounterStore | undefined; + let middlewareIndex = 0; + if (counterStore) { + middlewareIndex = counterStore.counters[routeId] ?? 0; + counterStore.counters[routeId] = middlewareIndex + 1; + } + + const middlewareName = getMiddlewareName(routeId, middlewareIndex); + const displayName = middlewareName || routeId; + await startSpan( { - name: routePattern, + name: `middleware ${displayName}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react_router.middleware', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + 'react_router.route.id': routeId, + 'react_router.route.pattern': routePattern, + ...(middlewareName && { 'react_router.middleware.name': middlewareName }), + 'react_router.middleware.index': middlewareIndex, }, }, async span => { diff --git a/packages/react-router/src/server/instrumentation/reactRouter.ts b/packages/react-router/src/server/instrumentation/reactRouter.ts index 2f24d2c7bcb7..68e648edf7a2 100644 --- a/packages/react-router/src/server/instrumentation/reactRouter.ts +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -15,6 +15,7 @@ import { } from '@sentry/core'; import type * as reactRouter from 'react-router'; import { DEBUG_BUILD } from '../../common/debug-build'; +import { isServerBuildLike, setServerBuild } from '../serverBuild'; import { isInstrumentationApiUsed } from '../serverGlobals'; import { getOpName, getSpanName, isDataRequest } from './util'; @@ -62,8 +63,20 @@ export class ReactRouterInstrumentation extends InstrumentationBase { return { name: INTEGRATION_NAME, setupOnce() { - // Skip OTEL patching if the instrumentation API is in use - if (isInstrumentationApiUsed()) { - return; - } - - if ( - (NODE_VERSION.major === 20 && NODE_VERSION.minor < 19) || // https://nodejs.org/en/blog/release/v20.19.0 - (NODE_VERSION.major === 22 && NODE_VERSION.minor < 12) // https://nodejs.org/en/blog/release/v22.12.0 - ) { - instrumentReactRouterServer(); - } + // Always install the OTEL instrumentation to capture the ServerBuild reference. + // This is needed for middleware name resolution regardless of Node version. + // When the instrumentation API is active, the OTEL wrapper captures the + // ServerBuild but returns the original handler without per-request wrapping. + // + // Note: On Node 20.19+ and 22.12+, ESM module patching requires the + // --import @sentry/node/import flag. Without it, middleware names will + // gracefully fall back to using routeId. + instrumentReactRouterServer(); }, processEvent(event) { // Express generates bogus `*` routes for data loaders, which we want to remove here diff --git a/packages/react-router/src/server/serverBuild.ts b/packages/react-router/src/server/serverBuild.ts new file mode 100644 index 000000000000..9126753d2231 --- /dev/null +++ b/packages/react-router/src/server/serverBuild.ts @@ -0,0 +1,66 @@ +/** + * Loose type for ServerBuild to access middleware properties + * that may not be in the official React Router types. + * @internal + */ +interface ServerBuildLike { + routes?: Record< + string, + { + id?: string; + module?: { + middleware?: Array<{ name?: string }>; + }; + } + >; +} + +// ServerBuild reference for middleware name lookup. Updated on each createRequestHandler call. +let _serverBuild: ServerBuildLike | undefined; + +/** + * Type guard to check if an object is a ServerBuild-like structure. + * @internal + */ +export function isServerBuildLike(build: unknown): build is ServerBuildLike { + if (build === null || typeof build !== 'object' || !('routes' in build)) { + return false; + } + const routes = (build as { routes: unknown }).routes; + return routes !== null && typeof routes === 'object'; +} + +/** + * Stores reference to the React Router ServerBuild. + * Called when createRequestHandler is invoked. + * @internal + */ +export function setServerBuild(build: ServerBuildLike): void { + _serverBuild = build; +} + +/** + * Looks up a middleware function name from the ServerBuild. + * @param routeId - The route ID + * @param index - The middleware function index within the route's middleware array + * @returns The middleware function name if available, undefined otherwise + * @internal + */ +export function getMiddlewareName(routeId: string, index: number): string | undefined { + if (!_serverBuild?.routes) return undefined; + + const route = _serverBuild.routes[routeId]; + if (!route?.module?.middleware) return undefined; + + const middlewareFn = route.module.middleware[index]; + return middlewareFn?.name || undefined; +} + +/** + * Clears the stored ServerBuild reference. + * Only used for testing purposes. + * @internal + */ +export function _resetServerBuild(): void { + _serverBuild = undefined; +} diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index 0078b2601c51..46db5a6191c2 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -453,7 +453,7 @@ describe('createSentryClientInstrumentation', () => { ); }); - it('should instrument route middleware with spans', async () => { + it('should instrument route middleware with spans (without function name)', async () => { const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); @@ -478,14 +478,67 @@ describe('createSentryClientInstrumentation', () => { expect(core.startSpan).toHaveBeenCalledWith( expect.objectContaining({ - name: '/users/:id', + name: 'middleware test-route', attributes: expect.objectContaining({ 'sentry.op': 'function.react_router.client_middleware', 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'react_router.route.id': 'test-route', + 'react_router.route.pattern': '/users/:id', + 'react_router.middleware.index': 0, + }), + }), + expect.any(Function), + ); + }); + + it('should use middleware function name when available from window.__reactRouterRouteModules', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + // Use a unique route ID to avoid counter conflicts with other tests + const routeId = 'test-route-with-name'; + + // Mock window.__reactRouterRouteModules + (globalThis as any).__reactRouterRouteModules = { + [routeId]: { + clientMiddleware: [{ name: 'authClientMiddleware' }], + }, + }; + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: routeId, + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.middleware(mockCallMiddleware, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'middleware authClientMiddleware', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react_router.client_middleware', + 'react_router.route.id': routeId, + 'react_router.middleware.name': 'authClientMiddleware', + 'react_router.middleware.index': 0, }), }), expect.any(Function), ); + + // Cleanup + delete (globalThis as any).__reactRouterRouteModules; }); it('should instrument lazy route loading with spans', async () => { diff --git a/packages/react-router/test/server/createServerInstrumentation.test.ts b/packages/react-router/test/server/createServerInstrumentation.test.ts index 33eb73f48ace..14937d82c158 100644 --- a/packages/react-router/test/server/createServerInstrumentation.test.ts +++ b/packages/react-router/test/server/createServerInstrumentation.test.ts @@ -1,9 +1,11 @@ +import * as otelApi from '@opentelemetry/api'; import * as core from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createSentryServerInstrumentation, isInstrumentationApiUsed, } from '../../src/server/createServerInstrumentation'; +import * as serverBuildModule from '../../src/server/serverBuild'; vi.mock('@sentry/core', async () => { const actual = await vi.importActual('@sentry/core'); @@ -22,6 +24,25 @@ vi.mock('@sentry/core', async () => { }; }); +vi.mock('../../src/server/serverBuild', () => ({ + getMiddlewareName: vi.fn(), +})); + +vi.mock('@opentelemetry/api', async () => { + const actual = await vi.importActual('@opentelemetry/api'); + return { + ...actual, + context: { + active: vi.fn(() => ({ + getValue: vi.fn(), + setValue: vi.fn(), + })), + with: vi.fn((ctx, fn) => fn()), + }, + createContextKey: actual.createContextKey, + }; +}); + describe('createSentryServerInstrumentation', () => { beforeEach(() => { vi.clearAllMocks(); @@ -287,12 +308,15 @@ describe('createSentryServerInstrumentation', () => { ); }); - it('should instrument route middleware with spans', async () => { + it('should instrument route middleware with spans (without function name)', async () => { const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); const mockSetAttributes = vi.fn(); const mockRootSpan = { setAttributes: mockSetAttributes }; + // No middleware name available + vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue(undefined); + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); (core.getActiveSpan as any).mockReturnValue({}); (core.getRootSpan as any).mockReturnValue(mockRootSpan); @@ -317,17 +341,18 @@ describe('createSentryServerInstrumentation', () => { expect(core.startSpan).toHaveBeenCalledWith( expect.objectContaining({ - name: '/users/:id', + name: 'middleware test-route', attributes: expect.objectContaining({ 'sentry.op': 'function.react_router.middleware', 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'react_router.route.id': 'test-route', + 'react_router.route.pattern': '/users/:id', + 'react_router.middleware.index': 0, }), }), expect.any(Function), ); - // Verify updateRootSpanWithRoute was called (same as loader/action) - // This updates the root span name and sets http.route for parameterized routes expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, 'GET /users/:id'); expect(mockSetAttributes).toHaveBeenCalledWith( expect.objectContaining({ @@ -337,6 +362,153 @@ describe('createSentryServerInstrumentation', () => { ); }); + it('should include middleware index attribute when tracking is available', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + const routeId = 'routes/users-with-middleware'; + + vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue(undefined); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: routeId, + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.middleware(mockCallMiddleware, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: `middleware ${routeId}`, + attributes: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': routeId, + 'react_router.route.pattern': '/users/:id', + 'react_router.middleware.index': 0, + }), + }), + expect.any(Function), + ); + }); + + it('should use middleware function name when available from serverBuild', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + const routeId = 'routes/protected'; + + vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue('authMiddleware'); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: routeId, + index: false, + path: '/protected', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.middleware(mockCallMiddleware, { + request: { method: 'GET', url: 'http://example.com/protected', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/protected', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'middleware authMiddleware', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': routeId, + 'react_router.route.pattern': '/protected', + 'react_router.middleware.name': 'authMiddleware', + 'react_router.middleware.index': 0, + }), + }), + expect.any(Function), + ); + }); + + it('should increment middleware index for multiple middleware calls in same request via OTel context', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + const routeId = 'routes/multi-middleware'; + + // Simulate counter store that would be created by handler and stored in OTel context + const counterStore = { counters: {} as Record }; + + // eslint-disable-next-line @typescript-eslint/unbound-method + vi.mocked(otelApi.context.active).mockReturnValue({ + getValue: vi.fn(() => counterStore), + setValue: vi.fn(), + } as any); + + vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue(undefined); + + const startSpanCalls: any[] = []; + (core.startSpan as any).mockImplementation((opts: any, fn: any) => { + startSpanCalls.push(opts); + return fn(); + }); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: routeId, + index: false, + path: '/multi-middleware', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + const requestInfo = { + request: { method: 'GET', url: 'http://example.com/multi-middleware', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/multi-middleware', + context: undefined, + }; + + // Call middleware 3 times (simulating 3 middlewares on same route) + await hooks.middleware(mockCallMiddleware, requestInfo); + await hooks.middleware(mockCallMiddleware, requestInfo); + await hooks.middleware(mockCallMiddleware, requestInfo); + + // Filter to only middleware spans + const middlewareSpans = startSpanCalls.filter( + opts => opts.attributes?.['sentry.op'] === 'function.react_router.middleware', + ); + + expect(middlewareSpans).toHaveLength(3); + expect(middlewareSpans[0].attributes['react_router.middleware.index']).toBe(0); + expect(middlewareSpans[1].attributes['react_router.middleware.index']).toBe(1); + expect(middlewareSpans[2].attributes['react_router.middleware.index']).toBe(2); + }); + it('should instrument lazy route loading with spans', async () => { const mockCallLazy = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); diff --git a/packages/react-router/test/server/integration/reactRouterServer.test.ts b/packages/react-router/test/server/integration/reactRouterServer.test.ts index a5eac42643e5..53a66b71fb90 100644 --- a/packages/react-router/test/server/integration/reactRouterServer.test.ts +++ b/packages/react-router/test/server/integration/reactRouterServer.test.ts @@ -1,4 +1,4 @@ -import * as SentryNode from '@sentry/node'; +import type { Event } from '@sentry/core'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter'; import { reactRouterServerIntegration } from '../../../src/server/integration/reactRouterServer'; @@ -14,11 +14,6 @@ vi.mock('@sentry/node', () => { generateInstrumentOnce: vi.fn((_name: string, callback: () => any) => { return Object.assign(callback, { id: 'test' }); }), - NODE_VERSION: { - major: 0, - minor: 0, - patch: 0, - }, }; }); @@ -27,39 +22,79 @@ describe('reactRouterServerIntegration', () => { vi.clearAllMocks(); }); - it('sets up ReactRouterInstrumentation for Node 20.18', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 20, minor: 18, patch: 0 }); - + it('sets up ReactRouterInstrumentation on setupOnce', () => { const integration = reactRouterServerIntegration(); integration.setupOnce!(); expect(ReactRouterInstrumentation).toHaveBeenCalled(); }); - it('sets up ReactRouterInstrumentationfor Node.js 22.11', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 22, minor: 11, patch: 0 }); - + it('always sets up ReactRouterInstrumentation to capture ServerBuild for middleware name resolution', () => { + // The instrumentation is always installed to capture the ServerBuild reference, + // which is needed for middleware name resolution. The OTEL wrapper internally + // skips creating loader/action spans when the instrumentation API is active. const integration = reactRouterServerIntegration(); integration.setupOnce!(); - expect(ReactRouterInstrumentation).toHaveBeenCalled(); + expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1); }); - it('does not set up ReactRouterInstrumentation for Node.js 20.19', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 20, minor: 19, patch: 0 }); - + it('has processEvent that removes bogus http.route attribute', () => { const integration = reactRouterServerIntegration(); - integration.setupOnce!(); - expect(ReactRouterInstrumentation).not.toHaveBeenCalled(); - }); + // Test with bogus * route and non-* transaction name + const event1 = { + type: 'transaction' as const, + contexts: { + trace: { + span_id: 'test-span-id', + trace_id: 'test-trace-id', + data: { + 'http.route': '*', + }, + }, + }, + transaction: 'GET /users', + } as Event; - it('does not set up ReactRouterInstrumentation for Node.js 22.12', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 22, minor: 12, patch: 0 }); + const result1 = integration.processEvent!(event1, {}, {} as any) as Event | null; + expect(result1?.contexts?.trace?.data?.['http.route']).toBeUndefined(); - const integration = reactRouterServerIntegration(); - integration.setupOnce!(); + // Test with bogus * route but GET * transaction name (should keep) + const event2 = { + type: 'transaction' as const, + contexts: { + trace: { + span_id: 'test-span-id', + trace_id: 'test-trace-id', + data: { + 'http.route': '*', + }, + }, + }, + transaction: 'GET *', + } as Event; + + const result2 = integration.processEvent!(event2, {}, {} as any) as Event | null; + expect(result2?.contexts?.trace?.data?.['http.route']).toBe('*'); + + // Test with instrumentation_api origin (should always remove) + const event3 = { + type: 'transaction' as const, + contexts: { + trace: { + span_id: 'test-span-id', + trace_id: 'test-trace-id', + origin: 'auto.http.react_router.instrumentation_api', + data: { + 'http.route': '*', + }, + }, + }, + transaction: 'GET *', + } as Event; - expect(ReactRouterInstrumentation).not.toHaveBeenCalled(); + const result3 = integration.processEvent!(event3, {}, {} as any) as Event | null; + expect(result3?.contexts?.trace?.data?.['http.route']).toBeUndefined(); }); }); diff --git a/packages/react-router/test/server/serverBuild.test.ts b/packages/react-router/test/server/serverBuild.test.ts new file mode 100644 index 000000000000..ec95953d5b5e --- /dev/null +++ b/packages/react-router/test/server/serverBuild.test.ts @@ -0,0 +1,188 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { _resetServerBuild, getMiddlewareName, isServerBuildLike, setServerBuild } from '../../src/server/serverBuild'; + +describe('serverBuild', () => { + afterEach(() => { + _resetServerBuild(); + }); + + describe('setServerBuild', () => { + it('should store the build reference', () => { + const mockBuild = { + routes: { + 'test-route': { + id: 'test-route', + module: { + middleware: [{ name: 'testMiddleware' }], + }, + }, + }, + }; + + setServerBuild(mockBuild); + + expect(getMiddlewareName('test-route', 0)).toBe('testMiddleware'); + }); + }); + + describe('getMiddlewareName', () => { + it('should return undefined when no build is stored', () => { + expect(getMiddlewareName('any-route', 0)).toBeUndefined(); + }); + + it('should return undefined when routes is undefined', () => { + setServerBuild({}); + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return undefined when route does not exist', () => { + setServerBuild({ + routes: { + 'other-route': { module: { middleware: [{ name: 'test' }] } }, + }, + }); + + expect(getMiddlewareName('non-existent-route', 0)).toBeUndefined(); + }); + + it('should return undefined when route has no module', () => { + setServerBuild({ + routes: { + 'test-route': {}, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return undefined when module has no middleware', () => { + setServerBuild({ + routes: { + 'test-route': { module: {} }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return undefined when middleware array is empty', () => { + setServerBuild({ + routes: { + 'test-route': { module: { middleware: [] } }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return undefined when index is out of bounds', () => { + setServerBuild({ + routes: { + 'test-route': { module: { middleware: [{ name: 'first' }] } }, + }, + }); + + expect(getMiddlewareName('test-route', 1)).toBeUndefined(); + }); + + it('should return undefined when middleware function has no name', () => { + setServerBuild({ + routes: { + 'test-route': { module: { middleware: [{}] } }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return undefined when middleware function name is empty string', () => { + setServerBuild({ + routes: { + 'test-route': { module: { middleware: [{ name: '' }] } }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + + it('should return the middleware function name', () => { + setServerBuild({ + routes: { + 'test-route': { + module: { + middleware: [{ name: 'authMiddleware' }, { name: 'loggingMiddleware' }], + }, + }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBe('authMiddleware'); + expect(getMiddlewareName('test-route', 1)).toBe('loggingMiddleware'); + }); + + it('should work with multiple routes', () => { + setServerBuild({ + routes: { + 'route-a': { module: { middleware: [{ name: 'middlewareA' }] } }, + 'route-b': { module: { middleware: [{ name: 'middlewareB' }] } }, + }, + }); + + expect(getMiddlewareName('route-a', 0)).toBe('middlewareA'); + expect(getMiddlewareName('route-b', 0)).toBe('middlewareB'); + }); + }); + + describe('_resetServerBuild', () => { + it('should clear the stored build reference', () => { + setServerBuild({ + routes: { + 'test-route': { module: { middleware: [{ name: 'test' }] } }, + }, + }); + + expect(getMiddlewareName('test-route', 0)).toBe('test'); + + _resetServerBuild(); + + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + }); + }); + + describe('isServerBuildLike', () => { + it('should return true for objects with routes property', () => { + expect(isServerBuildLike({ routes: {} })).toBe(true); + expect(isServerBuildLike({ routes: { 'test-route': {} } })).toBe(true); + }); + + it('should return false for null', () => { + expect(isServerBuildLike(null)).toBe(false); + }); + + it('should return false for undefined', () => { + expect(isServerBuildLike(undefined)).toBe(false); + }); + + it('should return false for primitives', () => { + expect(isServerBuildLike('string')).toBe(false); + expect(isServerBuildLike(123)).toBe(false); + expect(isServerBuildLike(true)).toBe(false); + }); + + it('should return false for objects without routes property', () => { + expect(isServerBuildLike({})).toBe(false); + expect(isServerBuildLike({ other: 'property' })).toBe(false); + }); + + it('should return false when routes is not an object', () => { + expect(isServerBuildLike({ routes: 'string' })).toBe(false); + expect(isServerBuildLike({ routes: 123 })).toBe(false); + expect(isServerBuildLike({ routes: null })).toBe(false); + }); + + it('should return false for arrays', () => { + expect(isServerBuildLike([])).toBe(false); + expect(isServerBuildLike([{ routes: {} }])).toBe(false); + }); + }); +});