From 577bade718d8f23a9347635ae9bea4bdd0bf1e75 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 10 Aug 2022 10:35:53 -0500 Subject: [PATCH 1/2] Ensure default _app is used when falling back to default _error --- packages/next/client/index.tsx | 18 ++++-- .../fatal-render-errror/app/pages/_app.js | 15 +++++ .../fatal-render-errror/app/pages/_error.js | 6 ++ .../fatal-render-errror/app/pages/index.js | 3 + .../app/pages/with-error.js | 6 ++ .../fatal-render-errror/index.test.ts | 58 +++++++++++++++++++ 6 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 test/production/fatal-render-errror/app/pages/_app.js create mode 100644 test/production/fatal-render-errror/app/pages/_error.js create mode 100644 test/production/fatal-render-errror/app/pages/index.js create mode 100644 test/production/fatal-render-errror/app/pages/with-error.js create mode 100644 test/production/fatal-render-errror/index.test.ts diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 5c4f482da59b..535584636276 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -468,7 +468,7 @@ async function render(renderingProps: RenderRouteInfo): Promise { // 404 and 500 errors are special kind of errors // and they are still handle via the main render method. function renderError(renderErrorProps: RenderErrorProps): Promise { - const { App, err } = renderErrorProps + let { App, err } = renderErrorProps // In development runtime errors are caught by our overlay // In production we catch runtime errors using componentDidCatch which will trigger renderError @@ -497,10 +497,18 @@ function renderError(renderErrorProps: RenderErrorProps): Promise { .loadPage('/_error') .then(({ page: ErrorComponent, styleSheets }) => { return lastAppProps?.Component === ErrorComponent - ? import('../pages/_error').then((m) => ({ - ErrorComponent: m.default as React.ComponentType<{}>, - styleSheets: [], - })) + ? import('../pages/_error') + .then((errorModule) => { + return import('../pages/_app').then((appModule) => { + App = appModule.default as any as AppComponent + renderErrorProps.App = App + return errorModule + }) + }) + .then((m) => ({ + ErrorComponent: m.default as React.ComponentType<{}>, + styleSheets: [], + })) : { ErrorComponent, styleSheets } }) .then(({ ErrorComponent, styleSheets }) => { diff --git a/test/production/fatal-render-errror/app/pages/_app.js b/test/production/fatal-render-errror/app/pages/_app.js new file mode 100644 index 000000000000..904b7838ceeb --- /dev/null +++ b/test/production/fatal-render-errror/app/pages/_app.js @@ -0,0 +1,15 @@ +export default function App({ Component, pageProps }) { + if (process.env.NODE_ENV === 'production' && typeof window !== 'undefined') { + if (!window.renderAttempts) { + window.renderAttempts = 0 + } + window.renderAttempts++ + throw new Error('error in custom _app') + } + return ( + <> +

from _app

+ + + ) +} diff --git a/test/production/fatal-render-errror/app/pages/_error.js b/test/production/fatal-render-errror/app/pages/_error.js new file mode 100644 index 000000000000..9b8ff8ee0587 --- /dev/null +++ b/test/production/fatal-render-errror/app/pages/_error.js @@ -0,0 +1,6 @@ +export default function Error() { + if (process.env.NODE_ENV === 'production' && typeof window !== 'undefined') { + throw new Error('error in custom _app') + } + return
Error encountered!
+} diff --git a/test/production/fatal-render-errror/app/pages/index.js b/test/production/fatal-render-errror/app/pages/index.js new file mode 100644 index 000000000000..08263e34c35f --- /dev/null +++ b/test/production/fatal-render-errror/app/pages/index.js @@ -0,0 +1,3 @@ +export default function Page() { + return

index page

+} diff --git a/test/production/fatal-render-errror/app/pages/with-error.js b/test/production/fatal-render-errror/app/pages/with-error.js new file mode 100644 index 000000000000..2a240431ba18 --- /dev/null +++ b/test/production/fatal-render-errror/app/pages/with-error.js @@ -0,0 +1,6 @@ +export default function Error() { + if (process.env.NODE_ENV === 'production' && typeof window !== 'undefined') { + throw new Error('error in pages/with-error') + } + return
with-error
+} diff --git a/test/production/fatal-render-errror/index.test.ts b/test/production/fatal-render-errror/index.test.ts new file mode 100644 index 000000000000..ca920284c328 --- /dev/null +++ b/test/production/fatal-render-errror/index.test.ts @@ -0,0 +1,58 @@ +import { createNext, FileRef } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { check, renderViaHTTP, waitFor } from 'next-test-utils' +import webdriver from 'next-webdriver' +import { join } from 'path' + +describe('fatal-render-errror', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: new FileRef(join(__dirname, 'app')), + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('should render page without error correctly', async () => { + const html = await renderViaHTTP(next.url, '/') + expect(html).toContain('index page') + expect(html).toContain('from _app') + }) + + it('should handle fatal error in _app and _error without loop on direct visit', async () => { + const browser = await webdriver(next.url, '/with-error') + + // wait a bit to see if we are rendering multiple times unexpectedly + await waitFor(500) + // there are 4 render attempts of _app due to first render attempt (initial + // error is thrown), second render attempt with custom _error, third render + // attempt with default _error, and final for query update + expect(await browser.eval('window.renderAttempts')).toBe(4) + + const html = await browser.eval('document.documentElement.innerHTML') + expect(html).not.toContain('from _app') + expect(html).toContain( + 'Application error: a client-side exception has occurred' + ) + }) + + it('should handle fatal error in _app and _error without loop on client-transition', async () => { + const browser = await webdriver(next.url, '/') + await browser.eval('window.renderAttempts = 0') + + await browser.eval('window.next.router.push("/with-error")') + await check(() => browser.eval('location.pathname'), '/with-error') + + // wait a bit to see if we are rendering multiple times unexpectedly + await waitFor(500) + expect(await browser.eval('window.renderAttempts')).toBe(4) + + const html = await browser.eval('document.documentElement.innerHTML') + expect(html).not.toContain('from _app') + expect(html).toContain( + 'Application error: a client-side exception has occurred' + ) + }) +}) From c0863e156caa72084c77baf70aba60da2881a83a Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 10 Aug 2022 10:48:32 -0500 Subject: [PATCH 2/2] make render check less specific due to arbitrary wait --- test/production/fatal-render-errror/index.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/production/fatal-render-errror/index.test.ts b/test/production/fatal-render-errror/index.test.ts index ca920284c328..1037cda593f2 100644 --- a/test/production/fatal-render-errror/index.test.ts +++ b/test/production/fatal-render-errror/index.test.ts @@ -26,10 +26,7 @@ describe('fatal-render-errror', () => { // wait a bit to see if we are rendering multiple times unexpectedly await waitFor(500) - // there are 4 render attempts of _app due to first render attempt (initial - // error is thrown), second render attempt with custom _error, third render - // attempt with default _error, and final for query update - expect(await browser.eval('window.renderAttempts')).toBe(4) + expect(await browser.eval('window.renderAttempts')).toBeLessThan(10) const html = await browser.eval('document.documentElement.innerHTML') expect(html).not.toContain('from _app') @@ -47,7 +44,7 @@ describe('fatal-render-errror', () => { // wait a bit to see if we are rendering multiple times unexpectedly await waitFor(500) - expect(await browser.eval('window.renderAttempts')).toBe(4) + expect(await browser.eval('window.renderAttempts')).toBeLessThan(10) const html = await browser.eval('document.documentElement.innerHTML') expect(html).not.toContain('from _app')