From 95cecbdfde8c1640ef9a5718586bd852202e6fe9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 17 Aug 2023 11:51:16 +0000 Subject: [PATCH 01/15] Pain --- packages/nextjs/rollup.npm.config.js | 1 + packages/nextjs/src/common/index.ts | 2 + packages/nextjs/src/common/types.ts | 7 ++ .../src/common/wrapRouteHandlerWithSentry.ts | 50 +++++++++++++ .../src/config/loaders/wrappingLoader.ts | 17 +++-- .../templates/routeHandlerWrapperTemplate.ts | 74 +++++++++++++++++++ packages/nextjs/src/config/webpack.ts | 24 +++++- 7 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts create mode 100644 packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index ce15b951235e..e033fd6f90c1 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -30,6 +30,7 @@ export default [ 'src/config/templates/requestAsyncStorageShim.ts', 'src/config/templates/sentryInitWrapperTemplate.ts', 'src/config/templates/serverComponentWrapperTemplate.ts', + 'src/config/templates/routeHandlerWrapperTemplate.ts', ], packageSpecificConfig: { diff --git a/packages/nextjs/src/common/index.ts b/packages/nextjs/src/common/index.ts index ccd4a628634e..3aeef33af760 100644 --- a/packages/nextjs/src/common/index.ts +++ b/packages/nextjs/src/common/index.ts @@ -36,6 +36,8 @@ export { export { wrapServerComponentWithSentry } from './wrapServerComponentWithSentry'; +export { wrapRouteHandlerWithSentry } from './wrapRouteHandlerWithSentry'; + export { wrapApiHandlerWithSentryVercelCrons } from './wrapApiHandlerWithSentryVercelCrons'; export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; diff --git a/packages/nextjs/src/common/types.ts b/packages/nextjs/src/common/types.ts index dc838b214276..7c1ce7425108 100644 --- a/packages/nextjs/src/common/types.ts +++ b/packages/nextjs/src/common/types.ts @@ -8,6 +8,13 @@ export type ServerComponentContext = { baggageHeader?: string; }; +export interface RouteHandlerContext { + method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'HEAD' | 'OPTIONS'; + parameterizedRoute: string; + sentryTraceHeader?: string; + baggageHeader?: string; +} + export type VercelCronsConfig = { path?: string; schedule?: string }[] | undefined; // The `NextApiHandler` and `WrappedNextApiHandler` types are the same as the official `NextApiHandler` type, except: diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts new file mode 100644 index 000000000000..79787da2bbc5 --- /dev/null +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -0,0 +1,50 @@ +import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; +import { RouteHandlerContext } from './types'; +import { tracingContextFromHeaders } from '@sentry/utils'; + +export function wrapRouteHandlerWithSentry any>( + routeHandler: F, + context: RouteHandlerContext, +): F { + addTracingExtensions(); + + const { method, parameterizedRoute, baggageHeader, sentryTraceHeader } = context; + + return new Proxy(routeHandler, { + apply: (originalFunction, thisArg, args) => { + return runWithAsyncContext(() => { + const hub = getCurrentHub(); + const currentScope = hub.getScope(); + + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTraceHeader, + baggageHeader, + ); + currentScope.setPropagationContext(propagationContext); + + console.log({ traceparentData, baggageHeader, sentryTraceHeader }); + + const res = trace( + { + op: 'http.server', + name: `${method} ${parameterizedRoute}`, + status: 'ok', + ...traceparentData, + metadata: { + source: 'route', + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + }, + }, + async () => { + return originalFunction.apply(thisArg, args); + }, + error => { + captureException(error); + }, + ); + + return res; + }); + }, + }); +} diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 57b913b23ab1..7da54309972d 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -41,12 +41,15 @@ const serverComponentWrapperTemplatePath = path.resolve( ); const serverComponentWrapperTemplateCode = fs.readFileSync(serverComponentWrapperTemplatePath, { encoding: 'utf8' }); +const routeHandlerWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'routeHandlerWrapperTemplate.js'); +const routeHandlerWrapperTemplateCode = fs.readFileSync(routeHandlerWrapperTemplatePath, { encoding: 'utf8' }); + type LoaderOptions = { pagesDir: string; appDir: string; pageExtensionRegex: string; excludeServerRoutes: Array; - wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component' | 'sentry-init'; + wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component' | 'sentry-init' | 'route-handler'; sentryConfigFilePath?: string; vercelCronsConfig?: VercelCronsConfig; }; @@ -144,14 +147,14 @@ export default function wrappingLoader( // Inject the route and the path to the file we're wrapping into the template templateCode = templateCode.replace(/__ROUTE__/g, parameterizedPagesRoute.replace(/\\/g, '\\\\')); - } else if (wrappingTargetKind === 'server-component') { + } else if (wrappingTargetKind === 'server-component' || wrappingTargetKind === 'route-handler') { // Get the parameterized route name from this page's filepath const parameterizedPagesRoute = path.posix .normalize(path.relative(appDir, this.resourcePath)) // Add a slash at the beginning .replace(/(.*)/, '/$1') // Pull off the file name - .replace(/\/[^/]+\.(js|jsx|tsx)$/, '') + .replace(/\/[^/]+\.(js|ts|jsx|tsx)$/, '') // Remove routing groups: https://beta.nextjs.org/docs/routing/defining-routes#example-creating-multiple-root-layouts .replace(/\/(\(.*?\)\/)+/g, '/') // In case all of the above have left us with an empty string (which will happen if we're dealing with the @@ -173,7 +176,11 @@ export default function wrappingLoader( return; } - templateCode = serverComponentWrapperTemplateCode; + if (wrappingTargetKind === 'server-component') { + templateCode = serverComponentWrapperTemplateCode; + } else { + templateCode = routeHandlerWrapperTemplateCode; + } if (requestAsyncStorageModuleExists) { templateCode = templateCode.replace( @@ -197,7 +204,7 @@ export default function wrappingLoader( const componentTypeMatch = path.posix .normalize(path.relative(appDir, this.resourcePath)) - .match(/\/?([^/]+)\.(?:js|jsx|tsx)$/); + .match(/\/?([^/]+)\.(?:js|ts|jsx|tsx)$/); if (componentTypeMatch && componentTypeMatch[1]) { let componentType; diff --git a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts new file mode 100644 index 000000000000..6da11b212b92 --- /dev/null +++ b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts @@ -0,0 +1,74 @@ +// @ts-ignore Because we cannot be sure if the RequestAsyncStorage module exists (it is not part of the Next.js public +// API) we use a shim if it doesn't exist. The logic for this is in the wrapping loader. +// eslint-disable-next-line import/no-unresolved +import { requestAsyncStorage } from '__SENTRY_NEXTJS_REQUEST_ASYNC_STORAGE_SHIM__'; +// @ts-ignore See above +// eslint-disable-next-line import/no-unresolved +import * as routeModule from '__SENTRY_WRAPPING_TARGET_FILE__'; +// eslint-disable-next-line import/no-extraneous-dependencies +import * as Sentry from '@sentry/nextjs'; + +import type { RequestAsyncStorage } from './requestAsyncStorageShim'; + +declare const requestAsyncStorage: RequestAsyncStorage; + +declare const routeModule: { + default: unknown; + GET?: (...args: unknown[]) => unknown; + POST?: (...args: unknown[]) => unknown; + PUT?: (...args: unknown[]) => unknown; + PATCH?: (...args: unknown[]) => unknown; + DELETE?: (...args: unknown[]) => unknown; + HEAD?: (...args: unknown[]) => unknown; + OPTIONS?: (...args: unknown[]) => unknown; +}; + +function wrapHandler(handler: T, method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'HEAD' | 'OPTIONS'): T { + // Running the instrumentation code during the build phase will mark any function as "dynamic" because we're accessing + // the Request object. We do not want to turn handlers dynamic so we skip instrumentation in the build phase. + if (process.env.NEXT_PHASE === 'phase-production-build') { + return handler; + } + + if (typeof handler !== 'function') { + return handler; + } + + return new Proxy(handler, { + apply: (originalFunction, thisArg, args) => { + let sentryTraceHeader: string | undefined | null = undefined; + let baggageHeader: string | undefined | null = undefined; + + // We try-catch here just in case the API around `requestAsyncStorage` changes unexpectedly since it is not public API + try { + const requestAsyncStore = requestAsyncStorage.getStore(); + sentryTraceHeader = requestAsyncStore?.headers.get('sentry-trace') ?? undefined; + baggageHeader = requestAsyncStore?.headers.get('baggage') ?? undefined; + } catch (e) { + /** empty */ + } + + return Sentry.wrapRouteHandlerWithSentry(originalFunction as any, { + method, + parameterizedRoute: '__ROUTE__', + sentryTraceHeader, + baggageHeader, + }).apply(thisArg, args); + }, + }); +} + +export const GET = wrapHandler(routeModule.GET, 'GET'); +export const POST = wrapHandler(routeModule.POST, 'POST'); +export const PUT = wrapHandler(routeModule.PUT, 'PUT'); +export const PATCH = wrapHandler(routeModule.PATCH, 'PATCH'); +export const DELETE = wrapHandler(routeModule.DELETE, 'DELETE'); +export const HEAD = wrapHandler(routeModule.HEAD, 'HEAD'); +export const OPTIONS = wrapHandler(routeModule.OPTIONS, 'OPTIONS'); + +// Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to +// not include anything whose name matchs something we've explicitly exported above. +// @ts-ignore See above +// eslint-disable-next-line import/no-unresolved +export * from '__SENTRY_WRAPPING_TARGET_FILE__'; +export default routeModule.default; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index d711fef5c14f..c70840a8c412 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -173,6 +173,14 @@ export function constructWebpackConfigFunction( ); }; + const isRouteHandlerResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && + !!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|ts|jsx|tsx)$/) + ); + }; + if (isServer && userSentryOptions.autoInstrumentServerFunctions !== false) { // It is very important that we insert our loaders at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. @@ -245,7 +253,7 @@ export function constructWebpackConfigFunction( } if (isServer && userSentryOptions.autoInstrumentAppDirectory !== false) { - // Wrap page server components + // Wrap server components newConfig.module.rules.unshift({ test: isServerComponentResource, use: [ @@ -258,6 +266,20 @@ export function constructWebpackConfigFunction( }, ], }); + + // Wrap route handlers + newConfig.module.rules.unshift({ + test: isRouteHandlerResource, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), + options: { + ...staticWrappingLoaderOptions, + wrappingTargetKind: 'route-handler', + }, + }, + ], + }); } if (isServer) { From 73da670f689e87eb1fab7923b1cdf825d113d04f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Aug 2023 10:04:51 +0000 Subject: [PATCH 02/15] fix: Defer tracing decision for tracing context without performance --- packages/core/src/scope.ts | 4 +--- packages/hub/test/scope.test.ts | 4 ++-- packages/node/test/integrations/http.test.ts | 5 +++-- .../tracing-internal/src/browser/browsertracing.ts | 11 +++++------ packages/types/src/tracing.ts | 2 +- packages/utils/src/tracing.ts | 5 ++--- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index b72defc56016..1c5987d709d1 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines */ +import { updateSession } from './session'; import type { Attachment, Breadcrumb, @@ -33,8 +34,6 @@ import { uuid4, } from '@sentry/utils'; -import { updateSession } from './session'; - /** * Default value for maximum number of breadcrumbs added to an event. */ @@ -636,6 +635,5 @@ function generatePropagationContext(): PropagationContext { return { traceId: uuid4(), spanId: uuid4().substring(16), - sampled: false, }; } diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 982cba766a87..4c0d830340ec 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -20,7 +20,7 @@ describe('Scope', () => { expect(scope._propagationContext).toEqual({ traceId: expect.any(String), spanId: expect.any(String), - sampled: false, + sampled: undefined, dsc: undefined, parentSpanId: undefined, }); @@ -442,7 +442,7 @@ describe('Scope', () => { expect(scope._propagationContext).toEqual({ traceId: expect.any(String), spanId: expect.any(String), - sampled: false, + sampled: undefined, }); // @ts-expect-error accessing private property expect(scope._propagationContext).not.toEqual(oldPropagationContext); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index e784eb308f54..0f1613a27345 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -208,10 +208,11 @@ describe('tracing', () => { const baggageHeader = request.getHeader('baggage') as string; const parts = sentryTraceHeader.split('-'); - expect(parts.length).toEqual(3); + + // Should not include sampling decision since we don't wanna influence the tracing decisions downstream + expect(parts.length).toEqual(2); expect(parts[0]).toEqual(traceId); expect(parts[1]).toEqual(expect.any(String)); - expect(parts[2]).toEqual('0'); expect(baggageHeader).toEqual( `sentry-environment=production,sentry-release=1.0.0,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=${traceId}`, diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index aae66bee3358..1a5cebeccd14 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,9 +1,4 @@ /* eslint-disable max-lines */ -import type { Hub, IdleTransaction } from '@sentry/core'; -import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; -import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; - import { registerBackgroundTabDetection } from './backgroundtab'; import { addPerformanceEntries, @@ -15,6 +10,10 @@ import type { RequestInstrumentationOptions } from './request'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; import { instrumentRoutingWithDefaults } from './router'; import { WINDOW } from './types'; +import type { Hub, IdleTransaction } from '@sentry/core'; +import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; +import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; @@ -363,7 +362,7 @@ export class BrowserTracing implements Integration { traceId: idleTransaction.traceId, spanId: idleTransaction.spanId, parentSpanId: idleTransaction.parentSpanId, - sampled: !!idleTransaction.sampled, + sampled: idleTransaction.sampled, }); } diff --git a/packages/types/src/tracing.ts b/packages/types/src/tracing.ts index 11c4a1658d50..7c5b02c45596 100644 --- a/packages/types/src/tracing.ts +++ b/packages/types/src/tracing.ts @@ -5,7 +5,7 @@ export type TracePropagationTargets = (string | RegExp)[]; export interface PropagationContext { traceId: string; spanId: string; - sampled: boolean; + sampled?: boolean; parentSpanId?: string; dsc?: DynamicSamplingContext; } diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index f879b856f9f2..845430fdc209 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -1,7 +1,6 @@ -import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; - import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { uuid4 } from './misc'; +import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace @@ -61,7 +60,7 @@ export function tracingContextFromHeaders( const propagationContext: PropagationContext = { traceId: traceId || uuid4(), spanId: uuid4().substring(16), - sampled: parentSampled === undefined ? false : parentSampled, + sampled: parentSampled, }; if (parentSpanId) { From 7b51e1b242042f5d63db34875444db64245d78af Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Aug 2023 10:12:05 +0000 Subject: [PATCH 03/15] Format --- packages/core/src/scope.ts | 3 ++- packages/tracing-internal/src/browser/browsertracing.ts | 9 +++++---- packages/utils/src/tracing.ts | 3 ++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 1c5987d709d1..8875f1c996f2 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -1,5 +1,4 @@ /* eslint-disable max-lines */ -import { updateSession } from './session'; import type { Attachment, Breadcrumb, @@ -34,6 +33,8 @@ import { uuid4, } from '@sentry/utils'; +import { updateSession } from './session'; + /** * Default value for maximum number of breadcrumbs added to an event. */ diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 1a5cebeccd14..ef40adc147ec 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,4 +1,9 @@ /* eslint-disable max-lines */ +import type { Hub, IdleTransaction } from '@sentry/core'; +import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; +import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; + import { registerBackgroundTabDetection } from './backgroundtab'; import { addPerformanceEntries, @@ -10,10 +15,6 @@ import type { RequestInstrumentationOptions } from './request'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; import { instrumentRoutingWithDefaults } from './router'; import { WINDOW } from './types'; -import type { Hub, IdleTransaction } from '@sentry/core'; -import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; -import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index 845430fdc209..6ad839f2b920 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -1,6 +1,7 @@ +import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; + import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { uuid4 } from './misc'; -import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace From 80b74dae6445053aa367bb00089eaee8ccc8af3e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Aug 2023 12:27:24 +0000 Subject: [PATCH 04/15] . --- .../src/common/wrapRouteHandlerWithSentry.ts | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 79787da2bbc5..33103703d021 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,7 +1,11 @@ -import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; -import { RouteHandlerContext } from './types'; -import { tracingContextFromHeaders } from '@sentry/utils'; +import { addTracingExtensions, captureException, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; +import { isThenable, tracingContextFromHeaders } from '@sentry/utils'; +import type { RouteHandlerContext } from './types'; + +/** + * Wraps a Next.js route handler with performance and error instrumentation. + */ export function wrapRouteHandlerWithSentry any>( routeHandler: F, context: RouteHandlerContext, @@ -22,8 +26,6 @@ export function wrapRouteHandlerWithSentry any>( ); currentScope.setPropagationContext(propagationContext); - console.log({ traceparentData, baggageHeader, sentryTraceHeader }); - const res = trace( { op: 'http.server', @@ -35,8 +37,26 @@ export function wrapRouteHandlerWithSentry any>( dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }, - async () => { - return originalFunction.apply(thisArg, args); + span => { + const maybePromiseResponse = originalFunction.apply(thisArg, args); + + const setSpanStatus = (response: Response): void => { + try { + span?.setHttpStatus(response.status); + } catch { + // best effort + } + }; + + if (isThenable(maybePromiseResponse)) { + return maybePromiseResponse.then(response => { + setSpanStatus(response); + return response; + }); + } else { + setSpanStatus(maybePromiseResponse); + return maybePromiseResponse; + } }, error => { captureException(error); From 109624cc9add46514e43e7d729fbd980c71f4639 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 28 Aug 2023 10:33:45 +0000 Subject: [PATCH 05/15] lint --- .../nextjs/src/config/templates/routeHandlerWrapperTemplate.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts index 6da11b212b92..813f860d35de 100644 --- a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts @@ -48,6 +48,7 @@ function wrapHandler(handler: T, method: 'GET' | 'POST' | 'PUT' | 'PATCH' | ' /** empty */ } + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any return Sentry.wrapRouteHandlerWithSentry(originalFunction as any, { method, parameterizedRoute: '__ROUTE__', From bd454b09b5c7c142a2b4271e3afc8d3d843c079c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 28 Aug 2023 11:58:14 +0000 Subject: [PATCH 06/15] flush in lambdas and stuff --- .../src/common/wrapRouteHandlerWithSentry.ts | 42 +++++++++---------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 33103703d021..5e7788c4186b 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,22 +1,24 @@ -import { addTracingExtensions, captureException, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; -import { isThenable, tracingContextFromHeaders } from '@sentry/utils'; +import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; +import { tracingContextFromHeaders } from '@sentry/utils'; import type { RouteHandlerContext } from './types'; +import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; /** * Wraps a Next.js route handler with performance and error instrumentation. */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function wrapRouteHandlerWithSentry any>( routeHandler: F, context: RouteHandlerContext, -): F { +): (...args: Parameters) => ReturnType extends Promise ? ReturnType : Promise> { addTracingExtensions(); const { method, parameterizedRoute, baggageHeader, sentryTraceHeader } = context; return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { - return runWithAsyncContext(() => { + return runWithAsyncContext(async () => { const hub = getCurrentHub(); const currentScope = hub.getScope(); @@ -26,7 +28,7 @@ export function wrapRouteHandlerWithSentry any>( ); currentScope.setPropagationContext(propagationContext); - const res = trace( + const res = await trace( { op: 'http.server', name: `${method} ${parameterizedRoute}`, @@ -37,32 +39,26 @@ export function wrapRouteHandlerWithSentry any>( dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }, - span => { - const maybePromiseResponse = originalFunction.apply(thisArg, args); + async span => { + const response: Response = await originalFunction.apply(thisArg, args); - const setSpanStatus = (response: Response): void => { - try { - span?.setHttpStatus(response.status); - } catch { - // best effort - } - }; - - if (isThenable(maybePromiseResponse)) { - return maybePromiseResponse.then(response => { - setSpanStatus(response); - return response; - }); - } else { - setSpanStatus(maybePromiseResponse); - return maybePromiseResponse; + try { + span?.setHttpStatus(response.status); + } catch { + // best effort } + + return response; }, error => { captureException(error); }, ); + if (!platformSupportsStreaming()) { + await flush(1000); + } + return res; }); }, From 42d4b31ac64b5a98ebd57b231149ee9ac756d72d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 09:07:43 +0000 Subject: [PATCH 07/15] Add tests --- .../app/route-handlers/[param]/error/route.ts | 3 ++ .../app/route-handlers/[param]/route.ts | 9 ++++ .../tests/route-handlers.test.ts | 50 +++++++++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts new file mode 100644 index 000000000000..fd50ef5c8a44 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts @@ -0,0 +1,3 @@ +export async function PUT() { + throw new Error('route-handler-error'); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts new file mode 100644 index 000000000000..386b8c6e117f --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts @@ -0,0 +1,9 @@ +import { NextResponse } from 'next/server'; + +export async function GET() { + return NextResponse.json({ name: 'John Doe' }); +} + +export async function POST() { + return NextResponse.json({ name: 'John Doe' }, { status: 404 }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts new file mode 100644 index 000000000000..fde04b37228f --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -0,0 +1,50 @@ +import { test, expect } from '@playwright/test'; +import { waitForTransaction, waitForError } from '../event-proxy-server'; + +test('Should create a transaction for route handlers (GET)', async ({ request }) => { + const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'GET /route-handlers/[param]'; + }); + + const response = await request.get('/route-handlers/foo'); + expect(await response.json()).toStrictEqual({ name: 'John Doe' }); + + const routehandlerTransaction = await routehandlerTransactionPromise; + + expect(routehandlerTransaction.contexts?.trace?.status).toBe('ok'); + expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); +}); + +test('Should create a transaction for route handlers (POST)', async ({ request }) => { + const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'POST /route-handlers/[param]'; + }); + + const response = await request.post('/route-handlers/bar'); + expect(await response.json()).toStrictEqual({ name: 'John Doe' }); + + const routehandlerTransaction = await routehandlerTransactionPromise; + + expect(routehandlerTransaction.contexts?.trace?.status).toBe('not_found'); + expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); +}); + +test('Should record exceptions and transactions for faulty route handlers', async ({ request }) => { + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'route-handler-error'; + }); + + const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'PUT /route-handlers/[param]/error'; + }); + + void request.put('/route-handlers/baz/error'); + + const routehandlerTransaction = await routehandlerTransactionPromise; + const routehandlerError = await errorEventPromise; + + expect(routehandlerTransaction.contexts?.trace?.status).toBe('internal_error'); + expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); + + expect(routehandlerError).toBe(1); +}); From 287ac32347d3d51e2d58d51171bd26313972b057 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 09:31:58 +0000 Subject: [PATCH 08/15] Assertion --- .../nextjs-app-dir/tests/route-handlers.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index fde04b37228f..33b5a8a32328 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -46,5 +46,6 @@ test('Should record exceptions and transactions for faulty route handlers', asyn expect(routehandlerTransaction.contexts?.trace?.status).toBe('internal_error'); expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); - expect(routehandlerError).toBe(1); + expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-error'); + expect(routehandlerError.tags?.transaction).toBe('PUT /route-handlers/[param]/error'); }); From 1cd3b2abc9baf49046fa7e7ddefbd30a56111d49 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 10:05:52 +0000 Subject: [PATCH 09/15] . --- .../app/route-handlers/[param]/edge/route.ts | 11 +++++ .../nextjs-app-dir/sentry.edge.config.ts | 1 + .../tests/route-handlers.test.ts | 45 ++++++++++++++++++- packages/nextjs/src/config/webpack.ts | 5 ++- 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts new file mode 100644 index 000000000000..8c96a39e5554 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts @@ -0,0 +1,11 @@ +import { NextResponse } from 'next/server'; + +export const runtime = 'edge'; + +export async function PATCH() { + return NextResponse.json({ name: 'John Doe' }, { status: 401 }); +} + +export async function DELETE() { + throw new Error('route-handler-edge-error'); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts index ad780407a5b7..22c9813c8702 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts @@ -5,4 +5,5 @@ Sentry.init({ dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1.0, + debug: true, }); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index 33b5a8a32328..c36f250a3349 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test'; import { waitForTransaction, waitForError } from '../event-proxy-server'; -test('Should create a transaction for route handlers (GET)', async ({ request }) => { +test('Should create a transaction for route handlers', async ({ request }) => { const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { return transactionEvent?.transaction === 'GET /route-handlers/[param]'; }); @@ -15,7 +15,9 @@ test('Should create a transaction for route handlers (GET)', async ({ request }) expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); }); -test('Should create a transaction for route handlers (POST)', async ({ request }) => { +test('Should create a transaction for route handlers and correctly set span status depending on http status', async ({ + request, +}) => { const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { return transactionEvent?.transaction === 'POST /route-handlers/[param]'; }); @@ -49,3 +51,42 @@ test('Should record exceptions and transactions for faulty route handlers', asyn expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-error'); expect(routehandlerError.tags?.transaction).toBe('PUT /route-handlers/[param]/error'); }); + +test.describe('Edge runtime', () => { + test('should create a transaction for route handlers', async ({ request }) => { + const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge'; + }); + + const response = await request.patch('/route-handlers/bar/edge'); + expect(await response.json()).toStrictEqual({ name: 'John Doe' }); + + const routehandlerTransaction = await routehandlerTransactionPromise; + + expect(routehandlerTransaction.contexts?.trace?.status).toBe('unauthenticated'); + expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); + }); + + test('should record exceptions and transactions for faulty route handlers', async ({ request }) => { + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'route-handler-edge-error'; + }); + + const routehandlerTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'DELETE /route-handlers/[param]/edge'; + }); + + void request.delete('/route-handlers/baz/edge'); + + const routehandlerTransaction = await routehandlerTransactionPromise; + const routehandlerError = await errorEventPromise; + + expect(routehandlerTransaction.contexts?.trace?.status).toBe('internal_error'); + expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); + expect(routehandlerTransaction.contexts?.runtime?.name).toBe('edge'); + + expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-edge-error'); + expect(routehandlerError.tags?.transaction).toBe('DELETE /route-handlers/[param]/edge'); + expect(routehandlerError.contexts?.runtime?.name).toBe('edge'); + }); +}); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index c70840a8c412..5ca49b798aca 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -177,7 +177,7 @@ export function constructWebpackConfigFunction( const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); return ( normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|ts|jsx|tsx)$/) + !!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|ts)$/) ); }; @@ -290,7 +290,8 @@ export function constructWebpackConfigFunction( isPageResource(resourcePath) || isApiRouteResource(resourcePath) || isMiddlewareResource(resourcePath) || - isServerComponentResource(resourcePath) + isServerComponentResource(resourcePath) || + isRouteHandlerResource(resourcePath) ); }, use: [ From f095cf6b3da3b6894aa7e957443a8b0e4859babf Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 11:20:38 +0000 Subject: [PATCH 10/15] . --- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 5e7788c4186b..05219ce26761 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -55,7 +55,9 @@ export function wrapRouteHandlerWithSentry any>( }, ); - if (!platformSupportsStreaming()) { + if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { + // 1. Edge tranpsort requires manual flushing + // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent await flush(1000); } From bf14c8aa9a64ec510c7ba84b6780b192e7ffa6a4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 11:38:11 +0000 Subject: [PATCH 11/15] properly flush --- .../src/common/wrapRouteHandlerWithSentry.ts | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 05219ce26761..e61e1a598bd7 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -28,37 +28,40 @@ export function wrapRouteHandlerWithSentry any>( ); currentScope.setPropagationContext(propagationContext); - const res = await trace( - { - op: 'http.server', - name: `${method} ${parameterizedRoute}`, - status: 'ok', - ...traceparentData, - metadata: { - source: 'route', - dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + let res; + try { + res = await trace( + { + op: 'http.server', + name: `${method} ${parameterizedRoute}`, + status: 'ok', + ...traceparentData, + metadata: { + source: 'route', + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + }, }, - }, - async span => { - const response: Response = await originalFunction.apply(thisArg, args); + async span => { + const response: Response = await originalFunction.apply(thisArg, args); - try { - span?.setHttpStatus(response.status); - } catch { - // best effort - } + try { + span?.setHttpStatus(response.status); + } catch { + // best effort + } - return response; - }, - error => { - captureException(error); - }, - ); - - if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { - // 1. Edge tranpsort requires manual flushing - // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent - await flush(1000); + return response; + }, + error => { + captureException(error); + }, + ); + } finally { + if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { + // 1. Edge tranpsort requires manual flushing + // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent + await flush(1000); + } } return res; From 12aa9f91fbd666d1448391a9c0555c45e967611f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 13:18:24 +0000 Subject: [PATCH 12/15] Fix test --- .../nextjs-app-dir/tests/route-handlers.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index c36f250a3349..ce4f1a8fc0ea 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -86,7 +86,6 @@ test.describe('Edge runtime', () => { expect(routehandlerTransaction.contexts?.runtime?.name).toBe('edge'); expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-edge-error'); - expect(routehandlerError.tags?.transaction).toBe('DELETE /route-handlers/[param]/edge'); expect(routehandlerError.contexts?.runtime?.name).toBe('edge'); }); }); From cef9ee7ba41dcfa131b32a89c7cea7ec19ec2a23 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 29 Aug 2023 13:44:24 +0000 Subject: [PATCH 13/15] hm --- .../nextjs-app-dir/tests/route-handlers.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index ce4f1a8fc0ea..566df1abb1d1 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -40,7 +40,9 @@ test('Should record exceptions and transactions for faulty route handlers', asyn return transactionEvent?.transaction === 'PUT /route-handlers/[param]/error'; }); - void request.put('/route-handlers/baz/error'); + await request.put('/route-handlers/baz/error').catch(() => { + // noop + }); const routehandlerTransaction = await routehandlerTransactionPromise; const routehandlerError = await errorEventPromise; @@ -76,7 +78,9 @@ test.describe('Edge runtime', () => { return transactionEvent?.transaction === 'DELETE /route-handlers/[param]/edge'; }); - void request.delete('/route-handlers/baz/edge'); + await request.delete('/route-handlers/baz/edge').catch(() => { + // noop + }); const routehandlerTransaction = await routehandlerTransactionPromise; const routehandlerError = await errorEventPromise; From 7e9415dc47b6bf0d27c0c51c4cd0ce3fb037bfc3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 30 Aug 2023 08:46:20 +0000 Subject: [PATCH 14/15] Apply review feedback --- .../nextjs-app-dir/sentry.edge.config.ts | 1 - .../nextjs/src/common/wrapRouteHandlerWithSentry.ts | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts index 22c9813c8702..ad780407a5b7 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/sentry.edge.config.ts @@ -5,5 +5,4 @@ Sentry.init({ dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1.0, - debug: true, }); diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index e61e1a598bd7..33ea97e5fe0c 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,5 +1,5 @@ import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; -import { tracingContextFromHeaders } from '@sentry/utils'; +import { addExceptionMechanism, tracingContextFromHeaders } from '@sentry/utils'; import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; @@ -53,7 +53,16 @@ export function wrapRouteHandlerWithSentry any>( return response; }, error => { - captureException(error); + captureException(error, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + handled: false, + }); + return event; + }); + + return scope; + }); }, ); } finally { From ea8765916f41bb27444d967dcd597b1589a57892 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 30 Aug 2023 10:26:26 +0000 Subject: [PATCH 15/15] Don't capture errors for redirects --- .../src/common/wrapRouteHandlerWithSentry.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 33ea97e5fe0c..a4d7a96b94fd 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,6 +1,7 @@ import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; import { addExceptionMechanism, tracingContextFromHeaders } from '@sentry/utils'; +import { isRedirectNavigationError } from './nextNavigationErrorUtils'; import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; @@ -53,16 +54,19 @@ export function wrapRouteHandlerWithSentry any>( return response; }, error => { - captureException(error, scope => { - scope.addEventProcessor(event => { - addExceptionMechanism(event, { - handled: false, + // Next.js throws errors when calling `redirect()`. We don't wanna report these. + if (!isRedirectNavigationError(error)) { + captureException(error, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + handled: false, + }); + return event; }); - return event; - }); - return scope; - }); + return scope; + }); + } }, ); } finally {