fix: handling of falsey values in error boundaries (#93134)

our error boundaries had a bunch of logic that set `state.error = error`
and then checked `if (state.error)`, which only works correctly if
thrown value is truthy. it breaks if something does e.g. `throw
undefined`. in this case, we would incorrectly think that no error
occurred and render children again (instead of a fallback), which can
then lead to an infinite loop if the children throw again.

the fix is to wrap the thrown value, so `state.error` is either `null`
(initial/reset) or `{ thrownValue: ... }` if something errored. i
initially considered using a separate `state.hasError` boolean, but
that's a bit annoying to type, and really we want to model this as a
discriminated union, so using a pseudo-Optional thing is nicer.

Author: lubieowoce

packages/next/src/client/components/catch-error.tsx

@@ -30,15 +30,15 @@ type CatchErrorProps<P extends UserProps> = {
 }
 
 type CatchErrorState = {
-  error: Error | null
+  error: null | { thrownValue: unknown }
   previousPathname: string | null
 }
 
 // This is forked from error-boundary.
 // TODO: Extend it instead of forking to easily sync the behavior?
 class CatchError<P extends UserProps> extends React.Component<
   CatchErrorProps<P>,
-  { error: Error | null; previousPathname: string | null }
+  CatchErrorState
 > {
   declare context: AppRouterInstance | null
   static contextType = AppRouterContext
@@ -54,14 +54,16 @@ class CatchError<P extends UserProps> extends React.Component<
     }
   }
 
-  static getDerivedStateFromError(error: Error) {
-    if (isNextRouterError(error)) {
+  static getDerivedStateFromError(
+    thrownValue: unknown
+  ): Partial<CatchErrorState> {
+    if (isNextRouterError(thrownValue)) {
       // Re-throw if an expected internal Next.js router error occurs
       // this means it should be handled by a different boundary (such as a NotFound boundary in a parent segment)
-      throw error
+      throw thrownValue
     }
 
-    return { error }
+    return { error: { thrownValue } }
   }
 
   static getDerivedStateFromProps(
@@ -75,7 +77,7 @@ class CatchError<P extends UserProps> extends React.Component<
     // the error boundary and instead should fallback
     // to a hard navigation to attempt recovering
     if (process.env.__NEXT_APP_NAV_FAIL_HANDLING) {
-      if (error && handleHardNavError(error)) {
+      if (error && handleHardNavError(error.thrownValue)) {
         // clear error so we don't render anything
         return {
           error: null,
@@ -124,13 +126,15 @@ class CatchError<P extends UserProps> extends React.Component<
     //When it's bot request, segment level error boundary will keep rendering the children,
     // the final error will be caught by the root error boundary and determine wether need to apply graceful degrade.
     if (this.state.error && !isBotUserAgent) {
-      handleISRError({ error: this.state.error })
+      const thrownValue = this.state.error.thrownValue
+      handleISRError({ error: thrownValue })
 
       return (
         <this.props.fallback
           props={this.props.props}
           errorInfo={{
-            error: this.state.error,
+            // TODO(NAR-804): Docs say this is an Error object, but we don't guarantee that
+            error: thrownValue,
             reset: this.reset,
             unstable_retry: this.unstable_retry,
           }}

packages/next/src/client/components/error-boundary.tsx

@@ -15,7 +15,7 @@ const isBotUserAgent =
   typeof window !== 'undefined' && isBot(window.navigator.userAgent)
 
 export type ErrorInfo = {
-  error: Error
+  error: unknown
   reset: () => void
   unstable_retry: () => void
 }
@@ -35,7 +35,7 @@ interface ErrorBoundaryHandlerProps extends ErrorBoundaryProps {
 }
 
 interface ErrorBoundaryHandlerState {
-  error: Error | null
+  error: null | { thrownValue: unknown }
   previousPathname: string | null
 }
 
@@ -54,14 +54,16 @@ export class ErrorBoundaryHandler extends React.Component<
     }
   }
 
-  static getDerivedStateFromError(error: Error) {
-    if (isNextRouterError(error)) {
+  static getDerivedStateFromError(
+    thrownValue: unknown
+  ): Partial<ErrorBoundaryHandlerState> {
+    if (isNextRouterError(thrownValue)) {
       // Re-throw if an expected internal Next.js router error occurs
       // this means it should be handled by a different boundary (such as a NotFound boundary in a parent segment)
-      throw error
+      throw thrownValue
     }
 
-    return { error }
+    return { error: { thrownValue } }
   }
 
   static getDerivedStateFromProps(
@@ -75,7 +77,7 @@ export class ErrorBoundaryHandler extends React.Component<
     // the error boundary and instead should fallback
     // to a hard navigation to attempt recovering
     if (process.env.__NEXT_APP_NAV_FAIL_HANDLING) {
-      if (error && handleHardNavError(error)) {
+      if (error && handleHardNavError(error.thrownValue)) {
         // clear error so we don't render anything
         return {
           error: null,
@@ -118,14 +120,16 @@ export class ErrorBoundaryHandler extends React.Component<
     //When it's bot request, segment level error boundary will keep rendering the children,
     // the final error will be caught by the root error boundary and determine wether need to apply graceful degrade.
     if (this.state.error && !isBotUserAgent) {
-      handleISRError({ error: this.state.error })
+      const thrownValue = this.state.error.thrownValue
+      handleISRError({ error: thrownValue })
 
       return (
         <>
           {this.props.errorStyles}
           {this.props.errorScripts}
           <this.props.errorComponent
-            error={this.state.error}
+            // TODO(NAR-804): Docs say this is an Error object, but we don't guarantee that
+            error={thrownValue}
             reset={this.reset}
             unstable_retry={this.unstable_retry}
           />

packages/next/src/client/components/http-access-fallback/error-boundary.tsx

@@ -77,7 +77,7 @@ class HTTPAccessFallbackErrorBoundary extends React.Component<
     }
   }
 
-  static getDerivedStateFromError(error: any) {
+  static getDerivedStateFromError(error: unknown) {
     if (isHTTPAccessFallbackError(error)) {
       const httpStatus = getAccessFallbackHTTPStatus(error)
       return {

packages/next/src/client/components/nav-failure-handler.ts

@@ -3,7 +3,6 @@ import { createHrefFromUrl } from './router-reducer/create-href-from-url'
 
 export function handleHardNavError(error: unknown): boolean {
   if (
-    error &&
     typeof window !== 'undefined' &&
     window.next.__pendingUrl &&
     createHrefFromUrl(new URL(window.location.href)) !==

packages/next/src/client/components/redirect-boundary.tsx

@@ -44,7 +44,7 @@ export class RedirectErrorBoundary extends React.Component<
     this.state = { redirect: null, redirectType: null }
   }
 
-  static getDerivedStateFromError(error: any) {
+  static getDerivedStateFromError(error: unknown) {
     if (isRedirectError(error)) {
       const url = getURLFromRedirectError(error)
       const redirectType = getRedirectTypeFromError(error)

packages/next/src/next-devtools/userspace/app/app-dev-overlay-error-boundary.tsx

@@ -9,40 +9,33 @@ import {
   AppRouterContext,
   type AppRouterInstance,
 } from '../../../shared/lib/app-router-context.shared-runtime'
+import isError from '../../../lib/is-error'
 
 type AppDevOverlayErrorBoundaryProps = {
   children: React.ReactNode
   globalError: GlobalErrorState
 }
 
 type AppDevOverlayErrorBoundaryState = {
-  reactError: unknown
+  error: null | { thrownValue: unknown }
 }
 
 function ErroredHtml({
   globalError: [GlobalError, globalErrorStyles],
-  error,
+  thrownValue,
   reset,
   unstable_retry,
 }: {
   globalError: GlobalErrorState
-  error: unknown
+  thrownValue: unknown
   reset: () => void
   unstable_retry: () => void
 }) {
-  if (!error) {
-    return (
-      <html>
-        <head />
-        <body />
-      </html>
-    )
-  }
   return (
     <ErrorBoundary errorComponent={DefaultGlobalError}>
       {globalErrorStyles}
       <GlobalError
-        error={error}
+        error={thrownValue}
         reset={reset}
         unstable_retry={unstable_retry}
       />
@@ -58,20 +51,23 @@ export class AppDevOverlayErrorBoundary extends PureComponent<
   declare context: AppRouterInstance | null
 
   state: AppDevOverlayErrorBoundaryState = {
-    reactError: null,
+    error: null,
   }
 
-  static getDerivedStateFromError(error: Error) {
+  static getDerivedStateFromError(
+    thrownValue: Error
+  ): Partial<AppDevOverlayErrorBoundaryState> {
     RuntimeErrorHandler.hadRuntimeError = true
 
     return {
-      reactError: error,
+      error: { thrownValue },
     }
   }
 
-  componentDidCatch(err: Error) {
+  componentDidCatch(err: unknown) {
     if (
       process.env.NODE_ENV === 'development' &&
+      isError(err) &&
       err.message === SEGMENT_EXPLORER_SIMULATED_ERROR_MESSAGE
     ) {
       return
@@ -87,22 +83,25 @@ export class AppDevOverlayErrorBoundary extends PureComponent<
   }
 
   reset = () => {
-    this.setState({ reactError: null })
+    this.setState({ error: null })
   }
 
   render() {
     const { children, globalError } = this.props
-    const { reactError } = this.state
+    const { error } = this.state
 
-    const fallback = (
-      <ErroredHtml
-        globalError={globalError}
-        error={reactError}
-        reset={this.reset}
-        unstable_retry={this.unstable_retry}
-      />
-    )
+    if (error !== null) {
+      const thrownValue = error.thrownValue
+      return (
+        <ErroredHtml
+          globalError={globalError}
+          thrownValue={thrownValue}
+          reset={this.reset}
+          unstable_retry={this.unstable_retry}
+        />
+      )
+    }
 
-    return reactError !== null ? fallback : children
+    return children
   }
 }

packages/next/src/next-devtools/userspace/pages/pages-dev-overlay-error-boundary.tsx

@@ -3,21 +3,25 @@ import React from 'react'
 type PagesDevOverlayErrorBoundaryProps = {
   children?: React.ReactNode
 }
-type PagesDevOverlayErrorBoundaryState = { error: Error | null }
+type PagesDevOverlayErrorBoundaryState = {
+  hasError: boolean
+}
 
 export class PagesDevOverlayErrorBoundary extends React.PureComponent<
   PagesDevOverlayErrorBoundaryProps,
   PagesDevOverlayErrorBoundaryState
 > {
-  state = { error: null }
+  state = { hasError: false }
 
-  static getDerivedStateFromError(error: Error) {
-    return { error }
+  static getDerivedStateFromError(
+    _: unknown
+  ): Partial<PagesDevOverlayErrorBoundaryState> {
+    return { hasError: true }
   }
 
   // Explicit type is needed to avoid the generated `.d.ts` having a wide return type that could be specific to the `@types/react` version.
   render(): React.ReactNode {
     // The component has to be unmounted or else it would continue to error
-    return this.state.error ? null : this.props.children
+    return this.state.hasError ? null : this.props.children
   }
 }

test/e2e/app-dir/catch-error/app/client-component/catch-error-wrapper.tsx

@@ -9,7 +9,7 @@ export function ErrorFallback(
 ) {
   return (
     <>
-      <p id="error-boundary-message">{error.message}</p>
+      <p id="error-boundary-message">{(error as Error).message}</p>
       <p id="error-boundary-title">{props.title}</p>
       <button id="reset" onClick={() => reset()}>
         Reset

test/e2e/app-dir/catch-error/app/client-component/throw-null/page.tsx

@@ -0,0 +1,37 @@
+'use client'
+
+import { useState } from 'react'
+import type { ErrorInfo } from 'next/error'
+import { unstable_catchError } from 'next/error'
+
+function Inner() {
+  const [clicked, setClicked] = useState(false)
+  if (clicked) {
+    // eslint-disable-next-line no-throw-literal -- testing bad values on purpose
+    throw null
+  }
+  return (
+    <button
+      id="error-trigger-button"
+      onClick={() => {
+        setClicked(true)
+      }}
+    >
+      Trigger Error!
+    </button>
+  )
+}
+
+function ErrorFallback(_props: {}, { error }: ErrorInfo) {
+  return <p id="error-boundary-message">{`An error occurred: ${error}`}</p>
+}
+
+const Wrapped = unstable_catchError(ErrorFallback)
+
+export default function Page() {
+  return (
+    <Wrapped>
+      <Inner />
+    </Wrapped>
+  )
+}

test/e2e/app-dir/catch-error/app/client-component/throw-undefined/page.tsx

@@ -0,0 +1,37 @@
+'use client'
+
+import { useState } from 'react'
+import type { ErrorInfo } from 'next/error'
+import { unstable_catchError } from 'next/error'
+
+function Inner() {
+  const [clicked, setClicked] = useState(false)
+  if (clicked) {
+    // eslint-disable-next-line no-throw-literal -- testing bad values on purpose
+    throw undefined
+  }
+  return (
+    <button
+      id="error-trigger-button"
+      onClick={() => {
+        setClicked(true)
+      }}
+    >
+      Trigger Error!
+    </button>
+  )
+}
+
+function ErrorFallback(_props: {}, { error }: ErrorInfo) {
+  return <p id="error-boundary-message">{`An error occurred: ${error}`}</p>
+}
+
+const Wrapped = unstable_catchError(ErrorFallback)
+
+export default function Page() {
+  return (
+    <Wrapped>
+      <Inner />
+    </Wrapped>
+  )
+}