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 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
21 changes: 21 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,22 @@ 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
}
}

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
81 changes: 81 additions & 0 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Expand Up @@ -903,4 +903,85 @@ 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"`
)

await cleanup()
})
})
2 changes: 1 addition & 1 deletion test/integration/client-navigation/test/rendering.js
Expand Up @@ -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:'
)
})
})
Expand Down