From 56d101b7efca5b9424d10954286ce4925f6fde00 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 May 2024 13:07:25 +0000 Subject: [PATCH 1/5] fix(nextjs): Don't capture suspense errors in server components --- .../app/suspense-error/client-page.tsx | 13 ++++++++++ .../nextjs-15/app/suspense-error/loading.tsx | 3 +++ .../nextjs-15/app/suspense-error/page.tsx | 8 +++++++ .../nextjs-15/tests/suspense-error.test.ts | 24 +++++++++++++++++++ 4 files changed, 48 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx new file mode 100644 index 000000000000..4ed18fbb8b01 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/nextjs'; +import { use } from 'react'; + +export function ClientComponentTakingAPromise({ promise }: { promise: Promise }) { + let value; + try { + value = use(promise); + } catch (e) { + Sentry.captureException(e); // This error should not be reported + throw e; + } + return

Promise value: {value}

; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx new file mode 100644 index 000000000000..99672a08c52a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx @@ -0,0 +1,3 @@ +export default function Loading() { + return

Loading...

; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx new file mode 100644 index 000000000000..36ca1d1cd01a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx @@ -0,0 +1,8 @@ +import { ClientComponentTakingAPromise } from './client-page'; + +export const dynamic = 'force-dynamic'; + +export default async function Page() { + const promise = fetch('http://example.com/').then(() => 'foobar'); + return ; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts new file mode 100644 index 000000000000..50fc4fa4f86c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts @@ -0,0 +1,24 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server'; + +test('should not capture serverside suspense errors', async ({ page }) => { + const pageServerComponentTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/suspense-error)'; + }); + + let errorEventReceived = false; + waitForError('nextjs-15', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/suspense-error)'; + }).then(() => { + errorEventReceived = true; + }); + + await page.goto(`/suspense-error`); + + await page.waitForTimeout(5000); + + const pageServerComponentTransaction = await pageServerComponentTransactionPromise; + expect(pageServerComponentTransaction).toBeDefined(); + + expect(errorEventReceived).toBe(false); +}); From 77d4600c876fe6d062fba798b8a769538adfa057 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 May 2024 13:22:16 +0000 Subject: [PATCH 2/5] fail tests --- .../nextjs-15/app/suspense-error/client-page.tsx | 13 ------------- .../nextjs-15/app/suspense-error/loading.tsx | 3 --- .../nextjs-15/app/suspense-error/page.tsx | 15 +++++++++++---- 3 files changed, 11 insertions(+), 20 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx delete mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx deleted file mode 100644 index 4ed18fbb8b01..000000000000 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/client-page.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import * as Sentry from '@sentry/nextjs'; -import { use } from 'react'; - -export function ClientComponentTakingAPromise({ promise }: { promise: Promise }) { - let value; - try { - value = use(promise); - } catch (e) { - Sentry.captureException(e); // This error should not be reported - throw e; - } - return

Promise value: {value}

; -} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx deleted file mode 100644 index 99672a08c52a..000000000000 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/loading.tsx +++ /dev/null @@ -1,3 +0,0 @@ -export default function Loading() { - return

Loading...

; -} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx index 36ca1d1cd01a..007e2e2feac2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/app/suspense-error/page.tsx @@ -1,8 +1,15 @@ -import { ClientComponentTakingAPromise } from './client-page'; - +import * as Sentry from '@sentry/nextjs'; +import { use } from 'react'; export const dynamic = 'force-dynamic'; export default async function Page() { - const promise = fetch('http://example.com/').then(() => 'foobar'); - return ; + try { + use(fetch('http://example.com/')); + } catch (e) { + Sentry.captureException(e); // This error should not be reported + await new Promise(resolve => setTimeout(resolve, 1000)); // Wait for any async event processors to run + await Sentry.flush(); + } + + return

test

; } From e8381dcf045880cc45a78e3a92aa66c7182745c4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 May 2024 13:22:59 +0000 Subject: [PATCH 3/5] fix --- packages/nextjs/src/server/index.ts | 30 ++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 2b5d7251186a..270152626c7c 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -10,7 +10,7 @@ import { isBuild } from '../common/utils/isBuild'; import { distDirRewriteFramesIntegration } from './distDirRewriteFramesIntegration'; export * from '@sentry/node'; -import type { EventProcessor } from '@sentry/types'; +import type { Event, EventProcessor } from '@sentry/types'; import { httpIntegration } from './httpIntegration'; export { httpIntegration }; @@ -190,13 +190,22 @@ export function init(options: NodeOptions): void { const originalException = hint.originalException; - const isReactControlFlowError = + const isPostponeError = typeof originalException === 'object' && originalException !== null && '$$typeof' in originalException && originalException.$$typeof === Symbol.for('react.postpone'); - if (isReactControlFlowError) { + if (isPostponeError) { + // Postpone errors are used for partial-pre-rendering (PPR) + return null; + } + + // We don't want to capture suspense errors as they are simply used by React/Next.js for control flow + if ( + isReactErrorEventWithErrorCode(event, 460) || // Suspense Exception: This is not a real error! It's an implementation detail... + isReactErrorEventWithErrorCode(event, 474) // Suspense Exception: This is not a real error, and should not leak into... + ) { return null; } @@ -220,3 +229,18 @@ function sdkAlreadyInitialized(): boolean { export * from '../common'; export { wrapApiHandlerWithSentry } from '../common/wrapApiHandlerWithSentry'; + +// https://github.com/facebook/react/blob/6f23540c7d39d7da2091284322008dadd055c031/scripts/error-codes/codes.json +function isReactErrorEventWithErrorCode(event: Event, errorCode: number): boolean { + const exceptionValue = event.exception && event.exception.values && event.exception.values[0].value; + if (typeof exceptionValue !== 'string') { + return false; + } + + // Example https://reactjs.org/docs/error-decoder.html?invariant=423 + // With newer React versions, the messages changed to a different website https://react.dev/errors/418 + return ( + exceptionValue.includes(`reactjs.org/docs/error-decoder.html?invariant=${errorCode}`) || + exceptionValue.includes(`react.dev/errors/${errorCode}`) + ); +} From 03fb671384fdb099c857c2e78aba90e3c2007af8 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 May 2024 14:16:18 +0000 Subject: [PATCH 4/5] meep --- .../nextjs-15/tests/suspense-error.test.ts | 8 ++++---- packages/nextjs/src/server/index.ts | 20 +++---------------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts index 50fc4fa4f86c..ddbc4a9edee3 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/tests/suspense-error.test.ts @@ -6,11 +6,11 @@ test('should not capture serverside suspense errors', async ({ page }) => { return transactionEvent?.transaction === 'Page Server Component (/suspense-error)'; }); - let errorEventReceived = false; + let errorEvent; waitForError('nextjs-15', async transactionEvent => { return transactionEvent?.transaction === 'Page Server Component (/suspense-error)'; - }).then(() => { - errorEventReceived = true; + }).then(event => { + errorEvent = event; }); await page.goto(`/suspense-error`); @@ -20,5 +20,5 @@ test('should not capture serverside suspense errors', async ({ page }) => { const pageServerComponentTransaction = await pageServerComponentTransactionPromise; expect(pageServerComponentTransaction).toBeDefined(); - expect(errorEventReceived).toBe(false); + expect(errorEvent).toBeUndefined(); }); diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 270152626c7c..eff5767405e3 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -202,9 +202,10 @@ export function init(options: NodeOptions): void { } // We don't want to capture suspense errors as they are simply used by React/Next.js for control flow + const exceptionMessage = event.exception?.values?.[0]?.value; if ( - isReactErrorEventWithErrorCode(event, 460) || // Suspense Exception: This is not a real error! It's an implementation detail... - isReactErrorEventWithErrorCode(event, 474) // Suspense Exception: This is not a real error, and should not leak into... + exceptionMessage?.includes('Suspense Exception: This is not a real error!') || + exceptionMessage?.includes('Suspense Exception: This is not a real error, and should not leak') ) { return null; } @@ -229,18 +230,3 @@ function sdkAlreadyInitialized(): boolean { export * from '../common'; export { wrapApiHandlerWithSentry } from '../common/wrapApiHandlerWithSentry'; - -// https://github.com/facebook/react/blob/6f23540c7d39d7da2091284322008dadd055c031/scripts/error-codes/codes.json -function isReactErrorEventWithErrorCode(event: Event, errorCode: number): boolean { - const exceptionValue = event.exception && event.exception.values && event.exception.values[0].value; - if (typeof exceptionValue !== 'string') { - return false; - } - - // Example https://reactjs.org/docs/error-decoder.html?invariant=423 - // With newer React versions, the messages changed to a different website https://react.dev/errors/418 - return ( - exceptionValue.includes(`reactjs.org/docs/error-decoder.html?invariant=${errorCode}`) || - exceptionValue.includes(`react.dev/errors/${errorCode}`) - ); -} From b243718208f5fe61ab930d42df819e1d127cda73 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 May 2024 14:23:45 +0000 Subject: [PATCH 5/5] lint --- packages/nextjs/src/server/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index eff5767405e3..9ffdcfdc6225 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -10,7 +10,7 @@ import { isBuild } from '../common/utils/isBuild'; import { distDirRewriteFramesIntegration } from './distDirRewriteFramesIntegration'; export * from '@sentry/node'; -import type { Event, EventProcessor } from '@sentry/types'; +import type { EventProcessor } from '@sentry/types'; import { httpIntegration } from './httpIntegration'; export { httpIntegration };