Skip to content

Commit

Permalink
Add util for normalizing errors (#33159)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
ijjk and mozeryansky committed Jan 11, 2022
1 parent 2d0fd34 commit 57a8705
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 51 deletions.
18 changes: 15 additions & 3 deletions 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.
6 changes: 3 additions & 3 deletions packages/next/client/index.tsx
Expand Up @@ -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'

Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/next/lib/eslint/runLintCheck.ts
Expand Up @@ -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[]
Expand Down Expand Up @@ -221,7 +221,7 @@ async function lint(
)
return null
} else {
throw new Error(err + '')
throw getProperError(err)
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions 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
Expand All @@ -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 + '')
}
12 changes: 12 additions & 0 deletions 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
}
15 changes: 2 additions & 13 deletions 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,
Expand Down
27 changes: 8 additions & 19 deletions packages/next/server/base-server.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -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')
}
Expand Down Expand Up @@ -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 || '')
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down
7 changes: 2 additions & 5 deletions packages/next/server/dev/hot-reloader.ts
Expand Up @@ -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 })
Expand Down Expand Up @@ -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 }
}

Expand Down
6 changes: 3 additions & 3 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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!, {
Expand Down
4 changes: 2 additions & 2 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down
109 changes: 109 additions & 0 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Expand Up @@ -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()
})
})

0 comments on commit 57a8705

Please sign in to comment.