From 90c642a0f19092c991b51d18d0d950f2d17d52f9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 7 Mar 2023 13:14:17 +0000 Subject: [PATCH 01/24] Add some test routes to E2E tests and stuff --- .../[...parameters]/captureException/route.ts | 41 ++++++++++++ .../[...parameters]/error/route.ts | 27 ++++++++ .../dynamic-route/[...parameters]/route.ts | 33 +++++++++ .../[parameter]/captureException/route.ts | 41 ++++++++++++ .../dynamic-route/[parameter]/error/route.ts | 27 ++++++++ .../app/dynamic-route/[parameter]/route.ts | 33 +++++++++ .../nextjs-app-dir/app/dynamic-route/route.ts | 29 ++++++++ .../nextjs-app-dir/app/static-route/route.ts | 5 ++ .../nextjs-app-dir/tests/exceptions.test.ts | 56 ++++++---------- .../nextjs-app-dir/tests/transactions.test.ts | 67 ++----------------- .../nextjs-app-dir/tsconfig.json | 9 ++- .../playwright-poll-sentry-event.ts | 37 ++++++++++ 12 files changed, 308 insertions(+), 97 deletions(-) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/static-route/route.ts create mode 100644 packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts new file mode 100644 index 000000000000..30d5d10a8c8e --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts @@ -0,0 +1,41 @@ +import { NextResponse } from 'next/server'; +import * as Sentry from '@sentry/nextjs'; + +interface Params { + params: Record; +} + +export async function GET(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +} + +export async function POST(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +} + +export async function PUT(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +} + +export async function PATCH(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +} + +export async function DELETE(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +} + +export async function HEAD(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); +} + +export async function OPTIONS(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts new file mode 100644 index 000000000000..ee53b56087c1 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts @@ -0,0 +1,27 @@ +export async function GET() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function POST() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function PUT() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function PATCH() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function DELETE() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function HEAD() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function OPTIONS() { + throw new Error('I am an error inside a dynamic route!'); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts new file mode 100644 index 000000000000..e35c98878021 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts @@ -0,0 +1,33 @@ +import { NextResponse } from 'next/server'; + +interface Params { + params: Record; +} + +export async function GET(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +} + +export async function POST(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +} + +export async function PUT(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +} + +export async function PATCH(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +} + +export async function DELETE(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +} + +export async function HEAD(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); +} + +export async function OPTIONS(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts new file mode 100644 index 000000000000..30d5d10a8c8e --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts @@ -0,0 +1,41 @@ +import { NextResponse } from 'next/server'; +import * as Sentry from '@sentry/nextjs'; + +interface Params { + params: Record; +} + +export async function GET(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +} + +export async function POST(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +} + +export async function PUT(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +} + +export async function PATCH(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +} + +export async function DELETE(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +} + +export async function HEAD(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); +} + +export async function OPTIONS(_request: Request, { params }: Params) { + Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts new file mode 100644 index 000000000000..ee53b56087c1 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts @@ -0,0 +1,27 @@ +export async function GET() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function POST() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function PUT() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function PATCH() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function DELETE() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function HEAD() { + throw new Error('I am an error inside a dynamic route!'); +} + +export async function OPTIONS() { + throw new Error('I am an error inside a dynamic route!'); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts new file mode 100644 index 000000000000..e35c98878021 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts @@ -0,0 +1,33 @@ +import { NextResponse } from 'next/server'; + +interface Params { + params: Record; +} + +export async function GET(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +} + +export async function POST(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +} + +export async function PUT(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +} + +export async function PATCH(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +} + +export async function DELETE(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +} + +export async function HEAD(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); +} + +export async function OPTIONS(_request: Request, { params }: Params) { + return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts new file mode 100644 index 000000000000..66a244d6f7d0 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts @@ -0,0 +1,29 @@ +import { NextResponse } from 'next/server'; + +export async function GET() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'GET' }); +} + +export async function POST() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'POST' }); +} + +export async function PUT() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PUT' }); +} + +export async function PATCH() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PATCH' }); +} + +export async function DELETE() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'DELETE' }); +} + +export async function HEAD() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'HEAD' }); +} + +export async function OPTIONS() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'OPTIONS' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/static-route/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/static-route/route.ts new file mode 100644 index 000000000000..0b6091c436a7 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/static-route/route.ts @@ -0,0 +1,5 @@ +import { NextResponse } from 'next/server'; + +export async function GET() { + return NextResponse.json({ data: 'I am a static route!', method: 'GET' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index ea96490b79ce..bd83eec16098 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -1,13 +1,8 @@ -import { test, expect } from '@playwright/test'; +import { expect, test } from '@playwright/test'; import { waitForError } from '../../../test-utils/event-proxy-server'; -import axios, { AxiosError } from 'axios'; +import { pollEventOnSentry } from '../../../test-utils/playwright-poll-sentry-event'; -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; -const EVENT_POLLING_TIMEOUT = 30_000; - -test('Sends a client-side exception to Sentry', async ({ page }) => { +test('Sends an ingestable client-side exception to Sentry', async ({ page }) => { await page.goto('/'); const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { @@ -19,31 +14,22 @@ test('Sends a client-side exception to Sentry', async ({ page }) => { const errorEvent = await errorEventPromise; const exceptionEventId = errorEvent.event_id; - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); + expect(exceptionEventId).toBeDefined(); + await pollEventOnSentry(exceptionEventId!); }); + +// test('Sends an ingestable route handler exception to Sentry', async ({ page }) => { +// await page.goto('/'); + +// const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { +// return errorEvent?.exception?.values?.[0]?.value === 'Click Error'; +// }); + +// await page.getByText('Throw error').click(); + +// const errorEvent = await errorEventPromise; +// const exceptionEventId = errorEvent.event_id; + +// expect(exceptionEventId).toBeDefined(); +// await pollEventOnSentry(exceptionEventId!); +// }); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index 5ef4e6f28b5f..0745cf4c4a2e 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -1,13 +1,8 @@ import { test, expect } from '@playwright/test'; import { waitForTransaction } from '../../../test-utils/event-proxy-server'; -import axios, { AxiosError } from 'axios'; +import { pollEventOnSentry } from '../../../test-utils/playwright-poll-sentry-event'; -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; -const EVENT_POLLING_TIMEOUT = 30_000; - -test('Sends a pageload transaction', async ({ page }) => { +test('Sends an ingestable pageload transaction to Sentry', async ({ page }) => { const pageloadTransactionEventPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { return transactionEvent?.contexts?.trace?.op === 'pageload' && transactionEvent?.transaction === '/'; }); @@ -17,33 +12,8 @@ test('Sends a pageload transaction', async ({ page }) => { const transactionEvent = await pageloadTransactionEventPromise; const transactionEventId = transactionEvent.event_id; - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); + expect(transactionEventId).toBeDefined(); + await pollEventOnSentry(transactionEventId!); }); if (process.env.TEST_ENV === 'production') { @@ -61,32 +31,7 @@ if (process.env.TEST_ENV === 'production') { const transactionEvent = await serverComponentTransactionPromise; const transactionEventId = transactionEvent.event_id; - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); + expect(transactionEventId).toBeDefined(); + await pollEventOnSentry(transactionEventId!); }); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tsconfig.json b/packages/e2e-tests/test-applications/nextjs-app-dir/tsconfig.json index bacd391b697e..bdc68535c5ad 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tsconfig.json +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tsconfig.json @@ -20,7 +20,14 @@ } ] }, - "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", "next.config.js", ".next/types/**/*.ts"], + "include": [ + "next-env.d.ts", + "**/*.ts", + "**/*.tsx", + "next.config.js", + ".next/types/**/*.ts", + "../../test-utils/**/*.ts" + ], "exclude": ["node_modules"], "ts-node": { "compilerOptions": { diff --git a/packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts b/packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts new file mode 100644 index 000000000000..ca0d2884ffca --- /dev/null +++ b/packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts @@ -0,0 +1,37 @@ +import { expect } from '@playwright/test'; +import axios, { AxiosError } from 'axios'; + +const authToken = process.env.E2E_TEST_AUTH_TOKEN; +const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; +const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; +const EVENT_POLLING_TIMEOUT = 30_000; + +export async function pollEventOnSentry(eventId: string): Promise { + await expect + .poll( + async () => { + try { + const response = await axios.get( + `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${eventId}/`, + { headers: { Authorization: `Bearer ${authToken}` } }, + ); + + return response.status; + } catch (e) { + if (e instanceof AxiosError && e.response) { + if (e.response.status !== 404) { + throw e; + } else { + return e.response.status; + } + } else { + throw e; + } + } + }, + { + timeout: EVENT_POLLING_TIMEOUT, + }, + ) + .toBe(200); +} From e2a5ccd90c8f13fe68fb761437b57d9d44851b02 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 7 Mar 2023 14:09:09 +0000 Subject: [PATCH 02/24] . --- .../src/common/wrapRouteHandlerWithSentry.ts | 38 +++++++++++++++++++ packages/utils/src/is.ts | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts new file mode 100644 index 000000000000..c3868adf3db9 --- /dev/null +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -0,0 +1,38 @@ +import { captureException } from '@sentry/core'; +import { isThenable } from '@sentry/utils'; + +interface WrappingParams { + method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'HEAD' | 'OPTIONS'; +} + +/** + * TODO + */ +export function wrapRouteHandlerWithSentry unknown>( + originalRouteHandler: H, + wrappingParams: WrappingParams, +): H { + return new Proxy(originalRouteHandler, { + apply: (wrappingTarget, thisArg: unknown, args: unknown[]) => { + const handleError = (e: unknown): never => { + captureException(e); + throw e; + }; + + let maybePromiseResult: unknown; + try { + maybePromiseResult = wrappingTarget.apply(thisArg, args); + } catch (e) { + handleError(e); + } + + if (isThenable(maybePromiseResult)) { + return maybePromiseResult.then(null, (err: unknown) => { + handleError(err); + }); + } else { + return maybePromiseResult; + } + }, + }); +} diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts index 350826cb567c..36c5a6991436 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -139,7 +139,7 @@ export function isRegExp(wat: unknown): wat is RegExp { */ export function isThenable(wat: any): wat is PromiseLike { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return Boolean(wat && wat.then && typeof wat.then === 'function'); + return Boolean(typeof wat === 'object' && wat !== null && 'then' in wat.then && typeof wat.then === 'function'); } /** From 4406d82019076db20fd604b99f81bf738e62da82 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Mar 2023 10:56:48 +0000 Subject: [PATCH 03/24] . --- .../wrapFunctionWithErrorInstrumentation.ts | 0 ...pFunctionWithPerformanceInstrumentation.ts | 171 ++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts create mode 100644 packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts diff --git a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts new file mode 100644 index 000000000000..643a7ac9fff1 --- /dev/null +++ b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts @@ -0,0 +1,171 @@ +import { getCurrentHub, hasTracingEnabled, startTransaction } from '@sentry/core'; +import type { Mechanism, Span, Transaction } from '@sentry/types'; +import { baggageHeaderToDynamicSamplingContext, extractTraceparentData, isThenable } from '@sentry/utils'; + +interface WrapperContext { + sentryTraceHeader?: string; + sentryBaggageHeader?: string; + requestContextObject?: object; + syntheticParentTransaction?: Transaction; +} + +interface WrapperContextExtractor { + (this: unknown, functionArgs: Args): WrapperContext; +} + +interface SpanInfo { + op: string; + name: string; + data: Record; + mechanism: Partial; +} + +interface SpanInfoCreator { + (this: unknown, functionArgs: Args, context: { willCreateTransaction: boolean }): SpanInfo; +} + +interface WrappedReturnValue { + returnValue: V; + usedSyntheticTransaction: Transaction | undefined; +} + +const requestContextTransactionMap = new WeakMap(); +const requestContextSyntheticTransactionMap = new WeakMap(); + +/** + * TODO + */ +export function wrapRequestLikeFunctionWithPerformanceInstrumentation any>( + originalFunction: F, + wrapperContextExtractor: WrapperContextExtractor, + spanInfoCreator: SpanInfoCreator, +): (...args: Parameters) => WrappedReturnValue> { + if (!hasTracingEnabled()) { + return new Proxy(originalFunction, { + apply: (originalFunction, thisArg: unknown, args: unknown[]): WrappedReturnValue> => { + return originalFunction.apply(thisArg, args); + }, + }); + } + + return new Proxy(originalFunction, { + apply: (originalFunction, thisArg: unknown, args: unknown[]): WrappedReturnValue> => { + const currentScope = getCurrentHub().getScope(); + + const wrapperContext: WrapperContext = wrapperContextExtractor.apply(thisArg, [args]); + + let parentSpan: Span | undefined = currentScope?.getSpan(); + + if (!parentSpan && wrapperContext.requestContextObject) { + parentSpan = requestContextTransactionMap.get(wrapperContext.requestContextObject); + } + + const spanInfo: SpanInfo = spanInfoCreator.apply(thisArg, [args, { willCreateTransaction: !!parentSpan }]); + + let span: Span; + let usedSyntheticTransaction: Transaction | undefined; + if (parentSpan) { + span = parentSpan.startChild({ + description: spanInfo.name, + op: spanInfo.op, + }); + } else { + let traceparentData; + if (wrapperContext.sentryTraceHeader) { + traceparentData = extractTraceparentData(wrapperContext.sentryTraceHeader); + } else { + if (wrapperContext.requestContextObject) { + usedSyntheticTransaction = requestContextSyntheticTransactionMap.get(wrapperContext.requestContextObject); + } + + if ( + wrapperContext.requestContextObject && + wrapperContext.syntheticParentTransaction && + !usedSyntheticTransaction + ) { + requestContextSyntheticTransactionMap.set( + wrapperContext.requestContextObject, + wrapperContext.syntheticParentTransaction, + ); + } + + if (wrapperContext.syntheticParentTransaction && !usedSyntheticTransaction) { + usedSyntheticTransaction = wrapperContext.syntheticParentTransaction; + } + + if (usedSyntheticTransaction) { + traceparentData = { + traceId: usedSyntheticTransaction.traceId, + parentSpanId: usedSyntheticTransaction.spanId, + }; + } + } + + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(wrapperContext.sentryBaggageHeader); + + const transaction = startTransaction({ + name: spanInfo.name, + op: spanInfo.op, + ...traceparentData, + status: 'ok', + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source: 'route', + }, + }); + + span = transaction; + + if (wrapperContext.requestContextObject) { + requestContextTransactionMap.set(wrapperContext.requestContextObject, transaction); + } + } + + if (currentScope) { + currentScope.setSpan(span); + } + + const handleFunctionError = (): void => { + span.setStatus('internal_error'); + }; + + const handleFunctionEnd = (): void => { + span.finish(); + }; + + let maybePromiseResult: ReturnType; + try { + maybePromiseResult = originalFunction.apply(thisArg, args); + } catch (e) { + handleFunctionError(); + handleFunctionEnd(); + throw e; + } + + if (isThenable(maybePromiseResult)) { + const promiseResult = maybePromiseResult.then( + (res: unknown) => { + handleFunctionEnd(); + return res; + }, + (err: unknown) => { + handleFunctionError(); + handleFunctionEnd(); + throw err; + }, + ); + + return { + returnValue: promiseResult, + usedSyntheticTransaction, + }; + } else { + handleFunctionEnd(); + return { + returnValue: maybePromiseResult, + usedSyntheticTransaction, + }; + } + }, + }); +} From 0268bfb260ca1314a276cd07ad4a477d7a112c55 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Mar 2023 15:11:28 +0000 Subject: [PATCH 04/24] . --- .../wrapFunctionWithErrorInstrumentation.ts | 61 +++++++++++++++++++ ...pFunctionWithPerformanceInstrumentation.ts | 60 ++++++++++++------ 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts index e69de29bb2d1..a1e4145af2d5 100644 --- a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts @@ -0,0 +1,61 @@ +import { captureException, getCurrentHub } from '@sentry/core'; +import { addExceptionMechanism, isThenable } from '@sentry/utils'; + +interface ErrorInfo { + wrappingTargetName: string; +} + +interface ErrorInfoCreator { + (functionArgs: Args): ErrorInfo; +} + +/** + * TODO + */ +export function wrapRequestLikeFunctionWithErrorInstrumentation any>( + originalFunction: F, + errorInfoCreator: ErrorInfoCreator, +): (...args: Parameters) => ReturnType { + return new Proxy(originalFunction, { + apply: (originalFunction, thisArg: unknown, args: Parameters): ReturnType => { + const errorInfo: ErrorInfo = errorInfoCreator(args); + + const scope = getCurrentHub().getScope(); + if (scope) { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'instrument', + handled: false, + data: { + wrapped_function: errorInfo.wrappingTargetName, + }, + }); + return event; + }); + } + + const handleError = (error: unknown): void => { + captureException(error); + }; + + let maybePromiseResult: ReturnType; + try { + maybePromiseResult = originalFunction.apply(thisArg, args); + } catch (err) { + handleError(err); + throw err; + } + + if (isThenable(maybePromiseResult)) { + const promiseResult = maybePromiseResult.then(null, (err: unknown) => { + handleError(err); + throw err; + }); + + return promiseResult; + } else { + return maybePromiseResult; + } + }, + }); +} diff --git a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts index 643a7ac9fff1..15be7bb14b78 100644 --- a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts +++ b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts @@ -1,5 +1,5 @@ import { getCurrentHub, hasTracingEnabled, startTransaction } from '@sentry/core'; -import type { Mechanism, Span, Transaction } from '@sentry/types'; +import type { Span, Transaction } from '@sentry/types'; import { baggageHeaderToDynamicSamplingContext, extractTraceparentData, isThenable } from '@sentry/utils'; interface WrapperContext { @@ -10,18 +10,30 @@ interface WrapperContext { } interface WrapperContextExtractor { - (this: unknown, functionArgs: Args): WrapperContext; + (functionArgs: Args, finishSpan: () => void): WrapperContext; } interface SpanInfo { op: string; name: string; - data: Record; - mechanism: Partial; + data?: Record; } interface SpanInfoCreator { - (this: unknown, functionArgs: Args, context: { willCreateTransaction: boolean }): SpanInfo; + (functionArgs: Args, context: { willCreateTransaction: boolean }): SpanInfo; +} + +interface OnFunctionEndHookResult { + shouldFinishSpan: boolean; +} + +interface OnFunctionEndHook { + ( + functionArgs: Args, + span: Span, + result: R | undefined, + error: unknown | undefined, + ): PromiseLike; } interface WrappedReturnValue { @@ -39,6 +51,7 @@ export function wrapRequestLikeFunctionWithPerformanceInstrumentation, spanInfoCreator: SpanInfoCreator, + onFunctionEnd: OnFunctionEndHook>, ): (...args: Parameters) => WrappedReturnValue> { if (!hasTracingEnabled()) { return new Proxy(originalFunction, { @@ -49,10 +62,16 @@ export function wrapRequestLikeFunctionWithPerformanceInstrumentation> => { + apply: (originalFunction, thisArg: unknown, args: A): WrappedReturnValue> => { const currentScope = getCurrentHub().getScope(); - const wrapperContext: WrapperContext = wrapperContextExtractor.apply(thisArg, [args]); + const userSpanFinish = (): void => { + if (span) { + span.finish(); + } + }; + + const wrapperContext = wrapperContextExtractor(args, userSpanFinish); let parentSpan: Span | undefined = currentScope?.getSpan(); @@ -60,7 +79,7 @@ export function wrapRequestLikeFunctionWithPerformanceInstrumentation { - span.finish(); + const handleFunctionEnd = (res: ReturnType | undefined, err: unknown | undefined): void => { + void onFunctionEnd(args, span, res, err).then(beforeFinishResult => { + if (!beforeFinishResult.shouldFinishSpan) { + span.finish(); + } + }); }; let maybePromiseResult: ReturnType; try { maybePromiseResult = originalFunction.apply(thisArg, args); - } catch (e) { + } catch (err) { handleFunctionError(); - handleFunctionEnd(); - throw e; + handleFunctionEnd(undefined, err); + throw err; } if (isThenable(maybePromiseResult)) { const promiseResult = maybePromiseResult.then( - (res: unknown) => { - handleFunctionEnd(); + (res: ReturnType) => { + handleFunctionEnd(res, undefined); return res; }, (err: unknown) => { handleFunctionError(); - handleFunctionEnd(); + handleFunctionEnd(undefined, err); throw err; }, ); @@ -160,7 +186,7 @@ export function wrapRequestLikeFunctionWithPerformanceInstrumentation Date: Wed, 8 Mar 2023 15:15:55 +0000 Subject: [PATCH 05/24] . --- .../src/common/wrapFunctionWithErrorInstrumentation.ts | 2 +- .../src/common/wrapFunctionWithPerformanceInstrumentation.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts index a1e4145af2d5..67f2246c2150 100644 --- a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts @@ -12,7 +12,7 @@ interface ErrorInfoCreator { /** * TODO */ -export function wrapRequestLikeFunctionWithErrorInstrumentation any>( +export function wrapRequestHandlerLikeFunctionWithErrorInstrumentation any>( originalFunction: F, errorInfoCreator: ErrorInfoCreator, ): (...args: Parameters) => ReturnType { diff --git a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts index 15be7bb14b78..b061a319ef4f 100644 --- a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts +++ b/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts @@ -47,7 +47,10 @@ const requestContextSyntheticTransactionMap = new WeakMap() /** * TODO */ -export function wrapRequestLikeFunctionWithPerformanceInstrumentation any>( +export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< + A extends any[], + F extends (...args: A) => any, +>( originalFunction: F, wrapperContextExtractor: WrapperContextExtractor, spanInfoCreator: SpanInfoCreator, From e50712a38a89656ae092d1661c076118df9a4d9d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Mar 2023 15:53:31 +0000 Subject: [PATCH 06/24] . --- .../wrapFunctionWithErrorInstrumentation.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts index 67f2246c2150..84ea2b3890ab 100644 --- a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts @@ -9,12 +9,21 @@ interface ErrorInfoCreator { (functionArgs: Args): ErrorInfo; } +interface BeforeCaptureErrorHookResult { + skipCapturingError?: boolean; +} + +interface BeforeCaptureErrorHook { + (functionArgs: Args, error: unknown): PromiseLike; +} + /** * TODO */ export function wrapRequestHandlerLikeFunctionWithErrorInstrumentation any>( originalFunction: F, errorInfoCreator: ErrorInfoCreator, + beforeCaptureError: BeforeCaptureErrorHook, ): (...args: Parameters) => ReturnType { return new Proxy(originalFunction, { apply: (originalFunction, thisArg: unknown, args: Parameters): ReturnType => { @@ -34,21 +43,25 @@ export function wrapRequestHandlerLikeFunctionWithErrorInstrumentation { - captureException(error); + const reportError = (error: unknown): void => { + void beforeCaptureError(args, error).then(beforeCaptureErrorResult => { + if (!beforeCaptureErrorResult.skipCapturingError) { + captureException(error); + } + }); }; let maybePromiseResult: ReturnType; try { maybePromiseResult = originalFunction.apply(thisArg, args); } catch (err) { - handleError(err); + reportError(err); throw err; } if (isThenable(maybePromiseResult)) { const promiseResult = maybePromiseResult.then(null, (err: unknown) => { - handleError(err); + reportError(err); throw err; }); From d866ee286d54dd529bf7332451d23057fe19c1dc Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Mar 2023 16:05:48 +0000 Subject: [PATCH 07/24] . --- ... => wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts} | 0 ...apRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/nextjs/src/common/{wrapFunctionWithErrorInstrumentation.ts => wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts} (100%) rename packages/nextjs/src/common/{wrapFunctionWithPerformanceInstrumentation.ts => wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts} (100%) diff --git a/packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts similarity index 100% rename from packages/nextjs/src/common/wrapFunctionWithErrorInstrumentation.ts rename to packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts diff --git a/packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts similarity index 100% rename from packages/nextjs/src/common/wrapFunctionWithPerformanceInstrumentation.ts rename to packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts From 25586a8b5947a3752b14d3fcf167ad50e5b26d17 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Mar 2023 21:24:53 +0000 Subject: [PATCH 08/24] . --- packages/nextjs/rollup.npm.config.js | 1 + packages/nextjs/src/common/types.ts | 7 ++ ...lerLikeFunctionWithErrorInstrumentation.ts | 8 ++- ...eFunctionWithPerformanceInstrumentation.ts | 58 +++++++-------- .../src/common/wrapRouteHandlerWithSentry.ts | 38 ---------- .../src/config/loaders/wrappingLoader.ts | 28 +++++++- .../templates/routeHandlerWrapperTemplate.ts | 70 +++++++++++++++++++ packages/nextjs/src/config/webpack.ts | 22 +++++- packages/nextjs/src/index.types.ts | 10 ++- packages/nextjs/src/server/index.ts | 1 + .../src/server/wrapRouteHandlerWithSentry.ts | 69 ++++++++++++++++++ packages/utils/src/is.ts | 2 +- 12 files changed, 242 insertions(+), 72 deletions(-) delete mode 100644 packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts create mode 100644 packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts create mode 100644 packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 19e349f70f8f..e8238ab36d88 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -28,6 +28,7 @@ export default [ 'src/config/templates/apiWrapperTemplate.ts', 'src/config/templates/middlewareWrapperTemplate.ts', 'src/config/templates/serverComponentWrapperTemplate.ts', + 'src/config/templates/routeHandlerWrapperTemplate.ts', ], packageSpecificConfig: { diff --git a/packages/nextjs/src/common/types.ts b/packages/nextjs/src/common/types.ts index cfac0c460a84..439c9fb098ab 100644 --- a/packages/nextjs/src/common/types.ts +++ b/packages/nextjs/src/common/types.ts @@ -4,3 +4,10 @@ export type ServerComponentContext = { sentryTraceHeader?: string; baggageHeader?: string; }; + +export type RouteHandlerContext = { + method: 'GET' | 'PUT' | 'POST' | 'PATCH' | 'DELETE' | 'PATCH' | 'OPTIONS'; + parameterizedRoute: string; + sentryTraceHeader?: string; + baggageHeader?: string; +}; diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts index 84ea2b3890ab..f58458c80cc4 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts @@ -17,13 +17,19 @@ interface BeforeCaptureErrorHook { (functionArgs: Args, error: unknown): PromiseLike; } +const defaultBeforeCaptureError = async (): Promise => { + return { + skipCapturingError: false, + }; +}; + /** * TODO */ export function wrapRequestHandlerLikeFunctionWithErrorInstrumentation any>( originalFunction: F, errorInfoCreator: ErrorInfoCreator, - beforeCaptureError: BeforeCaptureErrorHook, + beforeCaptureError: BeforeCaptureErrorHook = defaultBeforeCaptureError, ): (...args: Parameters) => ReturnType { return new Proxy(originalFunction, { apply: (originalFunction, thisArg: unknown, args: Parameters): ReturnType => { diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts index b061a319ef4f..ceb0655046ea 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts @@ -3,14 +3,14 @@ import type { Span, Transaction } from '@sentry/types'; import { baggageHeaderToDynamicSamplingContext, extractTraceparentData, isThenable } from '@sentry/utils'; interface WrapperContext { - sentryTraceHeader?: string; - sentryBaggageHeader?: string; + sentryTraceHeader?: string | null; + baggageHeader?: string | null; requestContextObject?: object; syntheticParentTransaction?: Transaction; } -interface WrapperContextExtractor { - (functionArgs: Args, finishSpan: () => void): WrapperContext; +interface WrapperContextExtractor { + (finishSpan: () => void): WrapperContext; } interface SpanInfo { @@ -19,21 +19,16 @@ interface SpanInfo { data?: Record; } -interface SpanInfoCreator { - (functionArgs: Args, context: { willCreateTransaction: boolean }): SpanInfo; +interface SpanInfoCreator { + (context: { willCreateTransaction: boolean }): SpanInfo; } interface OnFunctionEndHookResult { shouldFinishSpan: boolean; } -interface OnFunctionEndHook { - ( - functionArgs: Args, - span: Span, - result: R | undefined, - error: unknown | undefined, - ): PromiseLike; +interface OnFunctionEndHook { + (span: Span, result: R | undefined, error: unknown | undefined): Promise; } interface WrappedReturnValue { @@ -44,6 +39,10 @@ interface WrappedReturnValue { const requestContextTransactionMap = new WeakMap(); const requestContextSyntheticTransactionMap = new WeakMap(); +const defaultOnFunctionEnd = async (): Promise => { + return { shouldFinishSpan: true }; +}; + /** * TODO */ @@ -52,20 +51,21 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< F extends (...args: A) => any, >( originalFunction: F, - wrapperContextExtractor: WrapperContextExtractor, - spanInfoCreator: SpanInfoCreator, - onFunctionEnd: OnFunctionEndHook>, + options: { + wrapperContextExtractor?: WrapperContextExtractor; + spanInfoCreator: SpanInfoCreator; + onFunctionEnd?: OnFunctionEndHook>; + }, ): (...args: Parameters) => WrappedReturnValue> { - if (!hasTracingEnabled()) { - return new Proxy(originalFunction, { - apply: (originalFunction, thisArg: unknown, args: unknown[]): WrappedReturnValue> => { - return originalFunction.apply(thisArg, args); - }, - }); - } - return new Proxy(originalFunction, { apply: (originalFunction, thisArg: unknown, args: A): WrappedReturnValue> => { + if (!hasTracingEnabled()) { + return { + returnValue: originalFunction.apply(thisArg, args), + usedSyntheticTransaction: undefined, + }; + } + const currentScope = getCurrentHub().getScope(); const userSpanFinish = (): void => { @@ -74,7 +74,7 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< } }; - const wrapperContext = wrapperContextExtractor(args, userSpanFinish); + const wrapperContext = options.wrapperContextExtractor?.(userSpanFinish) || {}; let parentSpan: Span | undefined = currentScope?.getSpan(); @@ -82,7 +82,7 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< parentSpan = requestContextTransactionMap.get(wrapperContext.requestContextObject); } - const spanInfo = spanInfoCreator(args, { willCreateTransaction: !!parentSpan }); + const spanInfo = options.spanInfoCreator({ willCreateTransaction: !parentSpan }); let span: Span; let usedSyntheticTransaction: Transaction | undefined; @@ -125,7 +125,7 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< } } - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(wrapperContext.sentryBaggageHeader); + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(wrapperContext.baggageHeader); const transaction = startTransaction({ name: spanInfo.name, @@ -155,8 +155,8 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< }; const handleFunctionEnd = (res: ReturnType | undefined, err: unknown | undefined): void => { - void onFunctionEnd(args, span, res, err).then(beforeFinishResult => { - if (!beforeFinishResult.shouldFinishSpan) { + void (options.onFunctionEnd || defaultOnFunctionEnd)(span, res, err).then(beforeFinishResult => { + if (beforeFinishResult.shouldFinishSpan) { span.finish(); } }); diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts deleted file mode 100644 index c3868adf3db9..000000000000 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { captureException } from '@sentry/core'; -import { isThenable } from '@sentry/utils'; - -interface WrappingParams { - method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'HEAD' | 'OPTIONS'; -} - -/** - * TODO - */ -export function wrapRouteHandlerWithSentry unknown>( - originalRouteHandler: H, - wrappingParams: WrappingParams, -): H { - return new Proxy(originalRouteHandler, { - apply: (wrappingTarget, thisArg: unknown, args: unknown[]) => { - const handleError = (e: unknown): never => { - captureException(e); - throw e; - }; - - let maybePromiseResult: unknown; - try { - maybePromiseResult = wrappingTarget.apply(thisArg, args); - } catch (e) { - handleError(e); - } - - if (isThenable(maybePromiseResult)) { - return maybePromiseResult.then(null, (err: unknown) => { - handleError(err); - }); - } else { - return maybePromiseResult; - } - }, - }); -} diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index fb3e76be72f0..326308811d02 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -23,6 +23,9 @@ 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' }); + // Just a simple placeholder to make referencing module consistent const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; @@ -34,7 +37,7 @@ type LoaderOptions = { appDir: string; pageExtensionRegex: string; excludeServerRoutes: Array; - wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component'; + wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component' | 'route-handler'; }; /** @@ -155,6 +158,29 @@ export default function wrappingLoader( } else { templateCode = templateCode.replace(/__COMPONENT_TYPE__/g, 'Unknown'); } + } else if (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|ts)$/, '') + // 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 + // homepage), sub back in the root route + .replace(/^$/, '/'); + + // Skip explicitly-ignored routes + if (stringMatchesSomePattern(parameterizedPagesRoute, excludeServerRoutes, true)) { + this.callback(null, userCode, userModuleSourceMap); + return; + } + + templateCode = routeHandlerWrapperTemplateCode; + + templateCode = templateCode.replace(/__ROUTE__/g, parameterizedPagesRoute.replace(/\\/g, '\\\\')); } else if (wrappingTargetKind === 'middleware') { templateCode = middlewareWrapperTemplateCode; } else { diff --git a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts new file mode 100644 index 000000000000..360658314732 --- /dev/null +++ b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts @@ -0,0 +1,70 @@ +/* + * This file is a template for the code which will be substituted when our webpack loader handles non-API files in the + * `pages/` directory. + * + * We use `__SENTRY_WRAPPING_TARGET_FILE__` as a placeholder for the path to the file being wrapped. Because it's not a real package, + * this causes both TS and ESLint to complain, hence the pragma comments below. + */ + +// @ts-ignore See above +// eslint-disable-next-line import/no-unresolved +import * as wrapee from '__SENTRY_WRAPPING_TARGET_FILE__'; +// eslint-disable-next-line import/no-extraneous-dependencies +import * as Sentry from '@sentry/nextjs'; +// @ts-ignore This template is only used with the app directory so we know that this dependency exists. +// eslint-disable-next-line import/no-unresolved +import { headers } from 'next/headers'; + +declare function headers(): { get: (header: string) => string | undefined }; + +type ServerComponentModule = { + 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; +}; + +const serverComponentModule = wrapee as ServerComponentModule; + +function wrapHandler(handler: T, method: string): T { + // TODO: explain why + if (process.env.NEXT_PHASE === 'phase-production-build') { + return handler; + } + + if (typeof handler !== 'function') { + return handler; + } + + return new Proxy(handler, { + apply: (originalFunction, thisArg, args) => { + const headersList = headers(); + const sentryTraceHeader = headersList.get('sentry-trace'); + const baggageHeader = headersList.get('baggage'); + + return Sentry.wrapRouteHandlerWithSentry(originalFunction, { + method, + parameterizedRoute: '__ROUTE__', + sentryTraceHeader, + baggageHeader, + }).apply(thisArg, args); + }, + }); +} + +export const GET = wrapHandler(serverComponentModule.GET, 'GET'); +export const POST = wrapHandler(serverComponentModule.POST, 'POST'); +export const PUT = wrapHandler(serverComponentModule.PUT, 'PUT'); +export const PATCH = wrapHandler(serverComponentModule.PATCH, 'PATCH'); +export const DELETE = wrapHandler(serverComponentModule.DELETE, 'DELETE'); +export const HEAD = wrapHandler(serverComponentModule.HEAD, 'HEAD'); +export const OPTIONS = wrapHandler(serverComponentModule.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__'; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 7fd142fc8aa8..b0264563fa77 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -196,7 +196,7 @@ export function constructWebpackConfigFunction( } if (isServer && userSentryOptions.autoInstrumentAppDirectory !== false) { - // Wrap page server components + // Wrap server components newConfig.module.rules.unshift({ test: resourcePath => { const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); @@ -218,6 +218,26 @@ export function constructWebpackConfigFunction( }, ], }); + + // Wrap route handlers + newConfig.module.rules.unshift({ + test: resourcePath => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(appDirPath) && + !!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|ts)$/) + ); + }, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), + options: { + ...staticWrappingLoaderOptions, + wrappingTargetKind: 'route-handler', + }, + }, + ], + }); } // The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index b0cd45e43084..34a02a0a5739 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -10,7 +10,7 @@ export * from './edge'; import type { Integration, Options, StackParser } from '@sentry/types'; import type * as clientSdk from './client'; -import type { ServerComponentContext } from './common/types'; +import type { RouteHandlerContext, ServerComponentContext } from './common/types'; import type * as edgeSdk from './edge'; import type * as serverSdk from './server'; @@ -177,3 +177,11 @@ export declare function wrapServerComponentWithSentry any>( + WrappingTarget: F, + context: RouteHandlerContext, +): F; diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index b38ee49946f2..ecebcd772a8c 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -229,3 +229,4 @@ export { } from './wrapApiHandlerWithSentry'; export { wrapServerComponentWithSentry } from './wrapServerComponentWithSentry'; +export { wrapRouteHandlerWithSentry } from './wrapRouteHandlerWithSentry'; diff --git a/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts new file mode 100644 index 000000000000..ed583d16032f --- /dev/null +++ b/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts @@ -0,0 +1,69 @@ +import * as domain from 'domain'; + +import type { RouteHandlerContext } from '../common/types'; +import { wrapRequestHandlerLikeFunctionWithErrorInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation'; +import { wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation'; +import { getCurrentHub } from '../edge'; + +type RouteHandlerArgs = [Request | undefined, { params?: Record } | undefined]; + +/** + * Wraps an `app` directory server component with Sentry error instrumentation. + */ +export function wrapRouteHandlerWithSentry unknown>( + routeHandler: F, + context: RouteHandlerContext, +): F { + return new Proxy(routeHandler, { + apply: (originalFunction, thisArg, args: Parameters) => { + return domain.create().bind(() => { + const errorWrappedFunction = wrapRequestHandlerLikeFunctionWithErrorInstrumentation(originalFunction, () => ({ + wrappingTargetName: context.method, + })); + + const req = args[0]; + const routeConfiguration = args[1]; + + const sendDefaultPiiOption = getCurrentHub().getClient()?.getOptions().sendDefaultPii; + let routeParameters: Record = {}; + + if (sendDefaultPiiOption && routeConfiguration?.params) { + routeParameters = routeConfiguration?.params; + } + + const errorAndPerformanceWrappedFunction = wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation( + errorWrappedFunction, + { + wrapperContextExtractor: () => ({ + requestContextObject: req, + baggageHeader: req?.headers.get('baggage'), + sentryTraceHeader: req?.headers.get('sentry-trace'), + }), + spanInfoCreator: ({ willCreateTransaction }) => { + if (willCreateTransaction) { + return { + name: `${context.method} ${context.parameterizedRoute}`, + op: 'http.server', + data: { + routeParameters, + }, + }; + } else { + return { + name: `${context.method}()`, + op: 'function', + data: { + route: context.parameterizedRoute, + }, + }; + } + }, + }, + ); + + const { returnValue } = errorAndPerformanceWrappedFunction.apply(thisArg, args); + return returnValue; + })(); + }, + }); +} diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts index 36c5a6991436..d380ef50b8aa 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -139,7 +139,7 @@ export function isRegExp(wat: unknown): wat is RegExp { */ export function isThenable(wat: any): wat is PromiseLike { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return Boolean(typeof wat === 'object' && wat !== null && 'then' in wat.then && typeof wat.then === 'function'); + return Boolean(typeof wat === 'object' && wat !== null && 'then' in wat && typeof wat.then === 'function'); } /** From 2054789e7d7729f959ea7f4822868895054c5132 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 08:12:27 +0000 Subject: [PATCH 09/24] . --- .../dynamic-route/[...parameters]/route.ts | 32 ++++++++----------- .../app/dynamic-route/[parameter]/route.ts | 32 ++++++++----------- .../[...parameters]}/route.ts | 0 .../[parameter]}/route.ts | 0 .../error => error/[...parameters]}/route.ts | 0 .../error => error/[parameter]}/route.ts | 0 .../nextjs-app-dir/app/dynamic-route/route.ts | 29 ----------------- 7 files changed, 28 insertions(+), 65 deletions(-) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/{[...parameters]/captureException => captureException/[...parameters]}/route.ts (100%) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/{[parameter]/captureException => captureException/[parameter]}/route.ts (100%) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/{[...parameters]/error => error/[...parameters]}/route.ts (100%) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/{[parameter]/error => error/[parameter]}/route.ts (100%) delete mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts index e35c98878021..66a244d6f7d0 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/route.ts @@ -1,33 +1,29 @@ import { NextResponse } from 'next/server'; -interface Params { - params: Record; +export async function GET() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'GET' }); } -export async function GET(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +export async function POST() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'POST' }); } -export async function POST(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +export async function PUT() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PUT' }); } -export async function PUT(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +export async function PATCH() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PATCH' }); } -export async function PATCH(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +export async function DELETE() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'DELETE' }); } -export async function DELETE(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +export async function HEAD() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'HEAD' }); } -export async function HEAD(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); -} - -export async function OPTIONS(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +export async function OPTIONS() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'OPTIONS' }); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts index e35c98878021..66a244d6f7d0 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/route.ts @@ -1,33 +1,29 @@ import { NextResponse } from 'next/server'; -interface Params { - params: Record; +export async function GET() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'GET' }); } -export async function GET(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); +export async function POST() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'POST' }); } -export async function POST(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); +export async function PUT() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PUT' }); } -export async function PUT(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); +export async function PATCH() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'PATCH' }); } -export async function PATCH(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); +export async function DELETE() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'DELETE' }); } -export async function DELETE(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); +export async function HEAD() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'HEAD' }); } -export async function HEAD(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); -} - -export async function OPTIONS(_request: Request, { params }: Params) { - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); +export async function OPTIONS() { + return NextResponse.json({ data: 'I am a dynamic route!', method: 'OPTIONS' }); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts similarity index 100% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/captureException/route.ts rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts similarity index 100% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/captureException/route.ts rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/error/[...parameters]/route.ts similarity index 100% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[...parameters]/error/route.ts rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/error/[...parameters]/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/error/[parameter]/route.ts similarity index 100% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/[parameter]/error/route.ts rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/error/[parameter]/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts deleted file mode 100644 index 66a244d6f7d0..000000000000 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/route.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { NextResponse } from 'next/server'; - -export async function GET() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'GET' }); -} - -export async function POST() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'POST' }); -} - -export async function PUT() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'PUT' }); -} - -export async function PATCH() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'PATCH' }); -} - -export async function DELETE() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'DELETE' }); -} - -export async function HEAD() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'HEAD' }); -} - -export async function OPTIONS() { - return NextResponse.json({ data: 'I am a dynamic route!', method: 'OPTIONS' }); -} From 9d3fb68dc87560aa1c399026b64bd78da74c9561 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 11:51:16 +0000 Subject: [PATCH 10/24] . --- .../nextjs-app-dir/assert-build.ts | 13 ++++- .../components/client-error-debug-tools.tsx | 49 +++++++++++++++++++ .../nextjs-app-dir/tests/trace.test.ts | 38 ++++++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts index cb7adf38cde8..c9bdb61455b1 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts @@ -3,7 +3,7 @@ import * as assert from 'assert/strict'; const stdin = fs.readFileSync(0).toString(); -// Assert that all static components stay static and ally dynamic components stay dynamic +// Assert that all static components stay static and all dynamic components stay dynamic assert.match(stdin, /○ \/client-component/); assert.match(stdin, /● \/client-component\/parameter\/\[\.\.\.parameters\]/); @@ -13,4 +13,13 @@ assert.match(stdin, /λ \/server-component/); assert.match(stdin, /λ \/server-component\/parameter\/\[\.\.\.parameters\]/); assert.match(stdin, /λ \/server-component\/parameter\/\[parameter\]/); -export {}; +// Assert that all static route hndlers stay static and all dynamic route handlers stay dynamic + +assert.match(stdin, /λ \/dynamic-route\/\[\.\.\.parameters\]/); +assert.match(stdin, /λ \/dynamic-route\/\[parameter\]/); +assert.match(stdin, /λ \/dynamic-route\/captureException\/\[\.\.\.parameters\]/); +assert.match(stdin, /λ \/dynamic-route\/captureException\/\[parameter\]/); +assert.match(stdin, /λ \/dynamic-route\/error\/\[\.\.\.parameters\]/); +assert.match(stdin, /λ \/dynamic-route\/error\/\[parameter\]/); + +// assert.match(stdin, /● \/static-route/); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx index 9eeaa227996f..5bc491f63f8a 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx @@ -7,10 +7,14 @@ import { captureException } from '@sentry/nextjs'; export function ClientErrorDebugTools() { const transactionContextValue = useContext(TransactionContext); const [transactionName, setTransactionName] = useState(''); + const [getRequestTarget, setGetRequestTarget] = useState(''); + const [postRequestTarget, setPostRequestTarget] = useState(''); const [isFetchingAPIRoute, setIsFetchingAPIRoute] = useState(); const [isFetchingEdgeAPIRoute, setIsFetchingEdgeAPIRoute] = useState(); const [isFetchingExternalAPIRoute, setIsFetchingExternalAPIRoute] = useState(); + const [isSendeingGetRequest, setIsSendingGetRequest] = useState(); + const [isSendeingPostRequest, setIsSendingPostRequest] = useState(); const [renderError, setRenderError] = useState(); if (renderError) { @@ -119,6 +123,51 @@ export function ClientErrorDebugTools() { Send request to external API route
+ { + setGetRequestTarget(e.target.value); + }} + /> + +
+ { + setPostRequestTarget(e.target.value); + }} + /> + ); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts index 99d03266d01f..487121ac9f7c 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts @@ -28,3 +28,41 @@ if (process.env.TEST_ENV === 'production') { await serverComponentTransaction; }); } + +test('Sends connected traces for route handlers', async ({ page }, testInfo) => { + await page.goto('/client-component'); + + const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; + + const getRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'GET /dynamic-route/42' && + (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id + ); + }); + + const postRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'POST /dynamic-route/42/1337' && + (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id + ); + }); + + const clientTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return transactionEvent?.transaction === clientTransactionName; + }); + + await page.getByPlaceholder('Transaction name').fill(clientTransactionName); + await page.getByText('Start transaction').click(); + + await page.getByPlaceholder('GET request target').fill('/dynamic-route/42'); + await page.getByText('Send GET request').click(); + + await page.getByPlaceholder('POST request target').fill('/dynamic-route/42/1337'); + await page.getByText('Send POST request').click(); + + await page.getByText('Stop transaction').click(); + + await getRequestTransaction; + await postRequestTransaction; +}); From a84029aae7b59184397b55d7b4d8fc5e89f35ef4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 12:48:50 +0000 Subject: [PATCH 11/24] comments --- .../wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts | 2 +- ...RequestHandlerLikeFunctionWithPerformanceInstrumentation.ts | 2 +- .../nextjs/src/config/templates/routeHandlerWrapperTemplate.ts | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts index f58458c80cc4..f93141387498 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts @@ -24,7 +24,7 @@ const defaultBeforeCaptureError = async (): Promise any>( originalFunction: F, diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts index ceb0655046ea..66a5ba27a05d 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts @@ -44,7 +44,7 @@ const defaultOnFunctionEnd = async (): Promise => { }; /** - * TODO + * Generic function that wraps any other function with Sentry performance instrumentation. */ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< A extends any[], diff --git a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts index 360658314732..75f0a5d9f399 100644 --- a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts @@ -30,7 +30,8 @@ type ServerComponentModule = { const serverComponentModule = wrapee as ServerComponentModule; function wrapHandler(handler: T, method: string): T { - // TODO: explain why + // 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; } From 0b1664aef82742c11aaf32c73c5adc47bf0e2983 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 13:43:49 +0000 Subject: [PATCH 12/24] . --- .../e2e-tests/test-applications/nextjs-app-dir/.gitignore | 1 + .../test-applications/nextjs-app-dir/tests/exceptions.test.ts | 2 +- .../test-applications/nextjs-app-dir/tests/trace.test.ts | 4 ++-- .../nextjs-app-dir/tests/transactions.test.ts | 2 +- .../nextjs-app-dir/tests/utils.ts} | 0 5 files changed, 5 insertions(+), 4 deletions(-) rename packages/e2e-tests/{test-utils/playwright-poll-sentry-event.ts => test-applications/nextjs-app-dir/tests/utils.ts} (100%) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/.gitignore b/packages/e2e-tests/test-applications/nextjs-app-dir/.gitignore index 35b1048ce099..73b58a0a62d0 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/.gitignore +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/.gitignore @@ -41,3 +41,4 @@ next-env.d.ts .sentryclirc .vscode +test-results diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index bd83eec16098..d215d4a79c11 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -1,6 +1,6 @@ import { expect, test } from '@playwright/test'; import { waitForError } from '../../../test-utils/event-proxy-server'; -import { pollEventOnSentry } from '../../../test-utils/playwright-poll-sentry-event'; +import { pollEventOnSentry } from './utils'; test('Sends an ingestable client-side exception to Sentry', async ({ page }) => { await page.goto('/'); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts index 487121ac9f7c..96bdbf242664 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts @@ -36,14 +36,14 @@ test('Sends connected traces for route handlers', async ({ page }, testInfo) => const getRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { return ( - transactionEvent?.transaction === 'GET /dynamic-route/42' && + transactionEvent?.transaction === 'GET /dynamic-route/[parameter]' && (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id ); }); const postRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { return ( - transactionEvent?.transaction === 'POST /dynamic-route/42/1337' && + transactionEvent?.transaction === 'POST /dynamic-route/[...parameters]' && (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id ); }); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index 0745cf4c4a2e..709d8ac5c863 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; import { waitForTransaction } from '../../../test-utils/event-proxy-server'; -import { pollEventOnSentry } from '../../../test-utils/playwright-poll-sentry-event'; +import { pollEventOnSentry } from './utils'; test('Sends an ingestable pageload transaction to Sentry', async ({ page }) => { const pageloadTransactionEventPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { diff --git a/packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/utils.ts similarity index 100% rename from packages/e2e-tests/test-utils/playwright-poll-sentry-event.ts rename to packages/e2e-tests/test-applications/nextjs-app-dir/tests/utils.ts From cd6cd621408b2488c752044be9e2dc511974ff1f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 13:51:48 +0000 Subject: [PATCH 13/24] . --- .../test-applications/nextjs-app-dir/tests/trace.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts index 96bdbf242664..bd8cda9eda87 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts @@ -4,7 +4,7 @@ import { waitForTransaction } from '../../../test-utils/event-proxy-server'; if (process.env.TEST_ENV === 'production') { // TODO: Fix that this is flakey on dev server - might be an SDK bug test('Sends connected traces for server components', async ({ page }, testInfo) => { - await page.goto('/client-component'); + await page.goto('/'); const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; @@ -30,7 +30,7 @@ if (process.env.TEST_ENV === 'production') { } test('Sends connected traces for route handlers', async ({ page }, testInfo) => { - await page.goto('/client-component'); + await page.goto('/'); const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; From 7206514339516d08054a394c6aa97716f5facec6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Mar 2023 13:55:48 +0000 Subject: [PATCH 14/24] . --- .../nextjs-app-dir/tests/trace.test.ts | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts index bd8cda9eda87..6e8537cac83e 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts @@ -1,8 +1,8 @@ import { test } from '@playwright/test'; import { waitForTransaction } from '../../../test-utils/event-proxy-server'; +// TODO: Fix that this is flakey on dev server - might be an SDK bug if (process.env.TEST_ENV === 'production') { - // TODO: Fix that this is flakey on dev server - might be an SDK bug test('Sends connected traces for server components', async ({ page }, testInfo) => { await page.goto('/'); @@ -27,42 +27,42 @@ if (process.env.TEST_ENV === 'production') { await serverComponentTransaction; }); -} -test('Sends connected traces for route handlers', async ({ page }, testInfo) => { - await page.goto('/'); + test('Sends connected traces for route handlers', async ({ page }, testInfo) => { + await page.goto('/'); - const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; + const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; - const getRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return ( - transactionEvent?.transaction === 'GET /dynamic-route/[parameter]' && - (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id - ); - }); + const getRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'GET /dynamic-route/[parameter]' && + (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id + ); + }); - const postRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return ( - transactionEvent?.transaction === 'POST /dynamic-route/[...parameters]' && - (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id - ); - }); + const postRequestTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'POST /dynamic-route/[...parameters]' && + (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id + ); + }); - const clientTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { - return transactionEvent?.transaction === clientTransactionName; - }); + const clientTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return transactionEvent?.transaction === clientTransactionName; + }); - await page.getByPlaceholder('Transaction name').fill(clientTransactionName); - await page.getByText('Start transaction').click(); + await page.getByPlaceholder('Transaction name').fill(clientTransactionName); + await page.getByText('Start transaction').click(); - await page.getByPlaceholder('GET request target').fill('/dynamic-route/42'); - await page.getByText('Send GET request').click(); + await page.getByPlaceholder('GET request target').fill('/dynamic-route/42'); + await page.getByText('Send GET request').click(); - await page.getByPlaceholder('POST request target').fill('/dynamic-route/42/1337'); - await page.getByText('Send POST request').click(); + await page.getByPlaceholder('POST request target').fill('/dynamic-route/42/1337'); + await page.getByText('Send POST request').click(); - await page.getByText('Stop transaction').click(); + await page.getByText('Stop transaction').click(); - await getRequestTransaction; - await postRequestTransaction; -}); + await getRequestTransaction; + await postRequestTransaction; + }); +} From 8bae6c608c0415876dbcd8ee4b7a98117ece32d5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 08:41:11 +0000 Subject: [PATCH 15/24] . --- .../nextjs-app-dir/app/edge-route/route.ts | 7 ++ .../templates/routeHandlerWrapperTemplate.ts | 16 +++- packages/nextjs/src/edge/index.ts | 2 + .../src/edge/wrapRouteHandlerWithSentry.ts | 79 +++++++++++++++++++ .../src/server/wrapRouteHandlerWithSentry.ts | 52 +----------- 5 files changed, 104 insertions(+), 52 deletions(-) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/route.ts create mode 100644 packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/route.ts new file mode 100644 index 000000000000..15bfa9d18d4d --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/route.ts @@ -0,0 +1,7 @@ +import { NextResponse } from 'next/server'; + +export async function GET() { + return NextResponse.json({ data: 'I am an edge route!', method: 'GET' }); +} + +export const runtime = 'experimental-edge'; diff --git a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts index 75f0a5d9f399..031885198108 100644 --- a/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/routeHandlerWrapperTemplate.ts @@ -42,9 +42,19 @@ function wrapHandler(handler: T, method: string): T { return new Proxy(handler, { apply: (originalFunction, thisArg, args) => { - const headersList = headers(); - const sentryTraceHeader = headersList.get('sentry-trace'); - const baggageHeader = headersList.get('baggage'); + let sentryTraceHeader: string | undefined; + let baggageHeader: string | undefined; + + try { + // For some odd Next.js magic reason, `headers()` will not work if used inside node_modules. + // Current assumption is that Next.js applies some loader magic to userfiles, but not files in node_modules. + // This file is technically a userfile so it gets the loader magic applied. + const headersList = headers(); + sentryTraceHeader = headersList.get('sentry-trace'); + baggageHeader = headersList.get('baggage'); + } catch (e) { + // This crashes on the edge runtime - at least at the time when this was written, which was during app dir alpha + } return Sentry.wrapRouteHandlerWithSentry(originalFunction, { method, diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 6f8cd2f42cc4..7b5059aad5db 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -141,3 +141,5 @@ export { export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; export { wrapServerComponentWithSentry } from './wrapServerComponentWithSentry'; + +export { wrapRouteHandlerWithSentry } from './wrapRouteHandlerWithSentry'; diff --git a/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts new file mode 100644 index 000000000000..675b659b0143 --- /dev/null +++ b/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts @@ -0,0 +1,79 @@ +import { getCurrentHub } from '@sentry/core'; + +import type { RouteHandlerContext } from '../common/types'; +import { wrapRequestHandlerLikeFunctionWithErrorInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation'; +import { wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation'; + +type RouteHandlerArgs = [Request | undefined, { params?: Record } | undefined]; + +/** + * Wraps an `app` directory server component with Sentry error instrumentation. + */ +export function wrapRouteHandlerWithSentry unknown>( + routeHandler: F, + context: RouteHandlerContext, +): F { + return new Proxy(routeHandler, { + apply: (originalFunction, thisArg, args: Parameters) => { + const errorWrappedFunction = wrapRequestHandlerLikeFunctionWithErrorInstrumentation(originalFunction, () => ({ + wrappingTargetName: context.method, + })); + + const req = args[0]; + const routeConfiguration = args[1]; + + const sendDefaultPiiOption = getCurrentHub().getClient()?.getOptions().sendDefaultPii; + let routeParameters: Record = {}; + + if (sendDefaultPiiOption && routeConfiguration?.params) { + routeParameters = routeConfiguration?.params; + } + + let requestBaggageHeader: string | null; + let requestSentryTraceHeader: string | null; + + try { + if (req instanceof Request) { + requestBaggageHeader = req.headers.get('baggage'); + requestSentryTraceHeader = req.headers.get('sentry-trace'); + } + } catch (e) { + // This crashes on the edge runtime - at least at the time when this was written, which was during app dir alpha + } + + const errorAndPerformanceWrappedFunction = wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation( + errorWrappedFunction, + { + wrapperContextExtractor: () => ({ + requestContextObject: req, + // Use context if available, fall back to request.headers + baggageHeader: context.baggageHeader || requestBaggageHeader, + sentryTraceHeader: context.sentryTraceHeader || requestSentryTraceHeader, + }), + spanInfoCreator: ({ willCreateTransaction }) => { + if (willCreateTransaction) { + return { + name: `${context.method} ${context.parameterizedRoute}`, + op: 'http.server', + data: { + routeParameters, + }, + }; + } else { + return { + name: `${context.method}()`, + op: 'function', + data: { + route: context.parameterizedRoute, + }, + }; + } + }, + }, + ); + + const { returnValue } = errorAndPerformanceWrappedFunction.apply(thisArg, args); + return returnValue; + }, + }); +} diff --git a/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts index ed583d16032f..8fc925372ec9 100644 --- a/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/server/wrapRouteHandlerWithSentry.ts @@ -1,15 +1,14 @@ import * as domain from 'domain'; import type { RouteHandlerContext } from '../common/types'; -import { wrapRequestHandlerLikeFunctionWithErrorInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation'; -import { wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation'; -import { getCurrentHub } from '../edge'; +import { wrapRouteHandlerWithSentry as edgeWrapRouteHandlerWithSentry } from '../edge/wrapRouteHandlerWithSentry'; type RouteHandlerArgs = [Request | undefined, { params?: Record } | undefined]; /** * Wraps an `app` directory server component with Sentry error instrumentation. */ +// This glorious function is essentially just a wrapper around the edge version with domain isolation. export function wrapRouteHandlerWithSentry unknown>( routeHandler: F, context: RouteHandlerContext, @@ -17,52 +16,7 @@ export function wrapRouteHandlerWithSentry) => { return domain.create().bind(() => { - const errorWrappedFunction = wrapRequestHandlerLikeFunctionWithErrorInstrumentation(originalFunction, () => ({ - wrappingTargetName: context.method, - })); - - const req = args[0]; - const routeConfiguration = args[1]; - - const sendDefaultPiiOption = getCurrentHub().getClient()?.getOptions().sendDefaultPii; - let routeParameters: Record = {}; - - if (sendDefaultPiiOption && routeConfiguration?.params) { - routeParameters = routeConfiguration?.params; - } - - const errorAndPerformanceWrappedFunction = wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation( - errorWrappedFunction, - { - wrapperContextExtractor: () => ({ - requestContextObject: req, - baggageHeader: req?.headers.get('baggage'), - sentryTraceHeader: req?.headers.get('sentry-trace'), - }), - spanInfoCreator: ({ willCreateTransaction }) => { - if (willCreateTransaction) { - return { - name: `${context.method} ${context.parameterizedRoute}`, - op: 'http.server', - data: { - routeParameters, - }, - }; - } else { - return { - name: `${context.method}()`, - op: 'function', - data: { - route: context.parameterizedRoute, - }, - }; - } - }, - }, - ); - - const { returnValue } = errorAndPerformanceWrappedFunction.apply(thisArg, args); - return returnValue; + return edgeWrapRouteHandlerWithSentry(originalFunction, context).apply(thisArg, args); })(); }, }); From 0b5cd46604f3190a5d357ff496f43f9446a3b97c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 09:50:56 +0000 Subject: [PATCH 16/24] . --- .../captureException/[...parameters]/route.ts | 41 ------------------- .../captureException/[parameter]/route.ts | 41 ------------------- .../app/edge-route/error/route.ts | 5 +++ .../nextjs-app-dir/assert-build.ts | 3 -- .../tests/devErrorSymbolification.test.ts | 2 +- .../nextjs-app-dir/tests/exceptions.test.ts | 37 ++++++++++++----- 6 files changed, 32 insertions(+), 97 deletions(-) delete mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts delete mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/error/route.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts deleted file mode 100644 index 30d5d10a8c8e..000000000000 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[...parameters]/route.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { NextResponse } from 'next/server'; -import * as Sentry from '@sentry/nextjs'; - -interface Params { - params: Record; -} - -export async function GET(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); -} - -export async function POST(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); -} - -export async function PUT(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); -} - -export async function PATCH(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); -} - -export async function DELETE(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); -} - -export async function HEAD(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); -} - -export async function OPTIONS(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); -} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts deleted file mode 100644 index 30d5d10a8c8e..000000000000 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/dynamic-route/captureException/[parameter]/route.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { NextResponse } from 'next/server'; -import * as Sentry from '@sentry/nextjs'; - -interface Params { - params: Record; -} - -export async function GET(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'GET' }); -} - -export async function POST(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'POST' }); -} - -export async function PUT(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PUT' }); -} - -export async function PATCH(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'PATCH' }); -} - -export async function DELETE(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'DELETE' }); -} - -export async function HEAD(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'HEAD' }); -} - -export async function OPTIONS(_request: Request, { params }: Params) { - Sentry.captureException(new Error('I am a capturedException inside a dynamic route!')); - return NextResponse.json({ data: 'I am a dynamic route!', params, method: 'OPTIONS' }); -} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/error/route.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/error/route.ts new file mode 100644 index 000000000000..6638fef63e26 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-route/error/route.ts @@ -0,0 +1,5 @@ +export async function GET() { + throw new Error('I am an error inside an edge route!'); +} + +export const runtime = 'experimental-edge'; diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts index c9bdb61455b1..d8c28381405c 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts @@ -17,9 +17,6 @@ assert.match(stdin, /λ \/server-component\/parameter\/\[parameter\]/); assert.match(stdin, /λ \/dynamic-route\/\[\.\.\.parameters\]/); assert.match(stdin, /λ \/dynamic-route\/\[parameter\]/); -assert.match(stdin, /λ \/dynamic-route\/captureException\/\[\.\.\.parameters\]/); -assert.match(stdin, /λ \/dynamic-route\/captureException\/\[parameter\]/); assert.match(stdin, /λ \/dynamic-route\/error\/\[\.\.\.parameters\]/); assert.match(stdin, /λ \/dynamic-route\/error\/\[parameter\]/); - // assert.match(stdin, /● \/static-route/); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/devErrorSymbolification.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/devErrorSymbolification.test.ts index 2f7173cee315..a6933bae2cd5 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/devErrorSymbolification.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/devErrorSymbolification.test.ts @@ -24,7 +24,7 @@ test.describe('dev mode error symbolification', () => { function: 'onClick', filename: 'components/client-error-debug-tools.tsx', abs_path: 'webpack-internal:///(app-client)/./components/client-error-debug-tools.tsx', - lineno: 54, + lineno: 58, colno: 16, in_app: true, pre_context: [' {'], diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index d215d4a79c11..42f3ca45f956 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -18,18 +18,33 @@ test('Sends an ingestable client-side exception to Sentry', async ({ page }) => await pollEventOnSentry(exceptionEventId!); }); -// test('Sends an ingestable route handler exception to Sentry', async ({ page }) => { -// await page.goto('/'); +// TODO: Fix that these tests are flakey on dev server - might be an SDK bug - might be Next.js itself +if (process.env.TEST_ENV !== 'development') { + test('Sends an ingestable route handler exception to Sentry', async ({ page }) => { + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside a dynamic route!'; + }); -// const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { -// return errorEvent?.exception?.values?.[0]?.value === 'Click Error'; -// }); + await page.request.get('/dynamic-route/error/42'); -// await page.getByText('Throw error').click(); + const errorEvent = await errorEventPromise; + const exceptionEventId = errorEvent.event_id; -// const errorEvent = await errorEventPromise; -// const exceptionEventId = errorEvent.event_id; + expect(exceptionEventId).toBeDefined(); + await pollEventOnSentry(exceptionEventId!); + }); + + test('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside an edge route!'; + }); + + await page.request.get('/edge-route/error'); -// expect(exceptionEventId).toBeDefined(); -// await pollEventOnSentry(exceptionEventId!); -// }); + const errorEvent = await errorEventPromise; + const exceptionEventId = errorEvent.event_id; + + expect(exceptionEventId).toBeDefined(); + await pollEventOnSentry(exceptionEventId!); + }); +} From 7b19b5fc87da02d9bcc61703babe0b2bf6bd8674 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 11:07:52 +0000 Subject: [PATCH 17/24] gzip --- packages/e2e-tests/test-utils/event-proxy-server.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-utils/event-proxy-server.ts b/packages/e2e-tests/test-utils/event-proxy-server.ts index c61e20d4081d..3b23044963b6 100644 --- a/packages/e2e-tests/test-utils/event-proxy-server.ts +++ b/packages/e2e-tests/test-utils/event-proxy-server.ts @@ -7,6 +7,7 @@ import type { AddressInfo } from 'net'; import * as os from 'os'; import * as path from 'path'; import * as util from 'util'; +import * as zlib from 'zlib'; const readFile = util.promisify(fs.readFile); const writeFile = util.promisify(fs.writeFile); @@ -44,7 +45,12 @@ export async function startEventProxyServer(options: EventProxyServerOptions): P }); proxyRequest.addListener('end', () => { - const proxyRequestBody = Buffer.concat(proxyRequestChunks).toString(); + const proxyRequestBody = ( + proxyRequest.headers['content-encoding'] === 'gzip' + ? zlib.unzipSync(Buffer.concat(proxyRequestChunks), { flush: zlib.constants.Z_FULL_FLUSH }) + : Buffer.concat(proxyRequestChunks) + ).toString(); + const envelopeHeader: { dsn?: string } = JSON.parse(proxyRequestBody.split('\n')[0]); if (!envelopeHeader.dsn) { From 265ba4d92d1dd4ecac3d1487ab1508c1fb39cdf0 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 13:44:11 +0000 Subject: [PATCH 18/24] unflake --- .../test-applications/nextjs-app-dir/playwright.config.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts index ae466dab4350..359b2c9f590f 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts @@ -26,7 +26,8 @@ const config: PlaywrightTestConfig = { /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, /* `next dev` is incredibly buggy with the app dir */ - retries: testEnv === 'development' ? 3 : 0, + /* `next build && next start` is also flakey. Current assumption: Next.js has a bug - but not sure. */ + retries: 3, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ From c9a5a503f918223fc00657a8ec9d8a253cfc094d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 14:18:08 +0000 Subject: [PATCH 19/24] . --- .../nextjs-app-dir/tests/exceptions.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index 42f3ca45f956..c5ade15a139a 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -35,15 +35,21 @@ if (process.env.TEST_ENV !== 'development') { }); test('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { + console.log('AAA'); const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside an edge route!'; }); + await new Promise(resolve => setTimeout(resolve, 500)); + + console.log('BBB'); await page.request.get('/edge-route/error'); + console.log('CCC'); const errorEvent = await errorEventPromise; const exceptionEventId = errorEvent.event_id; + console.log('DDD'); expect(exceptionEventId).toBeDefined(); await pollEventOnSentry(exceptionEventId!); }); From c4a3b4a02801aab89de9417c5cd05337baacf19c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 14:18:48 +0000 Subject: [PATCH 20/24] . --- .../test-applications/nextjs-app-dir/tests/exceptions.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index c5ade15a139a..ba7c55421690 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -25,6 +25,8 @@ if (process.env.TEST_ENV !== 'development') { return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside a dynamic route!'; }); + await new Promise(resolve => setTimeout(resolve, 500)); + await page.request.get('/dynamic-route/error/42'); const errorEvent = await errorEventPromise; From a7ffdfb6fb0556bf305d4aec45ad6c387ca63470 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 14:38:34 +0000 Subject: [PATCH 21/24] . --- .../nextjs-app-dir/tests/exceptions.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index ba7c55421690..e2463e17bdeb 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -37,21 +37,18 @@ if (process.env.TEST_ENV !== 'development') { }); test('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { - console.log('AAA'); const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + console.log(errorEvent?.exception?.values?.[0]?.value); return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside an edge route!'; }); await new Promise(resolve => setTimeout(resolve, 500)); - console.log('BBB'); - await page.request.get('/edge-route/error'); + await page.request.post('/edge-route/error'); - console.log('CCC'); const errorEvent = await errorEventPromise; const exceptionEventId = errorEvent.event_id; - console.log('DDD'); expect(exceptionEventId).toBeDefined(); await pollEventOnSentry(exceptionEventId!); }); From f9427369fa5c69f2fd2726f7f6522d78175beb1a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 10 Mar 2023 16:30:28 +0000 Subject: [PATCH 22/24] god --- packages/core/src/baseclient.ts | 1 + .../nextjs-app-dir/playwright.config.ts | 2 +- .../nextjs-app-dir/tests/exceptions.test.ts | 9 ++------- ...estHandlerLikeFunctionWithErrorInstrumentation.ts | 2 +- ...dlerLikeFunctionWithPerformanceInstrumentation.ts | 6 ++++-- .../nextjs/src/edge/wrapRouteHandlerWithSentry.ts | 12 ++++++++++-- 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index bf1ae616c4d6..fb8d4aa3d8e6 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -245,6 +245,7 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public flush(timeout?: number): PromiseLike { + __DEBUG_BUILD__ && logger.warn('Flushing events.'); const transport = this._transport; if (transport) { return this._isClientDoneProcessing(timeout).then(clientFinished => { diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts index 359b2c9f590f..a06a72a311cf 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts @@ -13,7 +13,7 @@ if (!testEnv) { const config: PlaywrightTestConfig = { testDir: './tests', /* Maximum time one test can run for. */ - timeout: 60 * 1000, + timeout: 30 * 1000, expect: { /** * Maximum time expect() should wait for the condition to be met. diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index e2463e17bdeb..c3f5d985cc24 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -25,8 +25,6 @@ if (process.env.TEST_ENV !== 'development') { return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside a dynamic route!'; }); - await new Promise(resolve => setTimeout(resolve, 500)); - await page.request.get('/dynamic-route/error/42'); const errorEvent = await errorEventPromise; @@ -36,15 +34,12 @@ if (process.env.TEST_ENV !== 'development') { await pollEventOnSentry(exceptionEventId!); }); - test('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { + test.only('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { - console.log(errorEvent?.exception?.values?.[0]?.value); return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside an edge route!'; }); - await new Promise(resolve => setTimeout(resolve, 500)); - - await page.request.post('/edge-route/error'); + await page.request.get('/edge-route/error'); const errorEvent = await errorEventPromise; const exceptionEventId = errorEvent.event_id; diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts index f93141387498..6a03d0ca9c56 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation.ts @@ -33,7 +33,7 @@ export function wrapRequestHandlerLikeFunctionWithErrorInstrumentation) => ReturnType { return new Proxy(originalFunction, { apply: (originalFunction, thisArg: unknown, args: Parameters): ReturnType => { - const errorInfo: ErrorInfo = errorInfoCreator(args); + const errorInfo = errorInfoCreator(args); const scope = getCurrentHub().getScope(); if (scope) { diff --git a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts index 66a5ba27a05d..59a18c0ab77b 100644 --- a/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts +++ b/packages/nextjs/src/common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation.ts @@ -54,7 +54,8 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< options: { wrapperContextExtractor?: WrapperContextExtractor; spanInfoCreator: SpanInfoCreator; - onFunctionEnd?: OnFunctionEndHook>; + afterFunctionEnd?: OnFunctionEndHook>; + afterSpanFinish?: () => void; }, ): (...args: Parameters) => WrappedReturnValue> { return new Proxy(originalFunction, { @@ -155,9 +156,10 @@ export function wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation< }; const handleFunctionEnd = (res: ReturnType | undefined, err: unknown | undefined): void => { - void (options.onFunctionEnd || defaultOnFunctionEnd)(span, res, err).then(beforeFinishResult => { + void (options.afterFunctionEnd || defaultOnFunctionEnd)(span, res, err).then(beforeFinishResult => { if (beforeFinishResult.shouldFinishSpan) { span.finish(); + options.afterSpanFinish?.(); } }); }; diff --git a/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts index 675b659b0143..ad3fa6c39e94 100644 --- a/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapRouteHandlerWithSentry.ts @@ -3,6 +3,7 @@ import { getCurrentHub } from '@sentry/core'; import type { RouteHandlerContext } from '../common/types'; import { wrapRequestHandlerLikeFunctionWithErrorInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithErrorInstrumentation'; import { wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation } from '../common/wrapRequestHandlerLikeFunctionWithPerformanceInstrumentation'; +import { flush } from './utils/flush'; type RouteHandlerArgs = [Request | undefined, { params?: Record } | undefined]; @@ -50,6 +51,9 @@ export function wrapRouteHandlerWithSentry { + void flush(2000); + }, spanInfoCreator: ({ willCreateTransaction }) => { if (willCreateTransaction) { return { @@ -72,8 +76,12 @@ export function wrapRouteHandlerWithSentry Date: Fri, 10 Mar 2023 17:10:33 +0000 Subject: [PATCH 23/24] . --- packages/utils/test/envelope.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 2996b6dcde06..9649da0e2108 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -36,7 +36,7 @@ describe('envelope', () => { expect(headers).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); }); - it.only('serializes an envelope with attachments', () => { + it('serializes an envelope with attachments', () => { const items: EventEnvelope[1] = [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], [{ type: 'attachment', filename: 'bar.txt', length: 6 }, Uint8Array.from([1, 2, 3, 4, 5, 6])], From 42208a8bc32524a89622fe250a1fce11bee55d90 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 12 Mar 2023 11:56:20 +0000 Subject: [PATCH 24/24] no only --- .../test-applications/nextjs-app-dir/tests/exceptions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts index c3f5d985cc24..42f3ca45f956 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/exceptions.test.ts @@ -34,7 +34,7 @@ if (process.env.TEST_ENV !== 'development') { await pollEventOnSentry(exceptionEventId!); }); - test.only('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { + test('Sends an ingestable edge route handler exception to Sentry', async ({ page }) => { const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { return errorEvent?.exception?.values?.[0]?.value === 'I am an error inside an edge route!'; });