Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add util for normalizing errors #33159

Merged
merged 7 commits into from
Jan 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions errors/threw-undefined.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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') {
ijjk marked this conversation as resolved.
Show resolved Hide resolved
return new Error(
'An undefined error was thrown, ' +
'see here for more info: https://nextjs.org/docs/messages/threw-undefined'
)
ijjk marked this conversation as resolved.
Show resolved Hide resolved
}

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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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()
})
})
Loading