diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts index 08a47ace671f..1c825e52947a 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts @@ -24,7 +24,7 @@ test('Sends a server-side exception to Sentry', async ({ baseURL }) => { expect(errorEvent.transaction).toEqual('GET /api/error'); - expect(errorEvent.contexts?.trace).toEqual({ + expect(errorEvent.contexts?.trace).toMatchObject({ trace_id: expect.stringMatching(/[a-f0-9]{32}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), }); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts index 731d1820ee61..09939c738e0b 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts @@ -22,32 +22,34 @@ test('Sends server-side transactions to Sentry', async ({ baseURL }) => { span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', data: expect.objectContaining({ 'http.response.status_code': 200, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.sample_rate': 1, 'sentry.source': 'route', }), status: 'ok', }, }), - spans: [ - { + spans: expect.arrayContaining([ + expect.objectContaining({ data: { 'sentry.origin': 'manual', }, description: 'test-span', origin: 'manual', - parent_span_id: transactionEvent.contexts?.trace?.span_id, + // Note: parent_span_id may be the root span or an intermediate "executing api route" span + // depending on Next.js instrumentation, so we just check it exists + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), status: 'ok', timestamp: expect.any(Number), trace_id: transactionEvent.contexts?.trace?.trace_id, - }, - ], + }), + ]), request: { headers: expect.any(Object), method: 'GET', diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/cjs-api-endpoints.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/cjs-api-endpoints.test.ts index 28cc91e9b879..9f07e32648a1 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/cjs-api-endpoints.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/cjs-api-endpoints.test.ts @@ -39,12 +39,12 @@ test('should create a transaction for a CJS pages router API endpoint', async ({ data: { 'http.response.status_code': 200, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.sample_rate': 1, 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'ok', trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -57,7 +57,7 @@ test('should create a transaction for a CJS pages router API endpoint', async ({ cookies: expect.any(Object), headers: expect.any(Object), method: 'GET', - url: expect.stringMatching(/^http.*\/api\/cjs-api-endpoint$/), + url: expect.stringMatching(/\/api\/cjs-api-endpoint$/), }, spans: expect.arrayContaining([]), start_timestamp: expect.any(Number), @@ -102,12 +102,12 @@ test('should not mess up require statements in CJS API endpoints', async ({ requ data: { 'http.response.status_code': 200, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.sample_rate': 1, 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'ok', trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -120,7 +120,7 @@ test('should not mess up require statements in CJS API endpoints', async ({ requ cookies: expect.any(Object), headers: expect.any(Object), method: 'GET', - url: expect.stringMatching(/^http.*\/api\/cjs-api-endpoint-with-require$/), + url: expect.stringMatching(/\/api\/cjs-api-endpoint-with-require$/), }, spans: expect.arrayContaining([]), start_timestamp: expect.any(Number), diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/pages-router-api-endpoints.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/pages-router-api-endpoints.test.ts index 9f5ff5db8434..de50ceee1076 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/pages-router-api-endpoints.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/pages-router-api-endpoints.test.ts @@ -55,11 +55,11 @@ test('Should report an error event for errors thrown in pages router api routes' data: { 'http.response.status_code': 500, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'internal_error', trace_id: (await errorEventPromise).contexts?.trace?.trace_id, @@ -98,11 +98,11 @@ test('Should report a transaction event for a successful pages router api route' data: { 'http.response.status_code': 200, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'ok', trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/wrapApiHandlerWithSentry.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/wrapApiHandlerWithSentry.test.ts index 798ea3409089..1f0e788fc8a4 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/wrapApiHandlerWithSentry.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/wrapApiHandlerWithSentry.test.ts @@ -39,11 +39,11 @@ cases.forEach(({ name, url, transactionName }) => { data: { 'http.response.status_code': 200, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.nextjs', + 'sentry.origin': 'auto', 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.http.nextjs', + origin: 'auto', span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'ok', trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts index 60a9b0d617f7..2016c79922cd 100644 --- a/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts @@ -1,21 +1,16 @@ import { captureException, - continueTrace, debug, getActiveSpan, + getIsolationScope, + getRootSpan, httpRequestToRequestData, - isString, objectify, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - setHttpStatus, - startSpanManual, - withIsolationScope, } from '@sentry/core'; import type { NextApiRequest } from 'next'; +import { TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL } from '../span-attributes-with-logic-attached'; import type { AugmentedNextApiResponse, NextApiHandler } from '../types'; -import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd'; -import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils'; +import { flushSafelyWithTimeout } from '../utils/responseEnd'; export type AugmentedNextApiRequest = NextApiRequest & { __withSentry_applied__?: boolean; @@ -31,15 +26,13 @@ export type AugmentedNextApiRequest = NextApiRequest & { */ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler { return new Proxy(apiHandler, { - apply: ( + apply: async ( wrappingTarget, thisArg, args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined], ) => { - dropNextjsRootContext(); - return escapeNextjsTracing(() => { + try { const [req, res] = args; - if (!req) { debug.log( `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, @@ -56,86 +49,55 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz if (req.__withSentry_applied__) { return wrappingTarget.apply(thisArg, args); } - req.__withSentry_applied__ = true; - - return withIsolationScope(isolationScope => { - // Normally, there is an active span here (from Next.js OTEL) and we just use that as parent - // Else, we manually continueTrace from the incoming headers - const continueTraceIfNoActiveSpan = getActiveSpan() - ? (_opts: unknown, callback: () => T) => callback() - : continueTrace; - - return continueTraceIfNoActiveSpan( - { - sentryTrace: - req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, - baggage: req.headers?.baggage, - }, - () => { - const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - const normalizedRequest = httpRequestToRequestData(req); - - isolationScope.setSDKProcessingMetadata({ normalizedRequest }); - isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`); - return startSpanManual( - { - name: `${reqMethod}${parameterizedRoute}`, - op: 'http.server', - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', - }, - }, - async span => { - // eslint-disable-next-line @typescript-eslint/unbound-method - res.end = new Proxy(res.end, { - apply(target, thisArg, argArray) { - setHttpStatus(span, res.statusCode); - span.end(); - waitUntil(flushSafelyWithTimeout()); - return target.apply(thisArg, argArray); - }, - }); - try { - return await wrappingTarget.apply(thisArg, args); - } catch (e) { - // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can - // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced - // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a - // way to prevent it from actually being reported twice.) - const objectifiedErr = objectify(e); + req.__withSentry_applied__ = true; - captureException(objectifiedErr, { - mechanism: { - type: 'auto.http.nextjs.api_handler', - handled: false, - data: { - wrapped_handler: wrappingTarget.name, - function: 'withSentry', - }, - }, - }); + // Set transaction name on isolation scope to ensure parameterized routes are used + // The HTTP server integration sets it on isolation scope, so we need to match that + const method = req.method || 'GET'; + const isolationScope = getIsolationScope(); + isolationScope.setTransactionName(`${method} ${parameterizedRoute}`); + // Set SDK processing metadata + isolationScope.setSDKProcessingMetadata({ + normalizedRequest: httpRequestToRequestData(req), + }); - setHttpStatus(span, 500); - span.end(); + // Set the route backfill attribute on the root span so that the transaction name + // gets updated to use the parameterized route during event processing + const activeSpan = getActiveSpan(); + if (activeSpan) { + const rootSpan = getRootSpan(activeSpan); + rootSpan.setAttribute(TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL, parameterizedRoute); + } - // we need to await the flush here to ensure that the error is captured - // as the runtime freezes as soon as the error is thrown below - await flushSafelyWithTimeout(); + return await wrappingTarget.apply(thisArg, args); + } catch (e) { + // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can + // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced + // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a + // way to prevent it from actually being reported twice.) + const objectifiedErr = objectify(e); - // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it - // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark - // the error as already having been captured.) - throw objectifiedErr; - } - }, - ); + captureException(objectifiedErr, { + mechanism: { + type: 'auto.http.nextjs.api_handler', + handled: false, + data: { + wrapped_handler: wrappingTarget.name, + function: 'withSentry', }, - ); + }, }); - }); + + // we need to await the flush here to ensure that the error is captured + // as the runtime freezes as soon as the error is thrown below + await flushSafelyWithTimeout(); + + // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it + // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark + // the error as already having been captured.) + throw objectifiedErr; + } }, }); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index bc5372274ad6..1b9315cdf706 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -368,6 +368,7 @@ export function init(options: NodeOptions): NodeClient | undefined { // backfill transaction name for pages that would otherwise contain unparameterized routes if (event.contexts.trace.data[TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL] && event.transaction !== 'GET /_app') { event.transaction = `${method} ${event.contexts.trace.data[TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL]}`; + event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; } const middlewareMatch = @@ -430,4 +431,5 @@ function sdkAlreadyInitialized(): boolean { export * from '../common'; +// eslint-disable-next-line max-lines export { wrapApiHandlerWithSentry } from '../common/pages-router-instrumentation/wrapApiHandlerWithSentry'; diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts deleted file mode 100644 index 3ed6672393ea..000000000000 --- a/packages/nextjs/test/config/withSentry.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import * as SentryCore from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import type { NextApiRequest, NextApiResponse } from 'next'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { AugmentedNextApiResponse, NextApiHandler } from '../../src/common/types'; -import { wrapApiHandlerWithSentry } from '../../src/server'; - -const startSpanManualSpy = vi.spyOn(SentryCore, 'startSpanManual'); - -describe('withSentry', () => { - let req: NextApiRequest, res: NextApiResponse; - - const origHandlerNoError: NextApiHandler = async (_req, res) => { - res.send('Good dog, Maisey!'); - }; - - const wrappedHandlerNoError = wrapApiHandlerWithSentry(origHandlerNoError, '/my-parameterized-route'); - - beforeEach(() => { - req = { url: 'http://dogs.are.great' } as NextApiRequest; - res = { - send: function (this: AugmentedNextApiResponse) { - this.end(); - }, - end: function (this: AugmentedNextApiResponse) { - // eslint-disable-next-line deprecation/deprecation - this.finished = true; - // @ts-expect-error This is a mock - this.writableEnded = true; - }, - } as unknown as AugmentedNextApiResponse; - }); - - afterEach(() => { - vi.clearAllMocks(); - }); - - describe('tracing', () => { - it('starts a transaction when tracing is enabled', async () => { - await wrappedHandlerNoError(req, res); - expect(startSpanManualSpy).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'GET /my-parameterized-route', - op: 'http.server', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', - }, - }), - expect.any(Function), - ); - }); - }); -});