From 57a87050e77e9398842a78e43d48b26542642330 Mon Sep 17 00:00:00 2001
From: JJ Kasper <jj@jjsweb.site>
Date: Tue, 11 Jan 2022 14:40:03 -0600
Subject: [PATCH] Add util for normalizing errors (#33159)

* JSON.stringify generic errors

* Add util for normalizing errors

* lint-fix

* Add better error for null case as well

Co-authored-by: Michael Ozeryansky <mozeryansky@users.noreply.github.com>
---
 errors/threw-undefined.md                     |  18 ++-
 packages/next/client/index.tsx                |   6 +-
 packages/next/lib/eslint/runLintCheck.ts      |   4 +-
 packages/next/lib/is-error.ts                 |  28 +++++
 packages/next/lib/is-plain-object.ts          |  12 ++
 packages/next/lib/is-serializable-props.ts    |  15 +--
 packages/next/server/base-server.ts           |  27 ++---
 packages/next/server/dev/hot-reloader.ts      |   7 +-
 packages/next/server/dev/next-dev-server.ts   |   6 +-
 packages/next/shared/lib/router/router.ts     |   4 +-
 .../acceptance/ReactRefreshLogBox.test.ts     | 109 ++++++++++++++++++
 .../client-navigation/test/rendering.js       |   2 +-
 12 files changed, 187 insertions(+), 51 deletions(-)
 create mode 100644 packages/next/lib/is-plain-object.ts

diff --git a/errors/threw-undefined.md b/errors/threw-undefined.md
index 0d4129ef4f115..4804488cafc49 100644
--- a/errors/threw-undefined.md
+++ b/errors/threw-undefined.md
@@ -1,9 +1,21 @@
-# Threw undefined in render
+# Threw `undefined`/`null`
 
 #### Why This Error Occurred
 
-Somewhere in your code you `throw` an `undefined` value. Since this isn't a valid error there isn't a stack trace. We show this error instead to let you know what to look for.
+Somewhere in your code you `throw` an `undefined` or `null` value. Since this isn't a valid error there isn't a stack trace. We show this error instead to let you know what to look for.
+
+```js
+function getData() {
+  let error
+  throw error
+}
+
+function Page() {
+  const error = data?.error || null
+  throw error
+}
+```
 
 #### Possible Ways to Fix It
 
-Look in your pages and find where an error could be throwing `undefined`
+Look in your pages and find where an error could be throwing `undefined` or `null` values and ensure `new Error()` is used instead.
diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx
index 2b5fa798bdc74..0029830836994 100644
--- a/packages/next/client/index.tsx
+++ b/packages/next/client/index.tsx
@@ -32,7 +32,7 @@ import PageLoader, { StyleSheetTuple } from './page-loader'
 import measureWebVitals from './performance-relayer'
 import { RouteAnnouncer } from './route-announcer'
 import { createRouter, makePublicRouterInstance } from './router'
-import isError from '../lib/is-error'
+import { getProperError } from '../lib/is-error'
 import { trackWebVitalMetric } from './vitals'
 import { RefreshContext } from './rsc/refresh'
 
@@ -339,7 +339,7 @@ export async function initNext(opts: { webpackHMR?: any } = {}) {
     }
   } catch (error) {
     // This catches errors like throwing in the top level of a module
-    initialErr = isError(error) ? error : new Error(error + '')
+    initialErr = getProperError(error)
   }
 
   if (process.env.NODE_ENV === 'development') {
@@ -439,7 +439,7 @@ export async function render(renderingProps: RenderRouteInfo): Promise<void> {
   try {
     await doRender(renderingProps)
   } catch (err) {
-    const renderErr = err instanceof Error ? err : new Error(err + '')
+    const renderErr = getProperError(err)
     // bubble up cancelation errors
     if ((renderErr as Error & { cancelled?: boolean }).cancelled) {
       throw renderErr
diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts
index a6d25c2e77e18..33b63b9492a2f 100644
--- a/packages/next/lib/eslint/runLintCheck.ts
+++ b/packages/next/lib/eslint/runLintCheck.ts
@@ -18,7 +18,7 @@ import { isYarn } from '../is-yarn'
 
 import * as Log from '../../build/output/log'
 import { EventLintCheckCompleted } from '../../telemetry/events/build'
-import isError from '../is-error'
+import isError, { getProperError } from '../is-error'
 
 type Config = {
   plugins: string[]
@@ -221,7 +221,7 @@ async function lint(
       )
       return null
     } else {
-      throw new Error(err + '')
+      throw getProperError(err)
     }
   }
 }
diff --git a/packages/next/lib/is-error.ts b/packages/next/lib/is-error.ts
index a7ef9bf44bc24..4560fa7e928e9 100644
--- a/packages/next/lib/is-error.ts
+++ b/packages/next/lib/is-error.ts
@@ -1,3 +1,5 @@
+import { isPlainObject } from './is-plain-object'
+
 // We allow some additional attached properties for Errors
 export interface NextError extends Error {
   type?: string
@@ -11,3 +13,29 @@ export default function isError(err: unknown): err is NextError {
     typeof err === 'object' && err !== null && 'name' in err && 'message' in err
   )
 }
+
+export function getProperError(err: unknown): Error {
+  if (isError(err)) {
+    return err
+  }
+
+  if (process.env.NODE_ENV === 'development') {
+    // provide better error for case where `throw undefined`
+    // is called in development
+    if (typeof err === 'undefined') {
+      return new Error(
+        'An undefined error was thrown, ' +
+          'see here for more info: https://nextjs.org/docs/messages/threw-undefined'
+      )
+    }
+
+    if (err === null) {
+      return new Error(
+        'A null error was thrown, ' +
+          'see here for more info: https://nextjs.org/docs/messages/threw-undefined'
+      )
+    }
+  }
+
+  return new Error(isPlainObject(err) ? JSON.stringify(err) : err + '')
+}
diff --git a/packages/next/lib/is-plain-object.ts b/packages/next/lib/is-plain-object.ts
new file mode 100644
index 0000000000000..1ab0d87e73919
--- /dev/null
+++ b/packages/next/lib/is-plain-object.ts
@@ -0,0 +1,12 @@
+export function getObjectClassLabel(value: any): string {
+  return Object.prototype.toString.call(value)
+}
+
+export function isPlainObject(value: any): boolean {
+  if (getObjectClassLabel(value) !== '[object Object]') {
+    return false
+  }
+
+  const prototype = Object.getPrototypeOf(value)
+  return prototype === null || prototype === Object.prototype
+}
diff --git a/packages/next/lib/is-serializable-props.ts b/packages/next/lib/is-serializable-props.ts
index 058bfcd386450..1202d4d98882d 100644
--- a/packages/next/lib/is-serializable-props.ts
+++ b/packages/next/lib/is-serializable-props.ts
@@ -1,17 +1,6 @@
-const regexpPlainIdentifier = /^[A-Za-z_$][A-Za-z0-9_$]*$/
-
-function getObjectClassLabel(value: any): string {
-  return Object.prototype.toString.call(value)
-}
+import { isPlainObject, getObjectClassLabel } from './is-plain-object'
 
-function isPlainObject(value: any): boolean {
-  if (getObjectClassLabel(value) !== '[object Object]') {
-    return false
-  }
-
-  const prototype = Object.getPrototypeOf(value)
-  return prototype === null || prototype === Object.prototype
-}
+const regexpPlainIdentifier = /^[A-Za-z_$][A-Za-z0-9_$]*$/
 
 export function isSerializableProps(
   page: string,
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
index e0feebf06e5b5..655826c9a0141 100644
--- a/packages/next/server/base-server.ts
+++ b/packages/next/server/base-server.ts
@@ -89,7 +89,7 @@ import { getUtils } from '../build/webpack/loaders/next-serverless-loader/utils'
 import { PreviewData } from 'next/types'
 import ResponseCache from './response-cache'
 import { parseNextUrl } from '../shared/lib/router/utils/parse-next-url'
-import isError from '../lib/is-error'
+import isError, { getProperError } from '../lib/is-error'
 import { getMiddlewareInfo } from './require'
 import { MIDDLEWARE_ROUTE } from '../lib/constants'
 import { run } from './web/sandbox'
@@ -567,7 +567,7 @@ export default abstract class Server {
       if (this.minimalMode || this.renderOpts.dev) {
         throw err
       }
-      this.logError(isError(err) ? err : new Error(err + ''))
+      this.logError(getProperError(err))
       res.statusCode = 500
       res.end('Internal Server Error')
     }
@@ -1225,7 +1225,7 @@ export default abstract class Server {
               return { finished: true }
             }
 
-            const error = isError(err) ? err : new Error(err + '')
+            const error = getProperError(err)
             console.error(error)
             res.statusCode = 500
             this.renderError(error, req, res, parsed.pathname || '')
@@ -2292,7 +2292,7 @@ export default abstract class Server {
         }
       }
     } catch (error) {
-      const err = isError(error) ? error : error ? new Error(error + '') : null
+      const err = getProperError(error)
       if (err instanceof NoFallbackError && bubbleNoFallback) {
         throw err
       }
@@ -2313,7 +2313,7 @@ export default abstract class Server {
           if (isError(err)) err.page = page
           throw err
         }
-        this.logError(err || new Error(error + ''))
+        this.logError(getProperError(err))
       }
       return response
     }
@@ -2370,16 +2370,9 @@ export default abstract class Server {
 
   private async renderErrorToResponse(
     ctx: RequestContext,
-    _err: Error | null
+    err: Error | null
   ): Promise<ResponsePayload | null> {
     const { res, query } = ctx
-    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'
-      )
-    }
     try {
       let result: null | FindComponentsResult = null
 
@@ -2430,14 +2423,10 @@ export default abstract class Server {
         throw maybeFallbackError
       }
     } catch (error) {
-      const renderToHtmlError = isError(error)
-        ? error
-        : error
-        ? new Error(error + '')
-        : null
+      const renderToHtmlError = getProperError(error)
       const isWrappedError = renderToHtmlError instanceof WrappedBuildError
       if (!isWrappedError) {
-        this.logError(renderToHtmlError || new Error(error + ''))
+        this.logError(renderToHtmlError)
       }
       res.statusCode = 500
       const fallbackComponents = await this.getFallbackErrorComponents()
diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts
index 3f3ba14a94c4d..ec0d144b88614 100644
--- a/packages/next/server/dev/hot-reloader.ts
+++ b/packages/next/server/dev/hot-reloader.ts
@@ -39,7 +39,7 @@ import { NextConfigComplete } from '../config-shared'
 import { CustomRoutes } from '../../lib/load-custom-routes'
 import { DecodeError } from '../../shared/lib/utils'
 import { Span, trace } from '../../trace'
-import isError from '../../lib/is-error'
+import { getProperError } from '../../lib/is-error'
 import ws from 'next/dist/compiled/ws'
 
 const wsServer = new ws.Server({ noServer: true })
@@ -249,10 +249,7 @@ export default class HotReloader {
         try {
           await this.ensurePage(page, true)
         } catch (error) {
-          await renderScriptError(
-            pageBundleRes,
-            isError(error) ? error : new Error(error + '')
-          )
+          await renderScriptError(pageBundleRes, getProperError(error))
           return { finished: true }
         }
 
diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts
index 2b1f95c9517ef..2ab5117a021e1 100644
--- a/packages/next/server/dev/next-dev-server.ts
+++ b/packages/next/server/dev/next-dev-server.ts
@@ -56,7 +56,7 @@ import {
   parseStack,
 } from 'next/dist/compiled/@next/react-dev-overlay/middleware'
 import * as Log from '../../build/output/log'
-import isError from '../../lib/is-error'
+import isError, { getProperError } from '../../lib/is-error'
 import { getMiddlewareRegex } from '../../shared/lib/router/utils/get-middleware-regex'
 import { isCustomErrorPage, isReservedPage } from '../../build/utils'
 
@@ -538,7 +538,7 @@ export default class DevServer extends Server {
       return result
     } catch (error) {
       this.logErrorWithOriginalStack(error, undefined, 'client')
-      const err = isError(error) ? error : new Error(error + '')
+      const err = getProperError(error)
       ;(err as any).middleware = true
       const { request, response, parsedUrl } = params
       this.renderError(err, request, response, parsedUrl.pathname)
@@ -591,7 +591,7 @@ export default class DevServer extends Server {
       return await super.run(req, res, parsedUrl)
     } catch (error) {
       res.statusCode = 500
-      const err = isError(error) ? error : error ? new Error(error + '') : null
+      const err = getProperError(error)
       try {
         this.logErrorWithOriginalStack(err).catch(() => {})
         return await this.renderError(err, req, res, pathname!, {
diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
index 0245de4740487..71bcc42d0ba58 100644
--- a/packages/next/shared/lib/router/router.ts
+++ b/packages/next/shared/lib/router/router.ts
@@ -16,7 +16,7 @@ import {
   isAssetError,
   markAssetError,
 } from '../../../client/route-loader'
-import isError from '../../../lib/is-error'
+import isError, { getProperError } from '../../../lib/is-error'
 import { denormalizePagePath } from '../../../server/denormalize-page-path'
 import { normalizeLocalePath } from '../i18n/normalize-locale-path'
 import mitt from '../mitt'
@@ -1559,7 +1559,7 @@ export default class Router implements BaseRouter {
       return routeInfo
     } catch (err) {
       return this.handleRouteInfoError(
-        isError(err) ? err : new Error(err + ''),
+        getProperError(err),
         pathname,
         query,
         as,
diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts
index 44ce8d9c6acd5..f730ed3b6113d 100644
--- a/test/development/acceptance/ReactRefreshLogBox.test.ts
+++ b/test/development/acceptance/ReactRefreshLogBox.test.ts
@@ -903,4 +903,113 @@ describe('ReactRefreshLogBox', () => {
 
     await cleanup()
   })
+
+  test('non-Error errors are handled properly', async () => {
+    const { session, cleanup } = await sandbox(next)
+
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          throw {'a': 1, 'b': 'x'};
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+
+    expect(await session.hasRedbox(true)).toBe(true)
+    expect(await session.getRedboxDescription()).toMatchInlineSnapshot(
+      `"Error: {\\"a\\":1,\\"b\\":\\"x\\"}"`
+    )
+
+    // fix previous error
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(false)).toBe(false)
+    await session.patch(
+      'index.js',
+      `
+        class Hello {}
+        
+        export default () => {
+          throw Hello
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(true)).toBe(true)
+    expect(await session.getRedboxDescription()).toContain(
+      `Error: class Hello {`
+    )
+
+    // fix previous error
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(false)).toBe(false)
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          throw "string error"
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(true)).toBe(true)
+    expect(await session.getRedboxDescription()).toMatchInlineSnapshot(
+      `"Error: string error"`
+    )
+
+    // fix previous error
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(false)).toBe(false)
+    await session.patch(
+      'index.js',
+      `
+        export default () => {
+          throw null
+          return (
+            <div>hello</div>
+          )
+        }
+      `
+    )
+    expect(await session.hasRedbox(true)).toBe(true)
+    expect(await session.getRedboxDescription()).toContain(
+      `Error: A null error was thrown`
+    )
+
+    await cleanup()
+  })
 })
diff --git a/test/integration/client-navigation/test/rendering.js b/test/integration/client-navigation/test/rendering.js
index b991454c2fa09..6e1055e4fde46 100644
--- a/test/integration/client-navigation/test/rendering.js
+++ b/test/integration/client-navigation/test/rendering.js
@@ -425,7 +425,7 @@ export default function (render, fetch, ctx) {
       const text = await getRedboxHeader(browser)
 
       expect(text).toContain(
-        'An undefined error was thrown sometime during render...'
+        'An undefined error was thrown, see here for more info:'
       )
     })
   })