From 63991434eec2dd5b3b37dfe1a33d639d2a827f51 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 4 Jan 2024 15:14:47 +0000 Subject: [PATCH 1/3] fix(nextjs): Don't capture not-found and redirect errors in generation functions --- .../with-notfound/page.tsx | 11 ++++++ .../with-redirect/page.tsx | 11 ++++++ .../test-applications/nextjs-14/app/page.tsx | 3 ++ .../tests/generation-functions.test.ts | 34 +++++++++++++++++++ .../wrapGenerationFunctionWithSentry.ts | 29 +++++++++++----- 5 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-notfound/page.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-redirect/page.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-14/app/page.tsx diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-notfound/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-notfound/page.tsx new file mode 100644 index 000000000000..46d4ddd7f962 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-notfound/page.tsx @@ -0,0 +1,11 @@ +import { notFound } from 'next/navigation'; + +export const dynamic = 'force-dynamic'; + +export default function PageWithRedirect() { + return

Hello World!

; +} + +export async function generateMetadata() { + notFound(); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-redirect/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-redirect/page.tsx new file mode 100644 index 000000000000..f1f37d7a32c6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-redirect/page.tsx @@ -0,0 +1,11 @@ +import { redirect } from 'next/navigation'; + +export const dynamic = 'force-dynamic'; + +export default function PageWithRedirect() { + return

Hello World!

; +} + +export async function generateMetadata() { + redirect('/'); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-14/app/page.tsx new file mode 100644 index 000000000000..6f4e63ef5748 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return

Home

; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts index 3828312607ea..b5fe7ee67393 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts @@ -77,3 +77,37 @@ test('Should send a transaction and an error event for a faulty generateViewport expect(await transactionPromise).toBeDefined(); expect(await errorEventPromise).toBeDefined(); }); + +test('Should send a transaction event with correct status for a generateMetadata() function invokation with redirect()', async ({ + page, +}) => { + const testTitle = 'redirect-foobar'; + + const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => { + return ( + transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-redirect)' && + transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle + ); + }); + + await page.goto(`/generation-functions/with-redirect?metadataTitle=${testTitle}`); + + expect((await transactionPromise).contexts?.trace?.status).toBe('ok'); +}); + +test('Should send a transaction event with correct status for a generateMetadata() function invokation with notfound()', async ({ + page, +}) => { + const testTitle = 'notfound-foobar'; + + const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => { + return ( + transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-notfound)' && + transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle + ); + }); + + await page.goto(`/generation-functions/with-notfound?metadataTitle=${testTitle}`); + + expect((await transactionPromise).contexts?.trace?.status).toBe('not_found'); +}); diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index d1765aa2c41e..4e44b94f898b 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -12,6 +12,7 @@ import type { WebFetchHeaders } from '@sentry/types'; import { winterCGHeadersToDict } from '@sentry/utils'; import type { GenerationFunctionContext } from '../common/types'; +import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; /** @@ -76,18 +77,28 @@ export function wrapGenerationFunctionWithSentry a }, }, }, - () => { + span => { return handleCallbackErrors( () => originalFunction.apply(thisArg, args), - err => - captureException(err, { - mechanism: { - handled: false, - data: { - function: 'wrapGenerationFunctionWithSentry', + err => { + if (isNotFoundNavigationError(err)) { + // We don't want to report "not-found"s + span?.setStatus('not_found'); + } else if (isRedirectNavigationError(err)) { + // We don't want to report redirects + span?.setStatus('ok'); + } else { + span?.setStatus('internal_error'); + captureException(err, { + mechanism: { + handled: false, + data: { + function: 'wrapGenerationFunctionWithSentry', + }, }, - }, - }), + }); + } + }, ); }, ); From 4b4b751237d5faf36080c7ef7f12813b879e31a9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 4 Jan 2024 15:47:09 +0000 Subject: [PATCH 2/3] Correct states --- .../nextjs/src/common/wrapGenerationFunctionWithSentry.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 4e44b94f898b..c0d0f8ccab57 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -6,7 +6,7 @@ import { getCurrentScope, handleCallbackErrors, runWithAsyncContext, - startSpan, + startSpanManual, } from '@sentry/core'; import type { WebFetchHeaders } from '@sentry/types'; import { winterCGHeadersToDict } from '@sentry/utils'; @@ -62,7 +62,7 @@ export function wrapGenerationFunctionWithSentry a transactionContext.parentSpanId = commonSpanId; } - return startSpan( + return startSpanManual( { op: 'function.nextjs', name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, @@ -98,6 +98,7 @@ export function wrapGenerationFunctionWithSentry a }, }); } + span?.end(); }, ); }, From b5a3da0372f9f3d9306150fe6315926954c1462f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 5 Jan 2024 08:49:35 +0000 Subject: [PATCH 3/3] ... --- packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index c0d0f8ccab57..fe90b6f6ca39 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -98,6 +98,8 @@ export function wrapGenerationFunctionWithSentry a }, }); } + }, + () => { span?.end(); }, );