Simplify next-dev-server implementation (vercel#26230)

`next-dev-server` having its own implementations of `renderToHTML` and `renderErrorToHTML` has historically made reasoning about streaming hard, as it adds additional places where status codes are explicitly set and the full HTML is blocked on.

Instead, this PR simplifies things considerably by moving the majority of the custom logic for e.g. hot reloading and on-demand compilation to when we're resolving the page to be loaded, rather than upfront when handling the request. It also cleans up a few other details (e.g. default error page rendering) that managed to creep into the base implementation over time.

One unfortunate side effect is that this makes compilation errors slightly more difficult. Previously, we'd render them directly. Now, we have to rethrow them. But since they've already been logged (by the watch pipeline), we have to make sure they don't get logged again.

Author: devknoll

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
 }
@@ -1357,7 +1352,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
@@ -1974,18 +1969,27 @@ 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
+      if (!isWrappedError) {
+        if (this.minimalMode) {
+          throw err
+        }
+        this.logError(err)
       }
-      this.logError(err)
       return html
     }
     res.statusCode = 404
@@ -2027,12 +2031,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
@@ -2083,24 +2094,29 @@ export default class Server {
         throw maybeFallbackError
       }
     } catch (renderToHtmlError) {
-      console.error(renderToHtmlError)
+      const isWrappedError = renderToHtmlError instanceof WrappedBuildError
+      if (!isWrappedError) {
+        this.logError(renderToHtmlError)
+      }
       res.statusCode = 500
+      const fallbackComponents = await this.getFallbackErrorComponents()
 
-      if (this.renderOpts.dev) {
-        await ((this as any).hotReloader as HotReloader).buildFallbackError()
-
-        const fallbackResult = await loadDefaultErrorComponents(this.distDir)
+      if (fallbackComponents) {
         return this.renderToHTMLWithComponents(
           req,
           res,
           '/_error',
           {
             query,
-            components: fallbackResult,
+            components: fallbackComponents,
           },
           {
             ...this.renderOpts,
-            err,
+            // We render `renderToHtmlError` here because `err` is
+            // already captured in the stacktrace.
+            err: isWrappedError
+              ? renderToHtmlError.innerError
+              : renderToHtmlError,
           }
         )
       }
@@ -2109,6 +2125,11 @@ export default class Server {
     return html
   }
 
+  protected async getFallbackErrorComponents(): Promise<LoadComponentsReturnType | null> {
+    // The development server will provide an implementation for this
+    return null
+  }
+
   public async render404(
     req: IncomingMessage,
     res: ServerResponse,
@@ -2269,3 +2290,14 @@ 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
+
+  constructor(innerError: Error) {
+    super()
+    this.innerError = innerError
+  }
+}

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'
@@ -19,7 +18,6 @@ import {
   PHASE_DEVELOPMENT_SERVER,
   CLIENT_STATIC_FILES_PATH,
   DEV_CLIENT_PAGES_MANIFEST,
-  STATIC_STATUS_PAGES,
 } from '../next-server/lib/constants'
 import {
   getRouteMatcher,
@@ -28,7 +26,11 @@ 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, {
+  WrappedBuildError,
+  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'
 import { eventCliSession } from '../telemetry/events'
@@ -39,6 +41,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(
@@ -600,95 +607,34 @@ 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<string | null> {
+    query: ParsedUrlQuery = {},
+    params: Params | null = null
+  ): Promise<FindComponentsResult | null> {
     await this.devReady
     const compilationErr = await this.getCompilationError(pathname)
     if (compilationErr) {
-      res.statusCode = 500
-      return this.renderErrorToHTML(compilationErr, req, res, pathname, query)
+      // Wrap build errors so that they don't get logged again
+      throw new WrappedBuildError(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<string | null> {
-    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 getFallbackErrorComponents(): Promise<LoadComponentsReturnType | null> {
+    await this.hotReloader!.buildFallbackError()
+    // 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(