diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts index 5386c75f31a9..d471308e58ec 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts @@ -21,6 +21,11 @@ test('Should create a transaction for middleware', async ({ request }) => { expect(middlewareTransaction.contexts?.runtime?.name).toBe('node'); expect(middlewareTransaction.transaction_info?.source).toBe('route'); + // The `Middleware.execute` OTEL root span is the only middleware span. The build-time + // `wrapMiddlewareWithSentry` wrapper used to start a second, redundant one nested inside it. + const nestedMiddlewareSpans = middlewareTransaction.spans?.filter(span => span.op === 'http.server.middleware'); + expect(nestedMiddlewareSpans).toHaveLength(0); + // Assert that isolation scope works properly expect(middlewareTransaction.tags?.['my-isolated-tag']).toBe(true); expect(middlewareTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts index d383837cbf17..81bdb0d3eb59 100644 --- a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts +++ b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts @@ -19,6 +19,12 @@ import type { EdgeRouteHandler } from '../edge/types'; /** * Wraps Next.js middleware with Sentry error and performance instrumentation. * + * From Next.js 14 onwards the middleware transaction is created by Next.js' native OpenTelemetry + * instrumentation (the `Middleware.execute` span, normalized by `enhanceMiddlewareRootSpan`). In that case this + * wrapper does not start a span of its own, as that would emit a second, redundant middleware span nested inside + * the root span. It only forks an isolation scope, captures errors, and flushes. Next.js 13 does not emit + * `Middleware.execute`, so there the wrapper still starts the transaction itself. + * * @param middleware The middleware handler. * @returns a wrapped middleware handler. */ @@ -32,6 +38,7 @@ export function wrapMiddlewareWithSentry( ? (globalThis as Record)._sentryRewritesTunnelPath : undefined; + // TODO: This can never work with Turbopack, need to remove it for consistency between builds. if (tunnelRoute && typeof tunnelRoute === 'string') { const req: unknown = args[0]; // Check if the current request matches the tunnel route @@ -52,6 +59,7 @@ export function wrapMiddlewareWithSentry( } } } + // TODO: We still should add central isolation scope creation for when our build-time instrumentation does not work anymore with turbopack. return withIsolationScope(isolationScope => { const req: unknown = args[0]; @@ -73,20 +81,37 @@ export function wrapMiddlewareWithSentry( currentScope.setTransactionName(spanName); - const activeSpan = getActiveSpan(); + const runMiddleware = (): ReturnType => + handleCallbackErrors( + () => wrappingTarget.apply(thisArg, args), + error => { + captureException(error, { + mechanism: { + type: 'auto.function.nextjs.wrap_middleware', + handled: false, + }, + }); + }, + () => { + waitUntil(flushSafelyWithTimeout()); + }, + ) as ReturnType; + const activeSpan = getActiveSpan(); if (activeSpan) { - // If there is an active span, it likely means that the automatic Next.js OTEL instrumentation worked and we can - // rely on that for parameterization. - spanName = 'middleware'; - spanSource = 'component'; - + // The native Next.js OTEL instrumentation created the middleware root span (`Middleware.execute`, + // normalized by `enhanceMiddlewareRootSpan`). Bind our forked scopes to it so the transaction picks up + // the isolation scope instead of the global one, and do not start a second, redundant span here. const rootSpan = getRootSpan(activeSpan); if (rootSpan) { setCapturedScopesOnSpan(rootSpan, currentScope, isolationScope); } + + return runMiddleware(); } + // Next.js only emits `Middleware.execute` from version 14 onwards. On Next.js 13 nothing else creates a + // middleware span, so this wrapper still has to provide the transaction itself. return startSpan( { name: spanName, @@ -96,22 +121,7 @@ export function wrapMiddlewareWithSentry( [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.wrap_middleware', }, }, - () => { - return handleCallbackErrors( - () => wrappingTarget.apply(thisArg, args), - error => { - captureException(error, { - mechanism: { - type: 'auto.function.nextjs.wrap_middleware', - handled: false, - }, - }); - }, - () => { - waitUntil(flushSafelyWithTimeout()); - }, - ); - }, + runMiddleware, ); }); }, diff --git a/packages/nextjs/test/common/wrapMiddlewareWithSentry.test.ts b/packages/nextjs/test/common/wrapMiddlewareWithSentry.test.ts new file mode 100644 index 000000000000..808666694d3d --- /dev/null +++ b/packages/nextjs/test/common/wrapMiddlewareWithSentry.test.ts @@ -0,0 +1,90 @@ +import * as SentryCore from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { wrapMiddlewareWithSentry } from '../../src/common/wrapMiddlewareWithSentry'; + +describe('wrapMiddlewareWithSentry', () => { + beforeEach(() => { + vi.spyOn(SentryCore, 'captureException').mockReturnValue(''); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('does not start its own span when the Next.js OTEL root span is already active (Next.js >= 14)', async () => { + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue({} as SentryCore.Span); + vi.spyOn(SentryCore, 'getRootSpan').mockReturnValue({} as SentryCore.Span); + const setCapturedScopesSpy = vi.spyOn(SentryCore, 'setCapturedScopesOnSpan').mockReturnValue(undefined); + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); + + const handler = vi.fn(async (_req: Request) => new Response('ok')); + const wrapped = wrapMiddlewareWithSentry(handler); + + await wrapped(new Request('https://example.com/foo', { method: 'GET' })); + + // The middleware runs and our forked scopes are bound to the existing OTEL root span... + expect(handler).toHaveBeenCalledTimes(1); + expect(setCapturedScopesSpy).toHaveBeenCalledTimes(1); + // ...but the wrapper never starts a span itself - the `Middleware.execute` span is the transaction. + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('starts its own span when no span is active (Next.js 13)', async () => { + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(undefined); + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); + + const handler = vi.fn(async (_req: Request) => new Response('ok')); + const wrapped = wrapMiddlewareWithSentry(handler); + + await wrapped(new Request('https://example.com/foo', { method: 'GET' })); + + expect(handler).toHaveBeenCalledTimes(1); + // Next.js 13 never emits `Middleware.execute`, so without this span there would be no middleware transaction. + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'middleware GET', + op: 'http.server.middleware', + }), + expect.any(Function), + ); + }); + + it('captures errors thrown by the middleware when a root span is already active', async () => { + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue({} as SentryCore.Span); + vi.spyOn(SentryCore, 'getRootSpan').mockReturnValue({} as SentryCore.Span); + vi.spyOn(SentryCore, 'setCapturedScopesOnSpan').mockReturnValue(undefined); + const captureExceptionSpy = vi.spyOn(SentryCore, 'captureException').mockReturnValue(''); + + const error = new Error('boom'); + const handler = vi.fn(async (_req: Request) => { + throw error; + }); + const wrapped = wrapMiddlewareWithSentry(handler); + + await expect(wrapped(new Request('https://example.com/foo', { method: 'GET' }))).rejects.toThrow('boom'); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith( + error, + expect.objectContaining({ + mechanism: { type: 'auto.function.nextjs.wrap_middleware', handled: false }, + }), + ); + }); + + it('captures errors thrown by the middleware when no span is active', async () => { + vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(undefined); + const captureExceptionSpy = vi.spyOn(SentryCore, 'captureException').mockReturnValue(''); + + const error = new Error('boom'); + const handler = vi.fn(async (_req: Request) => { + throw error; + }); + const wrapped = wrapMiddlewareWithSentry(handler); + + await expect(wrapped(new Request('https://example.com/foo', { method: 'GET' }))).rejects.toThrow('boom'); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + }); +});