From 02b760ae6ebb231e2d981b572ec8b2a48cac34f1 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 15:14:45 +0200 Subject: [PATCH 1/7] Create hash digest for errors in app in production --- packages/next/server/app-render.tsx | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index ce766b8aaab81..4b9d4715ec8e6 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -42,6 +42,7 @@ import { DYNAMIC_ERROR_CODE } from '../client/components/hooks-server-context' import { NOT_FOUND_ERROR_CODE } from '../client/components/not-found' import { HeadManagerContext } from '../shared/lib/head-manager-context' import { Writable } from 'stream' +import stringHash from 'next/dist/compiled/string-hash' const INTERNAL_HEADERS_INSTANCE = Symbol('internal for headers readonly') @@ -178,19 +179,23 @@ function createErrorHandler( ) { return (err: any) => { if ( - // TODO-APP: Handle redirect throw - err.digest !== DYNAMIC_ERROR_CODE && - err.digest !== NOT_FOUND_ERROR_CODE && - !err.digest?.startsWith(REDIRECT_ERROR_CODE) + err.digest === DYNAMIC_ERROR_CODE || + err.digest === NOT_FOUND_ERROR_CODE || + err.digest?.startsWith(REDIRECT_ERROR_CODE) ) { - // Used for debugging error source - // console.error(_source, err) - console.error(err) - capturedErrors.push(err) - return err.digest || err.message + return err.digest } - return err.digest + // Used for debugging error source + // console.error(_source, err) + console.error(err) + capturedErrors.push(err) + if (err.digest) { + return err.digest + } + + // TODO-APP: look at using webcrypto instead. Requires a promise to be awaited. + return stringHash(err.message) } } From 63eba886eeacdd8ab005b298d0dac2941de1683c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 15:21:21 +0200 Subject: [PATCH 2/7] Ensure return value is a string --- packages/next/server/app-render.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 4b9d4715ec8e6..bf4ce2148efcc 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -195,7 +195,7 @@ function createErrorHandler( } // TODO-APP: look at using webcrypto instead. Requires a promise to be awaited. - return stringHash(err.message) + return stringHash(err.message).toString() } } From 623d64bea79554cdc5589accc9d29fb194ae93c2 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 15:21:46 +0200 Subject: [PATCH 3/7] Add string type --- packages/next/server/app-render.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index bf4ce2148efcc..1da8c16eda666 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -177,7 +177,7 @@ function createErrorHandler( _source: string, capturedErrors: Error[] ) { - return (err: any) => { + return (err: any): string => { if ( err.digest === DYNAMIC_ERROR_CODE || err.digest === NOT_FOUND_ERROR_CODE || From e6ffbddaf554478221ce448a2eb6fed53ca24aa7 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 15:41:51 +0200 Subject: [PATCH 4/7] Add test for digest --- .../app/app/error/server-component/error.js | 13 +++++++ .../app/app/error/server-component/page.js | 7 ++++ test/e2e/app-dir/index.test.ts | 36 +++++++++++++++++-- 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 test/e2e/app-dir/app/app/error/server-component/error.js create mode 100644 test/e2e/app-dir/app/app/error/server-component/page.js diff --git a/test/e2e/app-dir/app/app/error/server-component/error.js b/test/e2e/app-dir/app/app/error/server-component/error.js new file mode 100644 index 0000000000000..e1089170d02c2 --- /dev/null +++ b/test/e2e/app-dir/app/app/error/server-component/error.js @@ -0,0 +1,13 @@ +'use client' + +export default function ErrorBoundary({ error, reset }) { + return ( + <> +

{error.message}

+

{error.digest}

+ + + ) +} diff --git a/test/e2e/app-dir/app/app/error/server-component/page.js b/test/e2e/app-dir/app/app/error/server-component/page.js new file mode 100644 index 0000000000000..f771b09bbbfd1 --- /dev/null +++ b/test/e2e/app-dir/app/app/error/server-component/page.js @@ -0,0 +1,7 @@ +export const config = { + revalidate: 0, +} + +export default function Page() { + throw new Error('this is a test') +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index ffb6334823352..e2249173847be 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1553,8 +1553,9 @@ describe('app dir', () => { if (isDev) { expect(await hasRedbox(browser)).toBe(true) - console.log('getRedboxHeader', await getRedboxHeader(browser)) - // expect(await getRedboxHeader(browser)).toMatch(/An error occurred: this is a test/) + expect(await getRedboxHeader(browser)).toMatch( + /An error occurred: this is a test/ + ) } else { await browser expect( @@ -1566,6 +1567,37 @@ describe('app dir', () => { } }) + it('should trigger error component when an error happens during server components rendering', async () => { + const browser = await webdriver(next.url, '/error/server-component') + + if (isDev) { + expect( + await browser + .waitForElementByCss('#error-boundary-message') + .elementByCss('#error-boundary-message') + .text() + ).toBe('this is a test') + expect( + await browser.waitForElementByCss('#error-boundary-digest').text() + // Digest of the error message should be stable. + ).toBe('2397877870') + // TODO-APP: ensure error overlay is shown for errors that happened before/during hydration + // expect(await hasRedbox(browser)).toBe(true) + // expect(await getRedboxHeader(browser)).toMatch(/this is a test/) + } else { + await browser + expect( + await browser.waitForElementByCss('#error-boundary-message').text() + ).toBe( + 'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.' + ) + expect( + await browser.waitForElementByCss('#error-boundary-digest').text() + // Digest of the error message should be stable. + ).toBe('2397877870') + } + }) + it('should use default error boundary for prod and overlay for dev when no error component specified', async () => { const browser = await webdriver( next.url, From d52333912bf12d7946fbabedf50df823642c25ae Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 15:54:09 +0200 Subject: [PATCH 5/7] Add test for custom digest --- .../server-component/custom-digest/page.js | 9 +++++ test/e2e/app-dir/index.test.ts | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 test/e2e/app-dir/app/app/error/server-component/custom-digest/page.js diff --git a/test/e2e/app-dir/app/app/error/server-component/custom-digest/page.js b/test/e2e/app-dir/app/app/error/server-component/custom-digest/page.js new file mode 100644 index 0000000000000..4bb541ae218e6 --- /dev/null +++ b/test/e2e/app-dir/app/app/error/server-component/custom-digest/page.js @@ -0,0 +1,9 @@ +export const config = { + revalidate: 0, +} + +export default function Page() { + const err = new Error('this is a test') + err.digest = 'custom' + throw err +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index e2249173847be..9437a421e92b5 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1598,6 +1598,40 @@ describe('app dir', () => { } }) + it('should trigger error component when an error with custom digest happens during server components rendering', async () => { + const browser = await webdriver( + next.url, + '/error/server-component/custom-digest' + ) + + if (isDev) { + expect( + await browser + .waitForElementByCss('#error-boundary-message') + .elementByCss('#error-boundary-message') + .text() + ).toBe('this is a test') + expect( + await browser.waitForElementByCss('#error-boundary-digest').text() + // Digest of the error message should be stable. + ).toBe('custom') + // TODO-APP: ensure error overlay is shown for errors that happened before/during hydration + // expect(await hasRedbox(browser)).toBe(true) + // expect(await getRedboxHeader(browser)).toMatch(/this is a test/) + } else { + await browser + expect( + await browser.waitForElementByCss('#error-boundary-message').text() + ).toBe( + 'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.' + ) + expect( + await browser.waitForElementByCss('#error-boundary-digest').text() + // Digest of the error message should be stable. + ).toBe('custom') + } + }) + it('should use default error boundary for prod and overlay for dev when no error component specified', async () => { const browser = await webdriver( next.url, From 8995f6b52f95d9b735b4b8ecb16e1189f5bb3581 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 16:51:57 +0200 Subject: [PATCH 6/7] Fix test --- test/e2e/app-dir/index.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 9437a421e92b5..fb5ec766d1c00 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1553,9 +1553,7 @@ describe('app dir', () => { if (isDev) { expect(await hasRedbox(browser)).toBe(true) - expect(await getRedboxHeader(browser)).toMatch( - /An error occurred: this is a test/ - ) + expect(await getRedboxHeader(browser)).toMatch(/this is a test/) } else { await browser expect( From c5b1580f918473a4b585724c9a3ac4dd6f8c26a0 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 19 Oct 2022 17:56:46 +0200 Subject: [PATCH 7/7] Remove custom digest support. Take into account stack --- packages/next/server/app-render.tsx | 6 +---- test/e2e/app-dir/index.test.ts | 38 ++--------------------------- 2 files changed, 3 insertions(+), 41 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 1da8c16eda666..7846bbb36f6f7 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -190,12 +190,8 @@ function createErrorHandler( // console.error(_source, err) console.error(err) capturedErrors.push(err) - if (err.digest) { - return err.digest - } - // TODO-APP: look at using webcrypto instead. Requires a promise to be awaited. - return stringHash(err.message).toString() + return stringHash(err.message + err.stack + (err.digest || '')).toString() } } diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index fb5ec766d1c00..cc6b318efe65b 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1578,7 +1578,7 @@ describe('app dir', () => { expect( await browser.waitForElementByCss('#error-boundary-digest').text() // Digest of the error message should be stable. - ).toBe('2397877870') + ).not.toBe('') // TODO-APP: ensure error overlay is shown for errors that happened before/during hydration // expect(await hasRedbox(browser)).toBe(true) // expect(await getRedboxHeader(browser)).toMatch(/this is a test/) @@ -1592,41 +1592,7 @@ describe('app dir', () => { expect( await browser.waitForElementByCss('#error-boundary-digest').text() // Digest of the error message should be stable. - ).toBe('2397877870') - } - }) - - it('should trigger error component when an error with custom digest happens during server components rendering', async () => { - const browser = await webdriver( - next.url, - '/error/server-component/custom-digest' - ) - - if (isDev) { - expect( - await browser - .waitForElementByCss('#error-boundary-message') - .elementByCss('#error-boundary-message') - .text() - ).toBe('this is a test') - expect( - await browser.waitForElementByCss('#error-boundary-digest').text() - // Digest of the error message should be stable. - ).toBe('custom') - // TODO-APP: ensure error overlay is shown for errors that happened before/during hydration - // expect(await hasRedbox(browser)).toBe(true) - // expect(await getRedboxHeader(browser)).toMatch(/this is a test/) - } else { - await browser - expect( - await browser.waitForElementByCss('#error-boundary-message').text() - ).toBe( - 'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.' - ) - expect( - await browser.waitForElementByCss('#error-boundary-digest').text() - // Digest of the error message should be stable. - ).toBe('custom') + ).not.toBe('') } })