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..f0c389733cfa 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,7 @@ 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('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..1ad5947d8bc9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/multi-middleware.tsx @@ -0,0 +1,26 @@ +import type { Route } from './+types/multi-middleware'; + +export const middleware: Route.MiddlewareFunction[] = [ + async function authMiddleware(_args, next) { + return next(); + }, + async function loggingMiddleware(_args, next) { + return next(); + }, + async function validationMiddleware(_args, next) { + return next(); + }, +]; + +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/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..c0c2248014a8 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,8 +2,6 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from '../constants'; -// 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', () => { test('should instrument server middleware with instrumentation API origin', async ({ page }) => { const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { @@ -43,20 +41,27 @@ test.describe('server - instrumentation API middleware', () => { (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware', ); + expect(middlewareSpan).toBeDefined(); expect(middlewareSpan).toMatchObject({ span_id: expect.any(String), trace_id: expect.any(String), - data: { + data: expect.objectContaining({ 'sentry.origin': 'auto.function.react_router.instrumentation_api', 'sentry.op': 'function.react_router.middleware', - }, - description: '/performance/with-middleware', + 'react_router.route.id': 'routes/performance/with-middleware', + 'react_router.route.pattern': '/performance/with-middleware', + 'react_router.middleware.index': 0, + }), parent_span_id: expect.any(String), start_timestamp: expect.any(Number), timestamp: expect.any(Number), op: 'function.react_router.middleware', origin: 'auto.function.react_router.instrumentation_api', }); + + // Middleware name is available via OTEL patching of createRequestHandler + expect(middlewareSpan!.data?.['react_router.middleware.name']).toBe('authMiddleware'); + expect(middlewareSpan!.description).toBe('middleware authMiddleware'); }); test('should have middleware span run before loader span', async ({ page }) => { @@ -80,6 +85,37 @@ test.describe('server - instrumentation API middleware', () => { 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', 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; + + await expect(page.locator('#multi-middleware-title')).toBeVisible(); + await expect(page.locator('#multi-middleware-content')).toHaveText('This route has 3 middlewares'); + + const middlewareSpans = transaction?.spans?.filter( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware', + ); + + expect(middlewareSpans).toHaveLength(3); + + const sortedSpans = [...middlewareSpans!].sort( + (a: any, b: any) => + (a.data?.['react_router.middleware.index'] ?? 0) - (b.data?.['react_router.middleware.index'] ?? 0), + ); + + expect(sortedSpans.map((s: any) => s.data?.['react_router.middleware.index'])).toEqual([0, 1, 2]); + expect(sortedSpans.map((s: any) => s.data?.['react_router.middleware.name'])).toEqual([ + 'authMiddleware', + 'loggingMiddleware', + 'validationMiddleware', + ]); }); }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts index 68ba30d69397..4da306d41cc7 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts @@ -1,6 +1,11 @@ import { reactRouter } from '@react-router/dev/vite'; +import { sentryReactRouter } from '@sentry/react-router'; import { defineConfig } from 'vite'; -export default defineConfig({ - plugins: [reactRouter()], -}); +export default defineConfig(async config => ({ + plugins: [ + reactRouter(), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...((await sentryReactRouter({ sourcemaps: { disable: true } }, config)) as any[]), + ], +})); diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index c465a25dd662..a200ac2fdff5 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -19,6 +19,9 @@ 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; +// Per-request middleware counters, keyed by Request +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 +217,8 @@ export function createSentryClientInstrumentation( }, route(route: InstrumentableRoute) { + const routeId = route.id; + route.instrument({ async loader(callLoader, info) { const urlPath = getPathFromRequest(info.request); @@ -267,12 +272,24 @@ export function createSentryClientInstrumentation( const urlPath = getPathFromRequest(info.request); const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + let counters = middlewareCountersMap.get(info.request); + if (!counters) { + counters = {}; + middlewareCountersMap.set(info.request, counters); + } + + const middlewareIndex = counters[routeId] ?? 0; + counters[routeId] = middlewareIndex + 1; + await startSpan( { - name: routePattern, + name: `middleware ${routeId}`, 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, + 'react_router.middleware.index': middlewareIndex, }, }, async span => { diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts index 3fceca6a4ff7..ace258b8bc31 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,11 @@ 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'; +const MIDDLEWARE_COUNTER_KEY = createContextKey('sentry_react_router_middleware_counter'); + // Re-export for backward compatibility and external use export { isInstrumentationApiUsed } from './serverGlobals'; @@ -53,61 +56,68 @@ export function createSentryServerInstrumentation( const activeSpan = getActiveSpan(); const existingRootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; - if (existingRootSpan) { - updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); - existingRootSpan.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - }); + const counterStore = { counters: {} as Record }; + const ctx = context.active().setValue(MIDDLEWARE_COUNTER_KEY, counterStore); - 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 () => { + if (existingRootSpan) { + updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); + existingRootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [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, + }); + } + } 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, + } 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, + }, }, - }, - 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 +178,29 @@ 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 + | { counters: Record } + | undefined; + let middlewareIndex = 0; + if (counterStore) { + middlewareIndex = counterStore.counters[routeId] ?? 0; + counterStore.counters[routeId] = middlewareIndex + 1; + } + + const middlewareName = getMiddlewareName(routeId, middlewareIndex); + await startSpan( { - name: routePattern, + name: `middleware ${middlewareName || routeId}`, 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..cbca773f6f13 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,29 @@ export class ReactRouterInstrumentation extends InstrumentationBase unknown; + args[0] = async function sentryWrappedBuildFn() { + const resolvedBuild = await originalBuildFn(); + if (isServerBuildLike(resolvedBuild)) { + setServerBuild(resolvedBuild); + } + return resolvedBuild; + }; + } + const originalRequestHandler = original.apply(this, args); + // Skip per-request wrapping when instrumentation API is active + if (isInstrumentationApiUsed()) { + return originalRequestHandler; + } + return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) { let url: URL; try { @@ -77,13 +99,6 @@ export class ReactRouterInstrumentation extends InstrumentationBase { export const instrumentReactRouterServer = Object.assign( (): void => { instrumentReactRouter(); + // Register global for Vite plugin ServerBuild capture + registerServerBuildGlobal(); }, { id: INTEGRATION_NAME }, ); @@ -24,17 +26,9 @@ export const reactRouterServerIntegration = defineIntegration(() => { 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 to capture ServerBuild for middleware names. + // Skips per-request wrapping when instrumentation API is active. + 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..3b46c745b4b7 --- /dev/null +++ b/packages/react-router/src/server/serverBuild.ts @@ -0,0 +1,66 @@ +import { GLOBAL_OBJ } from '@sentry/core'; + +/** + * Subset of ServerBuild shape for middleware name lookup. + * The official React Router types don't expose `middleware` on route modules yet. + * @internal + */ +interface ServerBuildLike { + routes?: Record< + string, + { + module?: { + middleware?: Array<{ name?: string }>; + }; + } + >; +} + +/** @internal */ +export const GLOBAL_KEY = '__sentrySetServerBuild'; + +type GlobalObjWithBuildCapture = typeof GLOBAL_OBJ & { + [GLOBAL_KEY]?: (build: ServerBuildLike) => void; +}; + +// ServerBuild reference for middleware name lookup. Updated on each createRequestHandler call. +let _serverBuild: ServerBuildLike | undefined; + +/** @internal */ +export function isServerBuildLike(build: unknown): build is ServerBuildLike { + return ( + build !== null && + typeof build === 'object' && + 'routes' in build && + build.routes !== null && + typeof build.routes === 'object' + ); +} + +/** @internal */ +export function setServerBuild(build: ServerBuildLike): void { + _serverBuild = build; +} + +/** @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; +} + +/** @internal */ +export function registerServerBuildGlobal(): void { + (GLOBAL_OBJ as GlobalObjWithBuildCapture)[GLOBAL_KEY] = setServerBuild; +} + +/** @internal Exported for testing. */ +export function _resetServerBuild(): void { + _serverBuild = undefined; + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete (GLOBAL_OBJ as GlobalObjWithBuildCapture)[GLOBAL_KEY]; +} diff --git a/packages/react-router/src/vite/makeServerBuildCapturePlugin.ts b/packages/react-router/src/vite/makeServerBuildCapturePlugin.ts new file mode 100644 index 000000000000..e7d081306bfb --- /dev/null +++ b/packages/react-router/src/vite/makeServerBuildCapturePlugin.ts @@ -0,0 +1,39 @@ +import { type Plugin } from 'vite'; +import { GLOBAL_KEY } from '../server/serverBuild'; + +const SERVER_BUILD_MODULE_ID = 'virtual:react-router/server-build'; + +/** + * A Sentry plugin for React Router to capture the server build for middleware name resolution. + */ +export function makeServerBuildCapturePlugin(): Plugin { + let isSsrBuild = false; + + return { + name: 'sentry-react-router-server-build-capture', + enforce: 'post', + + configResolved(config) { + isSsrBuild = !!config.build.ssr; + }, + + transform(code, id) { + if (!isSsrBuild) { + return null; + } + + if (!id.includes(SERVER_BUILD_MODULE_ID)) { + return null; + } + + // `routes` is a module-scope export in the virtual:react-router/server-build module + const injectedCode = `${code} +if (typeof globalThis !== 'undefined' && typeof globalThis["${GLOBAL_KEY}"] === 'function') { + globalThis["${GLOBAL_KEY}"]({ routes }); +} +`; + + return { code: injectedCode, map: null }; + }, + }; +} diff --git a/packages/react-router/src/vite/plugin.ts b/packages/react-router/src/vite/plugin.ts index d58b08df3fa2..4a66a2575987 100644 --- a/packages/react-router/src/vite/plugin.ts +++ b/packages/react-router/src/vite/plugin.ts @@ -2,6 +2,7 @@ import type { ConfigEnv, Plugin } from 'vite'; import { makeConfigInjectorPlugin } from './makeConfigInjectorPlugin'; import { makeCustomSentryVitePlugins } from './makeCustomSentryVitePlugins'; import { makeEnableSourceMapsPlugin } from './makeEnableSourceMapsPlugin'; +import { makeServerBuildCapturePlugin } from './makeServerBuildCapturePlugin'; import type { SentryReactRouterBuildOptions } from './types'; /** @@ -18,6 +19,7 @@ export async function sentryReactRouter( const plugins: Plugin[] = []; plugins.push(makeConfigInjectorPlugin(options)); + plugins.push(makeServerBuildCapturePlugin()); if (process.env.NODE_ENV !== 'development' && viteConfig.command === 'build' && viteConfig.mode !== 'development') { plugins.push(makeEnableSourceMapsPlugin(options)); diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index 0078b2601c51..8cb5a125b147 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -478,10 +478,13 @@ 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), diff --git a/packages/react-router/test/server/createServerInstrumentation.test.ts b/packages/react-router/test/server/createServerInstrumentation.test.ts index 33eb73f48ace..427995a1810e 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,47 +308,65 @@ describe('createSentryServerInstrumentation', () => { ); }); - it('should instrument route middleware with spans', async () => { + async function callMiddlewareHook(options: { + middlewareName: string | undefined; + routeId: string; + routePath: string; + url: string; + }) { const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); const mockSetAttributes = vi.fn(); const mockRootSpan = { setAttributes: mockSetAttributes }; + vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue(options.middlewareName); + (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: 'test-route', + id: options.routeId, index: false, - path: '/users/:id', + path: options.routePath, instrument: mockInstrument, }); const hooks = mockInstrument.mock.calls[0]![0]; - // Call the middleware hook with RouteHandlerInstrumentationInfo await hooks.middleware(mockCallMiddleware, { - request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, - params: { id: '123' }, - unstable_pattern: '/users/:id', + request: { method: 'GET', url: options.url, headers: { get: () => null } }, + params: {}, + unstable_pattern: options.routePath, context: undefined, }); + return { mockSetAttributes, mockRootSpan }; + } + + it('should instrument route middleware with spans (without function name)', async () => { + const { mockSetAttributes, mockRootSpan } = await callMiddlewareHook({ + middlewareName: undefined, + routeId: 'test-route', + routePath: '/users/:id', + url: 'http://example.com/users/123', + }); + 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 +376,87 @@ describe('createSentryServerInstrumentation', () => { ); }); + it('should use middleware function name when available from serverBuild', async () => { + await callMiddlewareHook({ + middlewareName: 'authMiddleware', + routeId: 'routes/protected', + routePath: '/protected', + url: 'http://example.com/protected', + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'middleware authMiddleware', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react_router.middleware', + 'react_router.route.id': 'routes/protected', + '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 on same route', 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/instrumentation/reactRouterServer.test.ts b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts index 93e0a91a1c2b..a4156ebdde05 100644 --- a/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts +++ b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts @@ -3,6 +3,8 @@ import * as SentryCore from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter'; import * as Util from '../../../src/server/instrumentation/util'; +import * as ServerBuild from '../../../src/server/serverBuild'; +import * as ServerGlobals from '../../../src/server/serverGlobals'; vi.mock('@sentry/core', async () => { return { @@ -117,4 +119,40 @@ describe('ReactRouterInstrumentation', () => { expect(originalHandler).toHaveBeenCalledWith(req, undefined); }); + + it('should call setServerBuild when static ServerBuild is passed', () => { + const spy = vi.spyOn(ServerBuild, 'setServerBuild'); + vi.spyOn(ServerBuild, 'isServerBuildLike').mockReturnValue(true); + + const staticBuild = { routes: { root: { id: 'root' } } }; + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + proxy.createRequestHandler(staticBuild); + + expect(spy).toHaveBeenCalledWith(staticBuild); + }); + + it('should capture ServerBuild from factory function', async () => { + const resolvedBuild = { routes: { root: { id: 'root' } } }; + const buildFactory = vi.fn().mockResolvedValue(resolvedBuild); + vi.spyOn(ServerBuild, 'isServerBuildLike').mockImplementation(val => val === resolvedBuild); + const spy = vi.spyOn(ServerBuild, 'setServerBuild'); + + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + proxy.createRequestHandler(buildFactory); + + // Factory gets wrapped — invoke it via the arg passed to the original createRequestHandler + const wrappedFactory = mockModule.createRequestHandler.mock.calls[0][0]; + await wrappedFactory(); + + expect(spy).toHaveBeenCalledWith(resolvedBuild); + }); + + it('should return original handler without wrapping when instrumentation API is active', () => { + vi.spyOn(ServerGlobals, 'isInstrumentationApiUsed').mockReturnValue(true); + + const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule); + const handler = proxy.createRequestHandler(); + + expect(handler).toBe(originalHandler); + }); }); diff --git a/packages/react-router/test/server/integration/reactRouterServer.test.ts b/packages/react-router/test/server/integration/reactRouterServer.test.ts index a5eac42643e5..b5a1f02f99a3 100644 --- a/packages/react-router/test/server/integration/reactRouterServer.test.ts +++ b/packages/react-router/test/server/integration/reactRouterServer.test.ts @@ -1,7 +1,7 @@ -import * as SentryNode from '@sentry/node'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter'; import { reactRouterServerIntegration } from '../../../src/server/integration/reactRouterServer'; +import * as serverBuild from '../../../src/server/serverBuild'; vi.mock('../../../src/server/instrumentation/reactRouter', () => { return { @@ -14,52 +14,32 @@ 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, - }, }; }); describe('reactRouterServerIntegration', () => { + let registerServerBuildGlobalSpy: ReturnType; + beforeEach(() => { vi.clearAllMocks(); + registerServerBuildGlobalSpy = vi.spyOn(serverBuild, 'registerServerBuildGlobal'); }); - it('sets up ReactRouterInstrumentation for Node 20.18', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 20, minor: 18, patch: 0 }); - - const integration = reactRouterServerIntegration(); - integration.setupOnce!(); - - expect(ReactRouterInstrumentation).toHaveBeenCalled(); + afterEach(() => { + vi.restoreAllMocks(); }); - it('sets up ReactRouterInstrumentationfor Node.js 22.11', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 22, minor: 11, patch: 0 }); - + it('sets up ReactRouterInstrumentation on setupOnce', () => { 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 }); - - const integration = reactRouterServerIntegration(); - integration.setupOnce!(); - - expect(ReactRouterInstrumentation).not.toHaveBeenCalled(); - }); - - it('does not set up ReactRouterInstrumentation for Node.js 22.12', () => { - vi.spyOn(SentryNode, 'NODE_VERSION', 'get').mockReturnValue({ major: 22, minor: 12, patch: 0 }); - + it('registers the server build global callback on setupOnce', () => { const integration = reactRouterServerIntegration(); integration.setupOnce!(); - expect(ReactRouterInstrumentation).not.toHaveBeenCalled(); + expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1); }); }); 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..80eb7e2028b7 --- /dev/null +++ b/packages/react-router/test/server/serverBuild.test.ts @@ -0,0 +1,77 @@ +import { GLOBAL_OBJ } from '@sentry/core'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + _resetServerBuild, + getMiddlewareName, + isServerBuildLike, + registerServerBuildGlobal, + setServerBuild, +} from '../../src/server/serverBuild'; + +describe('serverBuild', () => { + afterEach(() => { + _resetServerBuild(); + }); + + describe('getMiddlewareName', () => { + it('should return undefined when build is missing or incomplete', () => { + expect(getMiddlewareName('any-route', 0)).toBeUndefined(); + + setServerBuild({ routes: { 'test-route': {} } }); + expect(getMiddlewareName('test-route', 0)).toBeUndefined(); + + setServerBuild({ routes: { 'test-route': { module: { middleware: [{ name: 'first' }] } } } }); + expect(getMiddlewareName('test-route', 1)).toBeUndefined(); + }); + + it('should return the middleware function name by index', () => { + setServerBuild({ + routes: { + 'route-a': { module: { middleware: [{ name: 'authMiddleware' }, { name: 'loggingMiddleware' }] } }, + }, + }); + + expect(getMiddlewareName('route-a', 0)).toBe('authMiddleware'); + expect(getMiddlewareName('route-a', 1)).toBe('loggingMiddleware'); + }); + + it('should return undefined for empty-string middleware names', () => { + setServerBuild({ + routes: { + 'route-a': { module: { middleware: [{ name: '' }] } }, + }, + }); + + expect(getMiddlewareName('route-a', 0)).toBeUndefined(); + }); + }); + + describe('isServerBuildLike', () => { + it('should return true for objects with a routes object', () => { + expect(isServerBuildLike({ routes: {} })).toBe(true); + expect(isServerBuildLike({ routes: { 'test-route': {} } })).toBe(true); + }); + + it('should return false for non-build values', () => { + expect(isServerBuildLike(null)).toBe(false); + expect(isServerBuildLike(undefined)).toBe(false); + expect(isServerBuildLike({})).toBe(false); + expect(isServerBuildLike({ routes: null })).toBe(false); + expect(isServerBuildLike({ routes: 'string' })).toBe(false); + }); + }); + + describe('registerServerBuildGlobal', () => { + it('should register a global callback that calls setServerBuild', () => { + registerServerBuildGlobal(); + + const callback = (GLOBAL_OBJ as any).__sentrySetServerBuild; + expect(typeof callback).toBe('function'); + + const build = { routes: { 'test-route': { module: { middleware: [{ name: 'testMiddleware' }] } } } }; + callback(build); + + expect(getMiddlewareName('test-route', 0)).toBe('testMiddleware'); + }); + }); +}); diff --git a/packages/react-router/test/vite/makeServerBuildCapturePlugin.test.ts b/packages/react-router/test/vite/makeServerBuildCapturePlugin.test.ts new file mode 100644 index 000000000000..f76090b60f56 --- /dev/null +++ b/packages/react-router/test/vite/makeServerBuildCapturePlugin.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import { makeServerBuildCapturePlugin } from '../../src/vite/makeServerBuildCapturePlugin'; + +const SERVER_BUILD_MODULE_ID = 'virtual:react-router/server-build'; +const SERVER_BUILD_CODE = 'const routes = {}; export { routes, entry, future };'; + +describe('makeServerBuildCapturePlugin', () => { + it('should create a plugin with the correct name and enforce post', () => { + const plugin = makeServerBuildCapturePlugin(); + expect(plugin.name).toBe('sentry-react-router-server-build-capture'); + expect(plugin.enforce).toBe('post'); + }); + + it('should return null for non-SSR builds', () => { + const plugin = makeServerBuildCapturePlugin(); + (plugin as any).configResolved({ build: { ssr: false } }); + + const result = (plugin as any).transform(SERVER_BUILD_CODE, SERVER_BUILD_MODULE_ID); + + expect(result).toBeNull(); + }); + + it('should return null for non-server-build modules in SSR mode', () => { + const plugin = makeServerBuildCapturePlugin(); + (plugin as any).configResolved({ build: { ssr: true } }); + + const result = (plugin as any).transform('export function helper() {}', 'src/utils.ts'); + + expect(result).toBeNull(); + }); + + it('should inject capture snippet into the server build module in SSR mode', () => { + const plugin = makeServerBuildCapturePlugin(); + (plugin as any).configResolved({ build: { ssr: true } }); + + const result = (plugin as any).transform(SERVER_BUILD_CODE, SERVER_BUILD_MODULE_ID); + + expect(result).not.toBeNull(); + expect(result.code).toContain(SERVER_BUILD_CODE); + expect(result.code).toContain('__sentrySetServerBuild'); + expect(result.code).toContain('({ routes })'); + expect(result.map).toBeNull(); + }); +}); diff --git a/packages/react-router/test/vite/plugin.test.ts b/packages/react-router/test/vite/plugin.test.ts index f01254ca8869..52306eb0dbd1 100644 --- a/packages/react-router/test/vite/plugin.test.ts +++ b/packages/react-router/test/vite/plugin.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { makeConfigInjectorPlugin } from '../../src/vite/makeConfigInjectorPlugin'; import { makeCustomSentryVitePlugins } from '../../src/vite/makeCustomSentryVitePlugins'; import { makeEnableSourceMapsPlugin } from '../../src/vite/makeEnableSourceMapsPlugin'; +import { makeServerBuildCapturePlugin } from '../../src/vite/makeServerBuildCapturePlugin'; import { sentryReactRouter } from '../../src/vite/plugin'; vi.spyOn(console, 'log').mockImplementation(() => { @@ -14,17 +15,20 @@ vi.spyOn(console, 'warn').mockImplementation(() => { vi.mock('../../src/vite/makeCustomSentryVitePlugins'); vi.mock('../../src/vite/makeEnableSourceMapsPlugin'); vi.mock('../../src/vite/makeConfigInjectorPlugin'); +vi.mock('../../src/vite/makeServerBuildCapturePlugin'); describe('sentryReactRouter', () => { const mockPlugins = [{ name: 'test-plugin' }]; const mockSourceMapsPlugin = { name: 'source-maps-plugin' }; const mockConfigInjectorPlugin = { name: 'sentry-config-injector' }; + const mockServerBuildCapturePlugin = { name: 'sentry-react-router-server-build-capture' }; beforeEach(() => { vi.clearAllMocks(); vi.mocked(makeCustomSentryVitePlugins).mockResolvedValue(mockPlugins); vi.mocked(makeEnableSourceMapsPlugin).mockReturnValue(mockSourceMapsPlugin); vi.mocked(makeConfigInjectorPlugin).mockReturnValue(mockConfigInjectorPlugin); + vi.mocked(makeServerBuildCapturePlugin).mockReturnValue(mockServerBuildCapturePlugin); }); afterEach(() => { @@ -37,7 +41,7 @@ describe('sentryReactRouter', () => { const result = await sentryReactRouter({}, { command: 'build', mode: 'production' }); - expect(result).toEqual([mockConfigInjectorPlugin]); + expect(result).toEqual([mockConfigInjectorPlugin, mockServerBuildCapturePlugin]); expect(makeCustomSentryVitePlugins).not.toHaveBeenCalled(); expect(makeEnableSourceMapsPlugin).not.toHaveBeenCalled(); @@ -47,7 +51,7 @@ describe('sentryReactRouter', () => { it('should return config injector plugin when not in build mode', async () => { const result = await sentryReactRouter({}, { command: 'serve', mode: 'production' }); - expect(result).toEqual([mockConfigInjectorPlugin]); + expect(result).toEqual([mockConfigInjectorPlugin, mockServerBuildCapturePlugin]); expect(makeCustomSentryVitePlugins).not.toHaveBeenCalled(); expect(makeEnableSourceMapsPlugin).not.toHaveBeenCalled(); }); @@ -55,7 +59,7 @@ describe('sentryReactRouter', () => { it('should return config injector plugin in development build mode', async () => { const result = await sentryReactRouter({}, { command: 'build', mode: 'development' }); - expect(result).toEqual([mockConfigInjectorPlugin]); + expect(result).toEqual([mockConfigInjectorPlugin, mockServerBuildCapturePlugin]); expect(makeCustomSentryVitePlugins).not.toHaveBeenCalled(); expect(makeEnableSourceMapsPlugin).not.toHaveBeenCalled(); }); @@ -66,8 +70,14 @@ describe('sentryReactRouter', () => { const result = await sentryReactRouter({}, { command: 'build', mode: 'production' }); - expect(result).toEqual([mockConfigInjectorPlugin, mockSourceMapsPlugin, ...mockPlugins]); + expect(result).toEqual([ + mockConfigInjectorPlugin, + mockServerBuildCapturePlugin, + mockSourceMapsPlugin, + ...mockPlugins, + ]); expect(makeConfigInjectorPlugin).toHaveBeenCalledWith({}); + expect(makeServerBuildCapturePlugin).toHaveBeenCalled(); expect(makeCustomSentryVitePlugins).toHaveBeenCalledWith({}); expect(makeEnableSourceMapsPlugin).toHaveBeenCalledWith({});