From 41fad8afbd13533612fceb14a4456b71f0f13b74 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 16:03:26 +0000 Subject: [PATCH 01/23] Simplify dev server implementation --- .../next/next-server/server/next-server.ts | 27 ++--- packages/next/server/next-dev-server.ts | 103 +++++------------- 2 files changed, 40 insertions(+), 90 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 885937ea74b58..f06c9feaa181a 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -52,11 +52,7 @@ import { import { DomainLocales, isTargetLikeServerless, NextConfig } from './config' import pathMatch from '../lib/router/utils/path-match' import { recursiveReadDirSync } from './lib/recursive-readdir-sync' -import { - loadComponents, - LoadComponentsReturnType, - loadDefaultErrorComponents, -} from './load-components' +import { loadComponents, LoadComponentsReturnType } from './load-components' import { normalizePagePath } from './normalize-page-path' import { RenderOpts, RenderOptsPartial, renderToHTML } from './render' import { getPagePath, requireFontManifest } from './require' @@ -92,7 +88,6 @@ import cookie from 'next/dist/compiled/cookie' import escapePathDelimiters from '../lib/router/utils/escape-path-delimiters' import { getUtils } from '../../build/webpack/loaders/next-serverless-loader/utils' import { PreviewData } from 'next/types' -import HotReloader from '../../server/hot-reloader' const getCustomRouteMatcher = pathMatch(true) @@ -102,7 +97,7 @@ type Middleware = ( next: (err?: Error) => void ) => void -type FindComponentsResult = { +export type FindComponentsResult = { components: LoadComponentsReturnType query: ParsedUrlQuery } @@ -1346,7 +1341,7 @@ export default class Server { return this.sendHTML(req, res, html) } - private async findPageComponents( + protected async findPageComponents( pathname: string, query: ParsedUrlQuery = {}, params: Params | null = null @@ -2067,13 +2062,12 @@ export default class Server { throw maybeFallbackError } } catch (renderToHtmlError) { - console.error(renderToHtmlError) res.statusCode = 500 + const fallbackResult = await this.handleRenderErrorFailure( + renderToHtmlError + ) - if (this.renderOpts.dev) { - await ((this as any).hotReloader as HotReloader).buildFallbackError() - - const fallbackResult = await loadDefaultErrorComponents(this.distDir) + if (fallbackResult) { return this.renderToHTMLWithComponents( req, res, @@ -2093,6 +2087,13 @@ export default class Server { return html } + protected async handleRenderErrorFailure( + err: Error + ): Promise { + console.error(err) + return null + } + public async render404( req: IncomingMessage, res: ServerResponse, diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 725dd63325035..3bad56ce448ad 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -28,7 +28,10 @@ import { isDynamicRoute, } from '../next-server/lib/router/utils' import { __ApiPreviewProps } from '../next-server/server/api-utils' -import Server, { ServerConstructor } from '../next-server/server/next-server' +import Server, { + FindComponentsResult, + ServerConstructor, +} from '../next-server/server/next-server' import { normalizePagePath } from '../next-server/server/normalize-page-path' import Router, { Params, route } from '../next-server/server/router' import { eventCliSession } from '../telemetry/events' @@ -39,6 +42,11 @@ import { findPageFile } from './lib/find-page-file' import { getNodeOptionsWithoutInspect } from './lib/utils' import { withCoalescedInvoke } from '../lib/coalesced-function' import { NextConfig } from '../next-server/server/config' +import { ParsedUrlQuery } from 'querystring' +import { + LoadComponentsReturnType, + loadDefaultErrorComponents, +} from '../next-server/server/load-components' if (typeof React.Suspense === 'undefined') { throw new Error( @@ -595,95 +603,36 @@ export default class DevServer extends Server { return this.hotReloader!.ensurePage(pathname) } - async renderToHTML( - req: IncomingMessage, - res: ServerResponse, + protected async findPageComponents( pathname: string, - query: { [key: string]: string } - ): Promise { + query: ParsedUrlQuery = {}, + params: Params | null = null + ): Promise { await this.devReady + const compilationErr = await this.getCompilationError(pathname) if (compilationErr) { - res.statusCode = 500 - return this.renderErrorToHTML(compilationErr, req, res, pathname, query) + throw compilationErr } - // In dev mode we use on demand entries to compile the page before rendering try { - await this.hotReloader!.ensurePage(pathname).catch(async (err: Error) => { - if ((err as any).code !== 'ENOENT') { - throw err - } - - for (const dynamicRoute of this.dynamicRoutes || []) { - const params = dynamicRoute.match(pathname) - if (!params) { - continue - } - - return this.hotReloader!.ensurePage(dynamicRoute.page) - } - throw err - }) + await this.hotReloader!.ensurePage(pathname) + return super.findPageComponents(pathname, query, params) } catch (err) { - if (err.code === 'ENOENT') { - try { - await this.hotReloader!.ensurePage('/404') - } catch (hotReloaderError) { - if (hotReloaderError.code !== 'ENOENT') { - throw hotReloaderError - } - } - - res.statusCode = 404 - return this.renderErrorToHTML(null, req, res, pathname, query) + if ((err as any).code !== 'ENOENT') { + throw err } if (!this.quiet) console.error(err) + return null } - const html = await super.renderToHTML(req, res, pathname, query) - return html } - async renderErrorToHTML( - err: Error | null, - req: IncomingMessage, - res: ServerResponse, - pathname: string, - query: { [key: string]: string } - ): Promise { - await this.devReady - if (res.statusCode === 404 && (await this.hasPage('/404'))) { - await this.hotReloader!.ensurePage('/404') - } else if ( - STATIC_STATUS_PAGES.includes(`/${res.statusCode}`) && - (await this.hasPage(`/${res.statusCode}`)) - ) { - await this.hotReloader!.ensurePage(`/${res.statusCode}`) - } else { - await this.hotReloader!.ensurePage('/_error') - } - - const compilationErr = await this.getCompilationError(pathname) - if (compilationErr) { - res.statusCode = 500 - return super.renderErrorToHTML(compilationErr, req, res, pathname, query) - } - - if (!err && res.statusCode === 500) { - err = new Error( - 'An undefined error was thrown sometime during render... ' + - 'See https://nextjs.org/docs/messages/threw-undefined' - ) - } - - try { - const out = await super.renderErrorToHTML(err, req, res, pathname, query) - return out - } catch (err2) { - if (!this.quiet) Log.error(err2) - res.statusCode = 500 - return super.renderErrorToHTML(err2, req, res, pathname, query) - } + protected async handleRenderErrorFailure( + err: Error + ): Promise { + if (!this.quiet) Log.error(err as any) + await this.hotReloader!.buildFallbackError() + return loadDefaultErrorComponents(this.distDir) } sendHTML( From 195d10dd849aba5c385d547de4f07db412a04808 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 16:11:16 +0000 Subject: [PATCH 02/23] Fix lint --- packages/next/server/next-dev-server.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 3bad56ce448ad..1e7b5c4ed7b69 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -19,7 +19,6 @@ import { PHASE_DEVELOPMENT_SERVER, CLIENT_STATIC_FILES_PATH, DEV_CLIENT_PAGES_MANIFEST, - STATIC_STATUS_PAGES, } from '../next-server/lib/constants' import { getRouteMatcher, From f1d7c00af9a0b600f2e5728e03f247133b865d02 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 17:31:47 +0000 Subject: [PATCH 03/23] Fix tests --- packages/next/server/next-dev-server.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 1e7b5c4ed7b69..da4b90b0763f4 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -608,20 +608,18 @@ export default class DevServer extends Server { params: Params | null = null ): Promise { await this.devReady - - const compilationErr = await this.getCompilationError(pathname) - if (compilationErr) { - throw compilationErr - } - try { await this.hotReloader!.ensurePage(pathname) + const compilationErr = await this.getCompilationError(pathname) + if (compilationErr) { + throw compilationErr + } return super.findPageComponents(pathname, query, params) } catch (err) { if ((err as any).code !== 'ENOENT') { + if (!this.quiet) console.error(err) throw err } - if (!this.quiet) console.error(err) return null } } From 89d974d22bb78825de95aaafc9143f639a343a3e Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 17:34:36 +0000 Subject: [PATCH 04/23] Don't log errors twice --- packages/next/server/next-dev-server.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index da4b90b0763f4..dd350500dd411 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -617,7 +617,6 @@ export default class DevServer extends Server { return super.findPageComponents(pathname, query, params) } catch (err) { if ((err as any).code !== 'ENOENT') { - if (!this.quiet) console.error(err) throw err } return null From 956ef2de942463912a9ab530da802a85a5bb1f38 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 17:40:47 +0000 Subject: [PATCH 05/23] Minor tweaks --- packages/next/next-server/server/next-server.ts | 6 ++++-- packages/next/server/next-dev-server.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index f06c9feaa181a..ba9f4bc9f5c4e 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2063,7 +2063,7 @@ export default class Server { } } catch (renderToHtmlError) { res.statusCode = 500 - const fallbackResult = await this.handleRenderErrorFailure( + const fallbackResult = await this.handleRenderErrorFallback( renderToHtmlError ) @@ -2087,7 +2087,9 @@ export default class Server { return html } - protected async handleRenderErrorFailure( + // Internal. This is overridden by the development server to render + // with default error components. + protected async handleRenderErrorFallback( err: Error ): Promise { console.error(err) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index dd350500dd411..cb1418b53905b 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -623,7 +623,7 @@ export default class DevServer extends Server { } } - protected async handleRenderErrorFailure( + protected async handleRenderErrorFallback( err: Error ): Promise { if (!this.quiet) Log.error(err as any) From 9fbfa87e5b3c25133e0377bdc6fe3d381a877c10 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 17:46:21 +0000 Subject: [PATCH 06/23] More clean up --- packages/next/next-server/server/next-server.ts | 14 +++++--------- packages/next/server/next-dev-server.ts | 5 +---- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ba9f4bc9f5c4e..a0baeecde26a3 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2062,10 +2062,9 @@ export default class Server { throw maybeFallbackError } } catch (renderToHtmlError) { + console.error(renderToHtmlError) res.statusCode = 500 - const fallbackResult = await this.handleRenderErrorFallback( - renderToHtmlError - ) + const fallbackResult = await this.getFallbackErrorComponents() if (fallbackResult) { return this.renderToHTMLWithComponents( @@ -2087,12 +2086,9 @@ export default class Server { return html } - // Internal. This is overridden by the development server to render - // with default error components. - protected async handleRenderErrorFallback( - err: Error - ): Promise { - console.error(err) + // Internal + protected async getFallbackErrorComponents(): Promise { + // The development server will provide an implementation for this return null } diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index cb1418b53905b..7f98fc6b4be72 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -623,10 +623,7 @@ export default class DevServer extends Server { } } - protected async handleRenderErrorFallback( - err: Error - ): Promise { - if (!this.quiet) Log.error(err as any) + protected async getFallbackErrorComponents(): Promise { await this.hotReloader!.buildFallbackError() return loadDefaultErrorComponents(this.distDir) } From 283c9c57290bb4ba7513f7c41472119a9200c43a Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 17:59:32 +0000 Subject: [PATCH 07/23] Add back default error --- packages/next/next-server/server/next-server.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index a0baeecde26a3..ee2a9694db947 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2006,12 +2006,19 @@ export default class Server { }) public async renderErrorToHTML( - err: Error | null, + _err: Error | null, req: IncomingMessage, res: ServerResponse, _pathname: string, query: ParsedUrlQuery = {} ) { + let err = _err + if (this.renderOpts.dev && !err && res.statusCode === 500) { + err = new Error( + 'An undefined error was thrown sometime during render... ' + + 'See https://nextjs.org/docs/messages/threw-undefined' + ) + } let html: string | null try { let result: null | FindComponentsResult = null From fe6e0c942c70c813c3af68da395af3eb4faa46c7 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 18:05:48 +0000 Subject: [PATCH 08/23] Fix tests --- test/integration/build-output/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 5b5e04ebb0c0d..66848e5e0c781 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -149,7 +149,7 @@ describe('Build Output', () => { true ) - expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.1 : 62.8, 1) + expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.1 : 62.7, 1) expect(mainSize.endsWith('kB')).toBe(true) expect(parseFloat(frameworkSize)).toBeCloseTo(gz ? 42.0 : 130, 1) From 474b873447286286449d1c3f02859557bea892fe Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 18:14:58 +0000 Subject: [PATCH 09/23] Fix lint --- packages/next/server/next-dev-server.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 7f98fc6b4be72..38f2ecc451cff 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -9,7 +9,6 @@ import React from 'react' import { UrlWithParsedQuery } from 'url' import Watchpack from 'watchpack' import { ampValidation } from '../build/output/index' -import * as Log from '../build/output/log' import { PUBLIC_DIR_MIDDLEWARE_CONFLICT } from '../lib/constants' import { fileExists } from '../lib/file-exists' import { findPagesDir } from '../lib/find-pages-dir' From 1c25044fd906ba0e6535b7049af4d61f1fadf8eb Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 18:41:45 +0000 Subject: [PATCH 10/23] Fix tests --- packages/next/next-server/server/next-server.ts | 8 ++++++-- packages/next/server/next-dev-server.ts | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ee2a9694db947..6cb52b79c8dbb 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2071,7 +2071,9 @@ export default class Server { } catch (renderToHtmlError) { console.error(renderToHtmlError) res.statusCode = 500 - const fallbackResult = await this.getFallbackErrorComponents() + const fallbackResult = await this.handleRenderErrorFallback( + renderToHtmlError + ) if (fallbackResult) { return this.renderToHTMLWithComponents( @@ -2094,7 +2096,9 @@ export default class Server { } // Internal - protected async getFallbackErrorComponents(): Promise { + protected async handleRenderErrorFallback( + _err: Error + ): Promise { // The development server will provide an implementation for this return null } diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 38f2ecc451cff..cb1418b53905b 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -9,6 +9,7 @@ import React from 'react' import { UrlWithParsedQuery } from 'url' import Watchpack from 'watchpack' import { ampValidation } from '../build/output/index' +import * as Log from '../build/output/log' import { PUBLIC_DIR_MIDDLEWARE_CONFLICT } from '../lib/constants' import { fileExists } from '../lib/file-exists' import { findPagesDir } from '../lib/find-pages-dir' @@ -622,7 +623,10 @@ export default class DevServer extends Server { } } - protected async getFallbackErrorComponents(): Promise { + protected async handleRenderErrorFallback( + err: Error + ): Promise { + if (!this.quiet) Log.error(err as any) await this.hotReloader!.buildFallbackError() return loadDefaultErrorComponents(this.distDir) } From 7a3cfb0e7cda0571c1da87aa02001231354e455c Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Tue, 15 Jun 2021 19:52:42 +0000 Subject: [PATCH 11/23] Fix tests --- packages/next/server/next-dev-server.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index cb1418b53905b..1e3d13fbee3fe 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -617,6 +617,7 @@ export default class DevServer extends Server { return super.findPageComponents(pathname, query, params) } catch (err) { if ((err as any).code !== 'ENOENT') { + if (!this.quiet) console.error(err) throw err } return null From 86171c73018d85212fad47257dbc7723de848ac3 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:19:42 +0000 Subject: [PATCH 12/23] Fix tests --- packages/next/next-server/server/next-server.ts | 11 +++++++++-- packages/next/server/next-dev-server.ts | 13 +++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 6cb52b79c8dbb..e2d9b20a142b8 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -165,6 +165,7 @@ export default class Server { protected router: Router protected dynamicRoutes?: DynamicRoutes protected customRoutes: CustomRoutes + private rethrownErrors: WeakSet public constructor({ dir = '.', @@ -174,6 +175,7 @@ export default class Server { minimalMode = false, customServer = true, }: ServerConstructor & { conf: NextConfig; minimalMode?: boolean }) { + this.rethrownErrors = new WeakSet() this.dir = resolve(dir) this.quiet = quiet loadEnvConfig(this.dir, dev, Log) @@ -279,7 +281,7 @@ export default class Server { } public logError(err: Error): void { - if (this.quiet) return + if (this.quiet || this.rethrownErrors.has(err)) return console.error(err) } @@ -619,6 +621,11 @@ export default class Server { return this.getPrerenderManifest().preview } + protected markRethrownError(error: T): T { + this.rethrownErrors.add(error) + return error + } + protected generateRoutes(): { basePath: string headers: Route[] @@ -2069,7 +2076,7 @@ export default class Server { throw maybeFallbackError } } catch (renderToHtmlError) { - console.error(renderToHtmlError) + this.logError(renderToHtmlError) res.statusCode = 500 const fallbackResult = await this.handleRenderErrorFallback( renderToHtmlError diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 1e3d13fbee3fe..3e74a39cf43fa 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -609,25 +609,22 @@ export default class DevServer extends Server { ): Promise { await this.devReady try { - await this.hotReloader!.ensurePage(pathname) const compilationErr = await this.getCompilationError(pathname) if (compilationErr) { throw compilationErr } - return super.findPageComponents(pathname, query, params) + await this.hotReloader!.ensurePage(pathname) } catch (err) { - if ((err as any).code !== 'ENOENT') { - if (!this.quiet) console.error(err) - throw err - } - return null + // Development compilation errors have already been logged, + // so we mark them before throwing so they aren't logged again. + throw this.markRethrownError(err) } + return super.findPageComponents(pathname, query, params) } protected async handleRenderErrorFallback( err: Error ): Promise { - if (!this.quiet) Log.error(err as any) await this.hotReloader!.buildFallbackError() return loadDefaultErrorComponents(this.distDir) } From fb5989149b678f48ea7a56c787392a92a28579de Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:21:08 +0000 Subject: [PATCH 13/23] Clean up --- packages/next/next-server/server/next-server.ts | 9 ++------- packages/next/server/next-dev-server.ts | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index e2d9b20a142b8..b351d304c08d6 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2078,9 +2078,7 @@ export default class Server { } catch (renderToHtmlError) { this.logError(renderToHtmlError) res.statusCode = 500 - const fallbackResult = await this.handleRenderErrorFallback( - renderToHtmlError - ) + const fallbackResult = await this.getFallbackErrorComponents() if (fallbackResult) { return this.renderToHTMLWithComponents( @@ -2102,10 +2100,7 @@ export default class Server { return html } - // Internal - protected async handleRenderErrorFallback( - _err: Error - ): Promise { + protected async getFallbackErrorComponents(): Promise { // The development server will provide an implementation for this return null } diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 3e74a39cf43fa..77547901006c1 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -622,9 +622,7 @@ export default class DevServer extends Server { return super.findPageComponents(pathname, query, params) } - protected async handleRenderErrorFallback( - err: Error - ): Promise { + protected async getFallbackErrorComponents(): Promise { await this.hotReloader!.buildFallbackError() return loadDefaultErrorComponents(this.distDir) } From 5b919b63c5885c8a35b893703e516455bc86197d Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:28:06 +0000 Subject: [PATCH 14/23] Fix lint --- packages/next/server/next-dev-server.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 77547901006c1..ac603e1c2bd5c 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -9,7 +9,6 @@ import React from 'react' import { UrlWithParsedQuery } from 'url' import Watchpack from 'watchpack' import { ampValidation } from '../build/output/index' -import * as Log from '../build/output/log' import { PUBLIC_DIR_MIDDLEWARE_CONFLICT } from '../lib/constants' import { fileExists } from '../lib/file-exists' import { findPagesDir } from '../lib/find-pages-dir' From 51c8dc30e9ab8a775d1841e05686e9a9474724bc Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:41:58 +0000 Subject: [PATCH 15/23] Fix tests --- packages/next/server/next-dev-server.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index ac603e1c2bd5c..cf42d83d0fd39 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -614,6 +614,9 @@ export default class DevServer extends Server { } await this.hotReloader!.ensurePage(pathname) } catch (err) { + if ((err as any).code === 'ENOENT') { + return null + } // Development compilation errors have already been logged, // so we mark them before throwing so they aren't logged again. throw this.markRethrownError(err) From ec34649cfd6086409bf9e8a4fc01d72fc180bead Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:53:15 +0000 Subject: [PATCH 16/23] Fix tests --- packages/next/server/next-dev-server.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index cf42d83d0fd39..556b30db9da23 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -608,20 +608,18 @@ export default class DevServer extends Server { ): Promise { await this.devReady try { + await this.hotReloader!.ensurePage(pathname) const compilationErr = await this.getCompilationError(pathname) if (compilationErr) { - throw compilationErr + throw this.markRethrownError(compilationErr) } - await this.hotReloader!.ensurePage(pathname) + return super.findPageComponents(pathname, query, params) } catch (err) { - if ((err as any).code === 'ENOENT') { - return null + if ((err as any).code !== 'ENOENT') { + throw err } - // Development compilation errors have already been logged, - // so we mark them before throwing so they aren't logged again. - throw this.markRethrownError(err) + return null } - return super.findPageComponents(pathname, query, params) } protected async getFallbackErrorComponents(): Promise { From 365f99cbed5c952472e32cce6cb7670d88c89dd8 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 02:54:53 +0000 Subject: [PATCH 17/23] Fix tests --- packages/next/server/next-dev-server.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 556b30db9da23..c5bd9d466f054 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -608,18 +608,18 @@ export default class DevServer extends Server { ): Promise { await this.devReady try { - await this.hotReloader!.ensurePage(pathname) const compilationErr = await this.getCompilationError(pathname) if (compilationErr) { - throw this.markRethrownError(compilationErr) + throw compilationErr } - return super.findPageComponents(pathname, query, params) + await this.hotReloader!.ensurePage(pathname) } catch (err) { if ((err as any).code !== 'ENOENT') { - throw err + throw this.markRethrownError(err) } return null } + return super.findPageComponents(pathname, query, params) } protected async getFallbackErrorComponents(): Promise { From 928778dcd53f4218410d3b26607b0179d55789e8 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 03:09:10 +0000 Subject: [PATCH 18/23] Fix tests --- packages/next/server/next-dev-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index c5bd9d466f054..6eba88ab37e22 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -608,11 +608,11 @@ export default class DevServer extends Server { ): Promise { await this.devReady try { + await this.hotReloader!.ensurePage(pathname) const compilationErr = await this.getCompilationError(pathname) if (compilationErr) { throw compilationErr } - await this.hotReloader!.ensurePage(pathname) } catch (err) { if ((err as any).code !== 'ENOENT') { throw this.markRethrownError(err) From 6627f3aabbafaa251c6ecf5ffa525e7a37aa83d6 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 03:21:01 +0000 Subject: [PATCH 19/23] Fix tests --- packages/next/server/next-dev-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 6eba88ab37e22..30e5196e0670a 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -613,13 +613,13 @@ export default class DevServer extends Server { if (compilationErr) { throw compilationErr } + return super.findPageComponents(pathname, query, params) } catch (err) { if ((err as any).code !== 'ENOENT') { throw this.markRethrownError(err) } return null } - return super.findPageComponents(pathname, query, params) } protected async getFallbackErrorComponents(): Promise { From a586d9b8e5145a7be60719e845cab1f28ced0d41 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 20:16:13 +0000 Subject: [PATCH 20/23] Fix tests --- .../next/next-server/server/next-server.ts | 34 ++++++++++++------- packages/next/server/next-dev-server.ts | 17 ++++++---- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index b351d304c08d6..e2b549e841231 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -165,7 +165,6 @@ export default class Server { protected router: Router protected dynamicRoutes?: DynamicRoutes protected customRoutes: CustomRoutes - private rethrownErrors: WeakSet public constructor({ dir = '.', @@ -175,7 +174,6 @@ export default class Server { minimalMode = false, customServer = true, }: ServerConstructor & { conf: NextConfig; minimalMode?: boolean }) { - this.rethrownErrors = new WeakSet() this.dir = resolve(dir) this.quiet = quiet loadEnvConfig(this.dir, dev, Log) @@ -281,7 +279,7 @@ export default class Server { } public logError(err: Error): void { - if (this.quiet || this.rethrownErrors.has(err)) return + if (this.quiet) return console.error(err) } @@ -621,11 +619,6 @@ export default class Server { return this.getPrerenderManifest().preview } - protected markRethrownError(error: T): T { - this.rethrownErrors.add(error) - return error - } - protected generateRoutes(): { basePath: string headers: Route[] @@ -1960,18 +1953,25 @@ export default class Server { if (isNoFallbackError && bubbleNoFallback) { throw err } + if (err && err.code === 'DECODE_FAILED') { - this.logError(err) res.statusCode = 400 return await this.renderErrorToHTML(err, req, res, pathname, query) } res.statusCode = 500 - const html = await this.renderErrorToHTML(err, req, res, pathname, query) + const isWrappedError = err instanceof WrappedBuildError + const html = await this.renderErrorToHTML( + isWrappedError ? err.innerError : err, + req, + res, + pathname, + query + ) if (this.minimalMode) { throw err } - this.logError(err) + if (!isWrappedError) this.logError(err) return html } res.statusCode = 404 @@ -2076,7 +2076,8 @@ export default class Server { throw maybeFallbackError } } catch (renderToHtmlError) { - this.logError(renderToHtmlError) + if (!(renderToHtmlError instanceof WrappedBuildError)) + this.logError(renderToHtmlError) res.statusCode = 500 const fallbackResult = await this.getFallbackErrorComponents() @@ -2265,3 +2266,12 @@ function prepareServerlessUrl( } class NoFallbackError extends Error {} + +export class WrappedBuildError extends Error { + innerError: Error + + constructor(innerError: Error) { + super() + this.innerError = innerError + } +} diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 30e5196e0670a..a5721dc075424 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -27,6 +27,7 @@ import { } from '../next-server/lib/router/utils' import { __ApiPreviewProps } from '../next-server/server/api-utils' import Server, { + WrappedBuildError, FindComponentsResult, ServerConstructor, } from '../next-server/server/next-server' @@ -607,16 +608,17 @@ export default class DevServer extends Server { params: Params | null = null ): Promise { await this.devReady + const compilationErr = await this.getCompilationError(pathname) + if (compilationErr) { + // Wrap build errors so that they don't get logged again + throw new WrappedBuildError(compilationErr) + } try { await this.hotReloader!.ensurePage(pathname) - const compilationErr = await this.getCompilationError(pathname) - if (compilationErr) { - throw compilationErr - } return super.findPageComponents(pathname, query, params) } catch (err) { if ((err as any).code !== 'ENOENT') { - throw this.markRethrownError(err) + throw err } return null } @@ -624,7 +626,10 @@ export default class DevServer extends Server { protected async getFallbackErrorComponents(): Promise { await this.hotReloader!.buildFallbackError() - return loadDefaultErrorComponents(this.distDir) + // Build the error page to ensure the fallback is built too. + // TODO: See if this can be moved into hotReloader or removed. + await this.hotReloader!.ensurePage('/_error') + return await loadDefaultErrorComponents(this.distDir) } sendHTML( From 970dadecd72a1bd4884db742fc12ec7049f93e99 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 20:26:43 +0000 Subject: [PATCH 21/23] Handle minimal mode properly --- packages/next/next-server/server/next-server.ts | 14 ++++++++------ packages/next/server/next-dev-server.ts | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index e2b549e841231..860d8d7c55668 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1968,10 +1968,12 @@ export default class Server { query ) - if (this.minimalMode) { - throw err + if (!isWrappedError) { + if (this.minimalMode) { + throw err + } + this.logError(err) } - if (!isWrappedError) this.logError(err) return html } res.statusCode = 404 @@ -2079,16 +2081,16 @@ export default class Server { if (!(renderToHtmlError instanceof WrappedBuildError)) this.logError(renderToHtmlError) res.statusCode = 500 - const fallbackResult = await this.getFallbackErrorComponents() + const fallbackComponents = await this.getFallbackErrorComponents() - if (fallbackResult) { + if (fallbackComponents) { return this.renderToHTMLWithComponents( req, res, '/_error', { query, - components: fallbackResult, + components: fallbackComponents, }, { ...this.renderOpts, diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index a5721dc075424..aa8fedac0742d 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -28,8 +28,8 @@ import { import { __ApiPreviewProps } from '../next-server/server/api-utils' import Server, { WrappedBuildError, - FindComponentsResult, ServerConstructor, + FindComponentsResult, } from '../next-server/server/next-server' import { normalizePagePath } from '../next-server/server/normalize-page-path' import Router, { Params, route } from '../next-server/server/router' From 9f91e2c0be3d01440058295bfaa1450e289082ab Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 21:01:22 +0000 Subject: [PATCH 22/23] Fix tests --- packages/next/next-server/server/next-server.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 860d8d7c55668..0347bad0841e8 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2078,8 +2078,10 @@ export default class Server { throw maybeFallbackError } } catch (renderToHtmlError) { - if (!(renderToHtmlError instanceof WrappedBuildError)) + const isWrappedError = renderToHtmlError instanceof WrappedBuildError + if (!isWrappedError) { this.logError(renderToHtmlError) + } res.statusCode = 500 const fallbackComponents = await this.getFallbackErrorComponents() @@ -2094,7 +2096,9 @@ export default class Server { }, { ...this.renderOpts, - err, + err: isWrappedError + ? renderToHtmlError.innerError + : renderToHtmlError, } ) } @@ -2269,6 +2273,8 @@ function prepareServerlessUrl( class NoFallbackError extends Error {} +// Internal wrapper around build errors at development +// time, to prevent us from propagating or logging them export class WrappedBuildError extends Error { innerError: Error From b88cfbb631a557a474c7a62d82e6983ff2f0bd56 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 16 Jun 2021 21:29:41 +0000 Subject: [PATCH 23/23] Comments --- packages/next/next-server/server/next-server.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 0347bad0841e8..a21f7f22f0585 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -2096,6 +2096,8 @@ export default class Server { }, { ...this.renderOpts, + // We render `renderToHtmlError` here because `err` is + // already captured in the stacktrace. err: isWrappedError ? renderToHtmlError.innerError : renderToHtmlError,