Skip to content

Commit

Permalink
remove unimplemented onError errorInfo argument (#70531)
Browse files Browse the repository at this point in the history
RSC has never supported the `errorInfo` argument to `onError` unlike
SSR. This PR updates our usage of onError in RSC contexts to clarify
that this value does not exist

This also updates the next interned types for react-dom/server to
reflect that `renderTo...` and `prerender` support the `ErrorInfo`
second arg and `resume` does not.
  • Loading branch information
gnoff authored Sep 26, 2024
1 parent 3a832f1 commit e6b1232
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 29 deletions.
8 changes: 4 additions & 4 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type { DeepReadonly } from '../../shared/lib/deep-readonly'
import type { BaseNextRequest, BaseNextResponse } from '../base-http'
import type { IncomingHttpHeaders } from 'http'

import React, { type JSX } from 'react'
import React, { type ErrorInfo, type JSX } from 'react'

import RenderResult, {
type AppPageRenderResultMetadata,
Expand Down Expand Up @@ -1959,13 +1959,13 @@ async function prerenderToStream(
dynamicTracking,
}
let SSRIsDynamic = false
function SSROnError(err: unknown) {
function SSROnError(err: unknown, errorInfo?: ErrorInfo) {
if (err === abortReason || isPrerenderInterruptedError(err)) {
SSRIsDynamic = true
return
}

return htmlRendererErrorHandler(err)
return htmlRendererErrorHandler(err, errorInfo)
}

function SSROnPostpone(reason: string) {
Expand Down Expand Up @@ -2256,7 +2256,7 @@ async function prerenderToStream(
const err = new DynamicServerError(
`Route ${staticGenerationStore.route} couldn't be rendered statically because it used IO that was not cached in a Server Component. See more info here: https://nextjs.org/docs/messages/dynamic-io`
)
serverComponentsErrorHandler(err, {})
serverComponentsErrorHandler(err)
throw err
}

Expand Down
39 changes: 18 additions & 21 deletions packages/next/src/server/app-render/create-error-handler.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { ErrorInfo } from 'react'

import stringHash from 'next/dist/compiled/string-hash'
import { formatServerError } from '../../lib/format-server-error'
import { SpanStatusCode, getTracer } from '../lib/trace/tracer'
Expand All @@ -10,27 +12,24 @@ declare global {
var __next_log_error__: undefined | ((err: unknown) => void)
}

type ErrorHandler = (err: unknown, errorInfo?: unknown) => string | undefined
type RSCErrorHandler = (err: unknown) => string | undefined
type SSRErrorHandler = (
err: unknown,
errorInfo?: ErrorInfo
) => string | undefined

export type DigestedError = Error & { digest: string }

export function createFlightReactServerErrorHandler(
dev: boolean,
onReactServerRenderError: (err: any) => void
): ErrorHandler {
return (err: any, errorInfo?: unknown) => {
): RSCErrorHandler {
return (err: any) => {
// If the error already has a digest, respect the original digest,
// so it won't get re-generated into another new error.
if (!err.digest) {
// TODO-APP: look at using webcrypto instead. Requires a promise to be awaited.
err.digest = stringHash(
err.message +
(typeof errorInfo === 'object' &&
errorInfo !== null &&
'stack' in errorInfo
? errorInfo.stack
: err.stack || '')
).toString()
err.digest = stringHash(err.message + err.stack || '').toString()
}

// If the response was closed, we don't need to log the error.
Expand Down Expand Up @@ -75,15 +74,13 @@ export function createHTMLReactServerErrorHandler(
reactServerErrors: Map<string, DigestedError>,
silenceLogger: boolean,
onReactServerRenderError: undefined | ((err: any) => void)
): ErrorHandler {
return (err: any, errorInfo: any) => {
): RSCErrorHandler {
return (err: any) => {
// If the error already has a digest, respect the original digest,
// so it won't get re-generated into another new error.
if (!err.digest) {
// TODO-APP: look at using webcrypto instead. Requires a promise to be awaited.
err.digest = stringHash(
err.message + (errorInfo?.stack || err.stack || '')
).toString()
err.digest = stringHash(err.message + (err.stack || '')).toString()
}

// If the response was closed, we don't need to log the error.
Expand Down Expand Up @@ -146,9 +143,9 @@ export function createHTMLErrorHandler(
reactServerErrors: Map<string, DigestedError>,
allCapturedErrors: Array<unknown>,
silenceLogger: boolean,
onHTMLRenderSSRError: (err: any) => void
): ErrorHandler {
return (err: any, errorInfo: any) => {
onHTMLRenderSSRError: (err: any, errorInfo?: ErrorInfo) => void
): SSRErrorHandler {
return (err: any, errorInfo?: ErrorInfo) => {
let isSSRError = true

// If the error already has a digest, respect the original digest,
Expand All @@ -165,7 +162,7 @@ export function createHTMLErrorHandler(
}
} else {
err.digest = stringHash(
err.message + (errorInfo?.stack || err.stack || '')
err.message + (errorInfo?.componentStack || err.stack || '')
).toString()
}

Expand Down Expand Up @@ -215,7 +212,7 @@ export function createHTMLErrorHandler(
// HTML errors contain RSC errors as well, filter them out before reporting
isSSRError
) {
onHTMLRenderSSRError(err)
onHTMLRenderSSRError(err, errorInfo)
}
}

Expand Down
8 changes: 4 additions & 4 deletions packages/next/types/react-dom.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
declare module 'react-dom/server.edge' {
import type { JSX } from 'react'
import type { ErrorInfo, JSX } from 'react'
/**
* https://github.com/facebook/react/blob/aec521a96d3f1bebc2ba38553d14f4989c6e88e0/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js#L329-L333
*/
Expand Down Expand Up @@ -42,7 +42,7 @@ declare module 'react-dom/server.edge' {
bootstrapModules?: Array<string | BootstrapScriptDescriptor>
progressiveChunkSize?: number
signal?: AbortSignal
onError?: (error: unknown) => string | undefined
onError?: (error: unknown, errorInfo: ErrorInfo) => string | undefined
onPostpone?: (reason: string) => void
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor
importMap?: {
Expand Down Expand Up @@ -71,7 +71,7 @@ declare module 'react-dom/server.edge' {
}

declare module 'react-dom/static.edge' {
import type { JSX } from 'react'
import type { ErrorInfo, JSX } from 'react'
/**
* https://github.com/facebook/react/blob/aec521a96d3f1bebc2ba38553d14f4989c6e88e0/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js#L329-L333
*/
Expand All @@ -94,7 +94,7 @@ declare module 'react-dom/static.edge' {
bootstrapModules?: Array<string | BootstrapScriptDescriptor>
progressiveChunkSize?: number
signal?: AbortSignal
onError?: (error: unknown) => string | undefined
onError?: (error: unknown, errorInfo: ErrorInfo) => string | undefined
onPostpone?: (reason: string) => void
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor
importMap?: {
Expand Down

0 comments on commit e6b1232

Please sign in to comment.