From dd2965ec3a24b0647b6523c6b23c9cfae597836b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 29 Mar 2023 10:41:09 +0000 Subject: [PATCH 1/4] fix(nextjs): Don't report `NEXT_NOT_FOUND` and `NEXT_REDIRECT` --- .../src/common/nextNavigationErrorUtils.ts | 21 +++++++++++++++++++ .../server/wrapServerComponentWithSentry.ts | 15 ++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 packages/nextjs/src/common/nextNavigationErrorUtils.ts diff --git a/packages/nextjs/src/common/nextNavigationErrorUtils.ts b/packages/nextjs/src/common/nextNavigationErrorUtils.ts new file mode 100644 index 000000000000..d4a67791525f --- /dev/null +++ b/packages/nextjs/src/common/nextNavigationErrorUtils.ts @@ -0,0 +1,21 @@ +import { isError } from '@sentry/utils'; + +/** + * Determines whether input is a Next.js not-found error. + * https://beta.nextjs.org/docs/api-reference/notfound#notfound + */ +export function isNotFoundNavigationError(subject: unknown): boolean { + return isError(subject) && (subject as Error & { digest?: unknown }).digest === 'NEXT_NOT_FOUND'; +} + +/** + * Determines whether input is a Next.js redirect error. + * https://beta.nextjs.org/docs/api-reference/redirect#redirect + */ +export function isRedirectNavigationError(subject: unknown): boolean { + return ( + isError(subject) && + typeof (subject as Error & { digest?: unknown }).digest === 'string' && + (subject as Error & { digest: string }).digest.startsWith('NEXT_REDIRECT;') // a redirect digest looks like "NEXT_REDIRECT;[redirect path]" + ); +} diff --git a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts index 12ff9ceb5f72..9d958332d3e3 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -2,6 +2,7 @@ import { addTracingExtensions, captureException, getCurrentHub, startTransaction import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; import * as domain from 'domain'; +import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils'; import type { ServerComponentContext } from '../common/types'; /** @@ -60,9 +61,17 @@ export function wrapServerComponentWithSentry any> () => { transaction.finish(); }, - (e: Error) => { - transaction.setStatus('internal_error'); - captureException(e); + e => { + if (isNotFoundNavigationError(e)) { + // We don't want to report "not-found"s + transaction.setStatus('not_found'); + } else if (isRedirectNavigationError(e)) { + // We don't want to report redirects + } else { + transaction.setStatus('internal_error'); + captureException(e); + } + transaction.finish(); }, ); From 33125178a28ce54254cd02725e4629bfac8cce7b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 29 Mar 2023 10:53:07 +0000 Subject: [PATCH 2/4] . --- .../server/wrapServerComponentWithSentry.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts index 9d958332d3e3..2c25e8811409 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -46,12 +46,24 @@ export function wrapServerComponentWithSentry any> currentScope.setSpan(transaction); } + const handleErrorCase = (e: unknown): void => { + if (isNotFoundNavigationError(e)) { + // We don't want to report "not-found"s + transaction.setStatus('not_found'); + } else if (isRedirectNavigationError(e)) { + // We don't want to report redirects + } else { + transaction.setStatus('internal_error'); + captureException(e); + } + + transaction.finish(); + }; + try { maybePromiseResult = originalFunction.apply(thisArg, args); } catch (e) { - transaction.setStatus('internal_error'); - captureException(e); - transaction.finish(); + handleErrorCase(e); throw e; } @@ -62,17 +74,7 @@ export function wrapServerComponentWithSentry any> transaction.finish(); }, e => { - if (isNotFoundNavigationError(e)) { - // We don't want to report "not-found"s - transaction.setStatus('not_found'); - } else if (isRedirectNavigationError(e)) { - // We don't want to report redirects - } else { - transaction.setStatus('internal_error'); - captureException(e); - } - - transaction.finish(); + handleErrorCase(e); }, ); From 515fd29718d649f35af5992efa41727fa8d43453 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 29 Mar 2023 11:30:02 +0000 Subject: [PATCH 3/4] Add e2e tests --- .../nextjs-app-dir/app/layout.tsx | 6 +++++ .../nextjs-app-dir/app/not-found.tsx | 3 +++ .../nextjs-app-dir/app/not-found/page.tsx | 5 +++++ .../nextjs-app-dir/app/redirect/page.tsx | 5 +++++ .../nextjs-app-dir/tests/transactions.test.ts | 22 +++++++++++++++++++ 5 files changed, 41 insertions(+) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx index 35c4704735e7..b93ad29e53cb 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx @@ -29,6 +29,12 @@ export default function Layout({ children }: { children: React.ReactNode }) {
  • /server-component/parameter/foo/bar/baz
    • +
  • + /not-found +
    • +
  • + /redirect +
    • {children} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found.tsx index 5e7b156553a6..87ce52a19e73 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found.tsx @@ -1,7 +1,10 @@ +import { ClientErrorDebugTools } from '../components/client-error-debug-tools'; + export default function NotFound() { return (

    Not found (/)

    ; +
    ); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx new file mode 100644 index 000000000000..fd085635a841 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx @@ -0,0 +1,5 @@ +import { notFound } from 'next/navigation'; + +export default function Page() { + notFound(); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx new file mode 100644 index 000000000000..ed42d3864bf2 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx @@ -0,0 +1,5 @@ +import { redirect } from 'next/navigation'; + +export default function Page() { + redirect('/'); +} 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..a106e239a16a 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 @@ -89,4 +89,26 @@ if (process.env.TEST_ENV === 'production') { ) .toBe(200); }); + + test('Should not set an error status on a server component transaction when it redirects', async ({ page }) => { + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/redirect)'; + }); + + await page.goto('/redirect'); + + expect((await serverComponentTransactionPromise).contexts?.trace?.status).not.toBe('internal_error'); + }); + + test('Should set a "not_found" status on a server component transaction when notFound() is called', async ({ + page, + }) => { + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/not-found)'; + }); + + await page.goto('/not-found'); + + expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); + }); } From 94589f4749c81724c75d16395d601ec6702eeea4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 29 Mar 2023 12:34:44 +0000 Subject: [PATCH 4/4] . --- .../app/{ => server-component}/not-found/page.tsx | 2 ++ .../app/{ => server-component}/redirect/page.tsx | 2 ++ .../nextjs-app-dir/tests/transactions.test.ts | 8 ++++---- 3 files changed, 8 insertions(+), 4 deletions(-) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/{ => server-component}/not-found/page.tsx (69%) rename packages/e2e-tests/test-applications/nextjs-app-dir/app/{ => server-component}/redirect/page.tsx (70%) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/not-found/page.tsx similarity index 69% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/not-found/page.tsx index fd085635a841..c88c2d097d4f 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/not-found/page.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/not-found/page.tsx @@ -1,5 +1,7 @@ import { notFound } from 'next/navigation'; +export const dynamic = 'force-dynamic'; + export default function Page() { notFound(); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/redirect/page.tsx similarity index 70% rename from packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx rename to packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/redirect/page.tsx index ed42d3864bf2..3df1746a97ff 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/app/redirect/page.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/redirect/page.tsx @@ -1,5 +1,7 @@ import { redirect } from 'next/navigation'; +export const dynamic = 'force-dynamic'; + export default function Page() { redirect('/'); } 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 a106e239a16a..8dac4e58e5ba 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 @@ -92,10 +92,10 @@ if (process.env.TEST_ENV === 'production') { test('Should not set an error status on a server component transaction when it redirects', async ({ page }) => { const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return transactionEvent?.transaction === 'Page Server Component (/redirect)'; + return transactionEvent?.transaction === 'Page Server Component (/server-component/redirect)'; }); - await page.goto('/redirect'); + await page.goto('/server-component/redirect'); expect((await serverComponentTransactionPromise).contexts?.trace?.status).not.toBe('internal_error'); }); @@ -104,10 +104,10 @@ if (process.env.TEST_ENV === 'production') { page, }) => { const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return transactionEvent?.transaction === 'Page Server Component (/not-found)'; + return transactionEvent?.transaction === 'Page Server Component (/server-component/not-found)'; }); - await page.goto('/not-found'); + await page.goto('/server-component/not-found'); expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); });