diff --git a/MIGRATION.md b/MIGRATION.md index 7dc88f4b78b1..caa1b80678d4 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -190,6 +190,32 @@ be removed. Instead, use the new performance APIs: You can [read more about the new performance APIs here](./docs/v8-new-performance-apis.md). +## Deprecate variations of `Sentry.continueTrace()` + +The version of `Sentry.continueTrace()` which does not take a callback argument will be removed in favor of the version +that does. Additionally, the callback argument will not receive an argument with the next major version. + +Use `Sentry.continueTrace()` as follows: + +```ts +app.get('/your-route', req => { + Sentry.withIsolationScope(isolationScope => { + Sentry.continueTrace( + { + sentryTrace: req.headers.get('sentry-trace'), + baggage: req.headers.get('baggage'), + }, + () => { + // All events recorded in this callback will be associated with the incoming trace. For example: + Sentry.startSpan({ name: '/my-route' }, async () => { + await doExpensiveWork(); + }); + }, + ); + }); +}); +``` + ## Deprecate `Sentry.lastEventId()` and `hub.lastEventId()` `Sentry.lastEventId()` sometimes causes race conditions, so we are deprecating it in favour of the `beforeSend` diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 8a7cc3d90384..cd59d2f73344 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -94,97 +94,95 @@ async function instrumentRequest( const { method, headers } = ctx.request; - const traceCtx = continueTrace({ - sentryTrace: headers.get('sentry-trace') || undefined, - baggage: headers.get('baggage'), - }); + return continueTrace( + { + sentryTrace: headers.get('sentry-trace') || undefined, + baggage: headers.get('baggage'), + }, + async () => { + const allHeaders: Record = {}; - const allHeaders: Record = {}; + if (options.trackHeaders) { + headers.forEach((value, key) => { + allHeaders[key] = value; + }); + } - if (options.trackHeaders) { - headers.forEach((value, key) => { - allHeaders[key] = value; - }); - } + if (options.trackClientIp) { + getCurrentScope().setUser({ ip_address: ctx.clientAddress }); + } - if (options.trackClientIp) { - getCurrentScope().setUser({ ip_address: ctx.clientAddress }); - } + try { + const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params); + const source = interpolatedRoute ? 'route' : 'url'; + // storing res in a variable instead of directly returning is necessary to + // invoke the catch block if next() throws + const res = await startSpan( + { + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro', + }, + name: `${method} ${interpolatedRoute || ctx.url.pathname}`, + op: 'http.server', + status: 'ok', + data: { + method, + url: stripUrlQueryAndFragment(ctx.url.href), + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + ...(ctx.url.search && { 'http.query': ctx.url.search }), + ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }), + ...(options.trackHeaders && { headers: allHeaders }), + }, + }, + async span => { + const originalResponse = await next(); - try { - const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params); - const source = interpolatedRoute ? 'route' : 'url'; - // storing res in a variable instead of directly returning is necessary to - // invoke the catch block if next() throws - const res = await startSpan( - { - ...traceCtx, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro', - }, - name: `${method} ${interpolatedRoute || ctx.url.pathname}`, - op: 'http.server', - status: 'ok', - metadata: { - // eslint-disable-next-line deprecation/deprecation - ...traceCtx?.metadata, - }, - data: { - method, - url: stripUrlQueryAndFragment(ctx.url.href), - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, - ...(ctx.url.search && { 'http.query': ctx.url.search }), - ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }), - ...(options.trackHeaders && { headers: allHeaders }), - }, - }, - async span => { - const originalResponse = await next(); - - if (span && originalResponse.status) { - setHttpStatus(span, originalResponse.status); - } - - const scope = getCurrentScope(); - const client = getClient(); - const contentType = originalResponse.headers.get('content-type'); - - const isPageloadRequest = contentType && contentType.startsWith('text/html'); - if (!isPageloadRequest || !client) { - return originalResponse; - } - - // Type case necessary b/c the body's ReadableStream type doesn't include - // the async iterator that is actually available in Node - // We later on use the async iterator to read the body chunks - // see https://github.com/microsoft/TypeScript/issues/39051 - const originalBody = originalResponse.body as NodeJS.ReadableStream | null; - if (!originalBody) { - return originalResponse; - } - - const decoder = new TextDecoder(); - - const newResponseStream = new ReadableStream({ - start: async controller => { - for await (const chunk of originalBody) { - const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html, scope, client, span); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); + if (span && originalResponse.status) { + setHttpStatus(span, originalResponse.status); } - controller.close(); - }, - }); - return new Response(newResponseStream, originalResponse); - }, - ); - return res; - } catch (e) { - sendErrorToSentry(e); - throw e; - } - // TODO: flush if serverless (first extract function) + const scope = getCurrentScope(); + const client = getClient(); + const contentType = originalResponse.headers.get('content-type'); + + const isPageloadRequest = contentType && contentType.startsWith('text/html'); + if (!isPageloadRequest || !client) { + return originalResponse; + } + + // Type case necessary b/c the body's ReadableStream type doesn't include + // the async iterator that is actually available in Node + // We later on use the async iterator to read the body chunks + // see https://github.com/microsoft/TypeScript/issues/39051 + const originalBody = originalResponse.body as NodeJS.ReadableStream | null; + if (!originalBody) { + return originalResponse; + } + + const decoder = new TextDecoder(); + + const newResponseStream = new ReadableStream({ + start: async controller => { + for await (const chunk of originalBody) { + const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); + const modifiedHtml = addMetaTagToHead(html, scope, client, span); + controller.enqueue(new TextEncoder().encode(modifiedHtml)); + } + controller.close(); + }, + }); + + return new Response(newResponseStream, originalResponse); + }, + ); + return res; + } catch (e) { + sendErrorToSentry(e); + throw e; + } + // TODO: flush if serverless (first extract function) + }, + ); } /** diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index c641f5ac6177..a83de3cb0eb2 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -66,7 +66,6 @@ describe('sentryMiddleware', () => { url: 'https://mydomain.io/users/123/details', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', }, - metadata: {}, name: 'GET /users/[id]/details', op: 'http.server', status: 'ok', @@ -104,7 +103,6 @@ describe('sentryMiddleware', () => { url: 'http://localhost:1234/a%xx', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, - metadata: {}, name: 'GET a%xx', op: 'http.server', status: 'ok', @@ -144,43 +142,6 @@ describe('sentryMiddleware', () => { }); }); - it('attaches tracing headers', async () => { - const middleware = handleRequest(); - const ctx = { - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', - baggage: 'sentry-release=1.0.0', - }), - }, - params: {}, - url: new URL('https://myDomain.io/users/'), - }; - const next = vi.fn(() => nextResult); - - // @ts-expect-error, a partial ctx object is fine here - await middleware(ctx, next); - - expect(startSpanSpy).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }), - metadata: { - dynamicSamplingContext: { - release: '1.0.0', - }, - }, - parentSampled: true, - parentSpanId: '1234567890123456', - traceId: '12345678901234567890123456789012', - }), - expect.any(Function), // the `next` function - ); - }); - it('attaches client IP and request headers if options are set', async () => { const middleware = handleRequest({ trackClientIp: true, trackHeaders: true }); const ctx = { diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index d8109d6a9179..1f8b45d5fd97 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -1,6 +1,5 @@ import type { Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types'; -import type { propagationContextFromHeaders } from '@sentry/utils'; import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; @@ -225,31 +224,58 @@ export function getActiveSpan(): Span | undefined { return getCurrentScope().getSpan(); } -export function continueTrace({ - sentryTrace, - baggage, -}: { - sentryTrace: Parameters[0]; - baggage: Parameters[1]; -}): Partial; -export function continueTrace( - { +interface ContinueTrace { + /** + * Continue a trace from `sentry-trace` and `baggage` values. + * These values can be obtained from incoming request headers, + * or in the browser from `` and `` HTML tags. + * + * @deprecated Use the version of this function taking a callback as second parameter instead: + * + * ``` + * Sentry.continueTrace(sentryTrace: '...', baggage: '...' }, () => { + * // ... + * }) + * ``` + * + */ + ({ sentryTrace, baggage, }: { - sentryTrace: Parameters[0]; - baggage: Parameters[1]; - }, - callback: (transactionContext: Partial) => V, -): V; -/** - * Continue a trace from `sentry-trace` and `baggage` values. - * These values can be obtained from incoming request headers, - * or in the browser from `` and `` HTML tags. - * - * The callback receives a transactionContext that may be used for `startTransaction` or `startSpan`. - */ -export function continueTrace( + // eslint-disable-next-line deprecation/deprecation + sentryTrace: Parameters[0]; + // eslint-disable-next-line deprecation/deprecation + baggage: Parameters[1]; + }): Partial; + + /** + * Continue a trace from `sentry-trace` and `baggage` values. + * These values can be obtained from incoming request headers, or in the browser from `` + * and `` HTML tags. + * + * Spans started with `startSpan`, `startSpanManual` and `startInactiveSpan`, within the callback will automatically + * be attached to the incoming trace. + * + * Deprecation notice: In the next major version of the SDK the provided callback will not receive a transaction + * context argument. + */ + ( + { + sentryTrace, + baggage, + }: { + // eslint-disable-next-line deprecation/deprecation + sentryTrace: Parameters[0]; + // eslint-disable-next-line deprecation/deprecation + baggage: Parameters[1]; + }, + // TODO(v8): Remove parameter from this callback. + callback: (transactionContext: Partial) => V, + ): V; +} + +export const continueTrace: ContinueTrace = ( { sentryTrace, baggage, @@ -260,12 +286,14 @@ export function continueTrace( baggage: Parameters[1]; }, callback?: (transactionContext: Partial) => V, -): V | Partial { +): V | Partial => { // TODO(v8): Change this function so it doesn't do anything besides setting the propagation context on the current scope: /* - const propagationContext = propagationContextFromHeaders(sentryTrace, baggage); - getCurrentScope().setPropagationContext(propagationContext); - return; + return withScope((scope) => { + const propagationContext = propagationContextFromHeaders(sentryTrace, baggage); + scope.setPropagationContext(propagationContext); + return callback(); + }) */ const currentScope = getCurrentScope(); @@ -293,8 +321,10 @@ export function continueTrace( return transactionContext; } - return callback(transactionContext); -} + return runWithAsyncContext(() => { + return callback(transactionContext); + }); +}; function createChildSpanOrTransaction( hub: Hub, diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index fca086f10c94..488e5ed14897 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -728,6 +728,7 @@ describe('continueTrace', () => { traceId: '12312012123120121231201212312012', }; + // eslint-disable-next-line deprecation/deprecation const ctx = continueTrace({ sentryTrace: '12312012123120121231201212312012-1121201211212012-0', baggage: undefined, diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts index fd98fe2328ee..8597228f6e83 100644 --- a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts @@ -32,52 +32,52 @@ export function withEdgeWrapping( baggage = req.headers.get('baggage'); } - const transactionContext = continueTrace({ - sentryTrace, - baggage, - }); - - return startSpan( + return continueTrace( { - ...transactionContext, - name: options.spanDescription, - op: options.spanOp, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', - }, - metadata: { - // eslint-disable-next-line deprecation/deprecation - ...transactionContext.metadata, - request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined, - }, + sentryTrace, + baggage, }, - async span => { - const handlerResult = await handleCallbackErrors( - () => handler.apply(this, args), - error => { - captureException(error, { - mechanism: { - type: 'instrument', - handled: false, - data: { - function: options.mechanismFunctionName, - }, - }, - }); + () => { + return startSpan( + { + name: options.spanDescription, + op: options.spanOp, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + }, + metadata: { + request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined, + }, }, - ); + async span => { + const handlerResult = await handleCallbackErrors( + () => handler.apply(this, args), + error => { + captureException(error, { + mechanism: { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, + }, + }); + }, + ); - if (span) { - if (handlerResult instanceof Response) { - setHttpStatus(span, handlerResult.status); - } else { - span.setStatus('ok'); - } - } + if (span) { + if (handlerResult instanceof Response) { + setHttpStatus(span, handlerResult.status); + } else { + span.setStatus('ok'); + } + } - return handlerResult; + return handlerResult; + }, + ).finally(() => flushQueue()); }, - ).finally(() => flushQueue()); + ); }; } diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 61a08ba18891..62124e46912e 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -78,130 +78,130 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri addTracingExtensions(); - return runWithAsyncContext(async () => { - const transactionContext = continueTrace({ - sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, - baggage: req.headers?.baggage, - }); - - // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) - let reqPath = parameterizedRoute; - - // If not, fake it by just replacing parameter values with their names, hoping that none of them match either - // each other or any hard-coded parts of the path - if (!reqPath) { - const url = `${req.url}`; - // pull off query string, if any - reqPath = stripUrlQueryAndFragment(url); - // Replace with placeholder - if (req.query) { - for (const [key, value] of Object.entries(req.query)) { - reqPath = reqPath.replace(`${value}`, `[${key}]`); - } - } - } - - const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - - getCurrentScope().setSDKProcessingMetadata({ request: req }); - - return startSpanManual( + return runWithAsyncContext(() => { + return continueTrace( { - ...transactionContext, - name: `${reqMethod}${reqPath}`, - op: 'http.server', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', - }, - metadata: { - // eslint-disable-next-line deprecation/deprecation - ...transactionContext.metadata, - request: req, - }, + sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, + baggage: req.headers?.baggage, }, - async span => { - // eslint-disable-next-line @typescript-eslint/unbound-method - res.end = new Proxy(res.end, { - apply(target, thisArg, argArray) { - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - target.apply(thisArg, argArray); - } else { - // flushQueue will not reject - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue().then(() => { - target.apply(thisArg, argArray); - }); + () => { + // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) + let reqPath = parameterizedRoute; + + // If not, fake it by just replacing parameter values with their names, hoping that none of them match either + // each other or any hard-coded parts of the path + if (!reqPath) { + const url = `${req.url}`; + // pull off query string, if any + reqPath = stripUrlQueryAndFragment(url); + // Replace with placeholder + if (req.query) { + for (const [key, value] of Object.entries(req.query)) { + reqPath = reqPath.replace(`${value}`, `[${key}]`); } - }, - }); - - try { - const handlerResult = await wrappingTarget.apply(thisArg, args); - if ( - process.env.NODE_ENV === 'development' && - !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && - !res.finished - // TODO(v8): Remove this warning? - // This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating. - // Warning suppression on Next.JS is only necessary in that case. - ) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `withSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', - ); - }); } + } - return handlerResult; - } 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); - - captureException(objectifiedErr, { - mechanism: { - type: 'instrument', - handled: false, - data: { - wrapped_handler: wrappingTarget.name, - function: 'withSentry', - }, - }, - }); + const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet - // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that - // the transaction was error-free - res.statusCode = 500; - res.statusMessage = 'Internal Server Error'; + getCurrentScope().setSDKProcessingMetadata({ request: req }); - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } + return startSpanManual( + { + name: `${reqMethod}${reqPath}`, + op: 'http.server', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', + }, + metadata: { + request: req, + }, + }, + async span => { + // eslint-disable-next-line @typescript-eslint/unbound-method + res.end = new Proxy(res.end, { + apply(target, thisArg, argArray) { + if (span) { + setHttpStatus(span, res.statusCode); + span.end(); + } + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + target.apply(thisArg, argArray); + } else { + // flushQueue will not reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue().then(() => { + target.apply(thisArg, argArray); + }); + } + }, + }); - // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors - // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the - // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not - // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already - // be finished and the queue will already be empty, so effectively it'll just no-op.) - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - await flushQueue(); - } + try { + const handlerResult = await wrappingTarget.apply(thisArg, args); + if ( + process.env.NODE_ENV === 'development' && + !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && + !res.finished + // TODO(v8): Remove this warning? + // This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating. + // Warning suppression on Next.JS is only necessary in that case. + ) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `withSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', + ); + }); + } + + return handlerResult; + } 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); + + captureException(objectifiedErr, { + mechanism: { + type: 'instrument', + handled: false, + data: { + wrapped_handler: wrappingTarget.name, + function: 'withSentry', + }, + }, + }); - // 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; - } + // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet + // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that + // the transaction was error-free + res.statusCode = 500; + res.statusMessage = 'Internal Server Error'; + + if (span) { + setHttpStatus(span, res.statusCode); + span.end(); + } + + // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors + // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the + // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not + // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already + // be finished and the queue will already be empty, so effectively it'll just no-op.) + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + await flushQueue(); + } + + // 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/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 5e6a051ffcfb..ec4fe9048900 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -47,6 +47,7 @@ export function wrapGenerationFunctionWithSentry a } return runWithAsyncContext(() => { + // eslint-disable-next-line deprecation/deprecation const transactionContext = continueTrace({ baggage: headers?.get('baggage'), sentryTrace: headers?.get('sentry-trace') ?? undefined, diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index f8b6c5698550..59a608406e09 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -37,6 +37,7 @@ export function wrapServerComponentWithSentry any> ? winterCGHeadersToDict(context.headers) : {}; + // eslint-disable-next-line deprecation/deprecation const transactionContext = continueTrace({ // eslint-disable-next-line deprecation/deprecation sentryTrace: context.sentryTraceHeader ?? completeHeadersDict['sentry-trace'], diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 10f0e0e29c81..240dff84eebc 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -354,24 +354,23 @@ export function wrapHandler( : undefined; const baggage = eventWithHeaders.headers?.baggage; - const continueTraceContext = continueTrace({ sentryTrace, baggage }); - - return startSpanManual( - { - name: context.functionName, - op: 'function.aws.lambda', - ...continueTraceContext, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', + return continueTrace({ sentryTrace, baggage }, () => { + return startSpanManual( + { + name: context.functionName, + op: 'function.aws.lambda', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', + }, }, - }, - span => { - enhanceScopeWithTransactionData(getCurrentScope(), context); + span => { + enhanceScopeWithTransactionData(getCurrentScope(), context); - return processResult(span); - }, - ); + return processResult(span); + }, + ); + }); } return withScope(async () => { diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index a90dd3a0423c..e02093acf438 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -77,58 +77,57 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { + return startSpanManual( + { + name: `${reqMethod} ${reqUrl}`, + op: 'function.gcp.http', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless.gcp_http', + }, }, - }, - span => { - getCurrentScope().setSDKProcessingMetadata({ - request: req, - requestDataOptionsFromGCPWrapper: options.addRequestDataToEventOptions, - }); - - if (span instanceof Transaction) { - // We also set __sentry_transaction on the response so people can grab the transaction there to add - // spans to it later. - // TODO(v8): Remove this - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access - (res as any).__sentry_transaction = span; - } - - // eslint-disable-next-line @typescript-eslint/unbound-method - const _end = res.end; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any { - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); + span => { + getCurrentScope().setSDKProcessingMetadata({ + request: req, + requestDataOptionsFromGCPWrapper: options.addRequestDataToEventOptions, + }); + + if (span instanceof Transaction) { + // We also set __sentry_transaction on the response so people can grab the transaction there to add + // spans to it later. + // TODO(v8): Remove this + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + (res as any).__sentry_transaction = span; } - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flush(options.flushTimeout) - .then(null, e => { - DEBUG_BUILD && logger.error(e); - }) - .then(() => { - _end.call(this, chunk, encoding, cb); - }); - }; - - return handleCallbackErrors( - () => fn(req, res), - err => { - captureException(err, scope => markEventUnhandled(scope)); - }, - ); - }, - ); + // eslint-disable-next-line @typescript-eslint/unbound-method + const _end = res.end; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any { + if (span) { + setHttpStatus(span, res.statusCode); + span.end(); + } + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flush(options.flushTimeout) + .then(null, e => { + DEBUG_BUILD && logger.error(e); + }) + .then(() => { + _end.call(this, chunk, encoding, cb); + }); + }; + + return handleCallbackErrors( + () => fn(req, res), + err => { + captureException(err, scope => markEventUnhandled(scope)); + }, + ); + }, + ); + }); }; } diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index 57feede5a102..772502057a34 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -247,7 +247,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(rv).toStrictEqual(42); @@ -277,7 +276,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); @@ -301,42 +299,6 @@ describe('AWSLambda', () => { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); }); - test('incoming trace headers are correctly parsed and used', async () => { - expect.assertions(1); - - fakeEvent.headers = { - 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', - baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', - }; - - const handler: Handler = (_event, _context, callback) => { - expect(mockStartSpanManual).toBeCalledWith( - expect.objectContaining({ - parentSpanId: '1121201211212012', - parentSampled: false, - op: 'function.aws.lambda', - name: 'functionName', - traceId: '12312012123120121231201212312012', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - metadata: { - dynamicSamplingContext: { - release: '2.12.1', - }, - }, - }), - expect.any(Function), - ); - - callback(undefined, { its: 'fine' }); - }; - - const wrappedHandler = wrapHandler(handler); - await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - }); - test('capture error', async () => { expect.assertions(10); @@ -347,20 +309,15 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); try { - fakeEvent.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' }; await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch (e) { const fakeTransactionContext = { name: 'functionName', op: 'function.aws.lambda', - traceId: '12312012123120121231201212312012', - parentSpanId: '1121201211212012', - parentSampled: false, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: { dynamicSamplingContext: {} }, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); @@ -390,7 +347,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(rv).toStrictEqual(42); @@ -431,7 +387,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); @@ -474,7 +429,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(rv).toStrictEqual(42); @@ -515,7 +469,6 @@ describe('AWSLambda', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', }, - metadata: {}, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 1fc58c37fdce..cde69e6b22d2 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -164,7 +164,6 @@ describe('GCPFunction', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless.gcp_http', }, - metadata: {}, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); @@ -172,38 +171,6 @@ describe('GCPFunction', () => { expect(mockFlush).toBeCalledWith(2000); }); - test('incoming trace headers are correctly parsed and used', async () => { - const handler: HttpFunction = (_req, res) => { - res.statusCode = 200; - res.end(); - }; - const wrappedHandler = wrapHttpFunction(handler); - const traceHeaders = { - 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', - baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', - }; - await handleHttp(wrappedHandler, traceHeaders); - - const fakeTransactionContext = { - name: 'POST /path', - op: 'function.gcp.http', - traceId: '12312012123120121231201212312012', - parentSpanId: '1121201211212012', - parentSampled: false, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless.gcp_http', - }, - metadata: { - dynamicSamplingContext: { - release: '2.12.1', - }, - }, - }; - - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - }); - test('capture error', async () => { const error = new Error('wat'); const handler: HttpFunction = (_req, _res) => { @@ -211,23 +178,15 @@ describe('GCPFunction', () => { }; const wrappedHandler = wrapHttpFunction(handler); - const trace_headers: { [key: string]: string } = { - 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', - }; - - await handleHttp(wrappedHandler, trace_headers); + await handleHttp(wrappedHandler); const fakeTransactionContext = { name: 'POST /path', op: 'function.gcp.http', - traceId: '12312012123120121231201212312012', - parentSpanId: '1121201211212012', - parentSampled: false, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless.gcp_http', }, - metadata: { dynamicSamplingContext: {} }, }; expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function));