[dev-overlay] Stop appending wrong Owner Stacks to SSR-only shell errors (#77302)

Author: eps1lon

packages/next/src/client/components/errors/stitched-error.ts

@@ -18,17 +18,13 @@ export function setOwnerStack(error: Error, stack: string | null) {
   ownerStacks.set(error, stack)
 }
 
-export function getReactStitchedError(err: unknown): Error {
-  const newError = isError(err)
-    ? err
-    : // TODO: stringify thrown value
-      new Error('Thrown value was ignored. This is a bug in Next.js.')
+export function coerceError(value: unknown): Error {
+  return isError(value) ? value : new Error('' + value)
+}
 
+export function setOwnerStackIfAvailable(error: Error): void {
   // React 18 and prod does not have `captureOwnerStack`
   if ('captureOwnerStack' in React) {
-    // TODO: Hoist these to callsites to ensure we set the correct Owner Stack.
-    setOwnerStack(newError, React.captureOwnerStack())
+    setOwnerStack(error, React.captureOwnerStack())
   }
-
-  return newError
 }

packages/next/src/client/components/errors/use-error-handler.ts

@@ -3,7 +3,7 @@ import { isNextRouterError } from '../is-next-router-error'
 import { formatConsoleArgs, parseConsoleArgs } from '../../lib/console'
 import isError from '../../../lib/is-error'
 import { createConsoleError } from './console-error'
-import { getReactStitchedError } from '../errors/stitched-error'
+import { coerceError, setOwnerStackIfAvailable } from '../errors/stitched-error'
 
 const queueMicroTask =
   globalThis.queueMicrotask || ((cb: () => void) => Promise.resolve().then(cb))
@@ -29,7 +29,7 @@ export function handleConsoleError(
       environmentName
     )
   }
-  error = getReactStitchedError(error)
+  setOwnerStackIfAvailable(error)
 
   errorQueue.push(error)
   for (const handler of errorHandlers) {
@@ -41,17 +41,7 @@ export function handleConsoleError(
   }
 }
 
-export function handleClientError(originError: unknown) {
-  let error: Error
-  if (isError(originError)) {
-    error = originError
-  } else {
-    // If it's not an error, format the args into an error
-    const formattedErrorMessage = originError + ''
-    error = new Error(formattedErrorMessage)
-  }
-  error = getReactStitchedError(error)
-
+export function handleClientError(error: Error) {
   errorQueue.push(error)
   for (const handler of errorHandlers) {
     // Delayed the error being passed to React Dev Overlay,
@@ -91,28 +81,30 @@ export function useErrorHandler(
 }
 
 function onUnhandledError(event: WindowEventMap['error']): void | boolean {
-  if (isNextRouterError(event.error)) {
+  const thrownValue: unknown = event.error
+  if (isNextRouterError(thrownValue)) {
     event.preventDefault()
     return false
   }
   // When there's an error property present, we log the error to error overlay.
   // Otherwise we don't do anything as it's not logging in the console either.
-  if (event.error) {
-    handleClientError(event.error)
+  if (thrownValue) {
+    const error = coerceError(thrownValue)
+    setOwnerStackIfAvailable(error)
+
+    handleClientError(error)
   }
 }
 
 function onUnhandledRejection(ev: WindowEventMap['unhandledrejection']): void {
-  const reason = ev?.reason
+  const reason: unknown = ev?.reason
   if (isNextRouterError(reason)) {
     ev.preventDefault()
     return
   }
 
-  let error = reason
-  if (error && !isError(error)) {
-    error = new Error(error + '')
-  }
+  const error = coerceError(reason)
+  setOwnerStackIfAvailable(error)
 
   rejectionQueue.push(error)
   for (const handler of rejectionHandlers) {

packages/next/src/client/components/react-dev-overlay/app/app-dev-overlay.tsx

@@ -52,7 +52,7 @@ function ReplaySsrOnlyErrors({
     // eslint-disable-next-line react-hooks/rules-of-hooks
     useEffect(() => {
       if (ssrError !== null) {
-        // TODO(veil): Produces wrong Owner Stack
+        // TODO(veil): Include original Owner Stack (NDX-905)
         // TODO(veil): Mark as recoverable error
         // TODO(veil): console.error
         handleClientError(ssrError)

packages/next/src/client/react-client-callbacks/error-boundary-callbacks.ts

@@ -2,8 +2,9 @@
 
 import type { ErrorInfo } from 'react'
 import {
-  getReactStitchedError,
+  setOwnerStackIfAvailable,
   setComponentStack,
+  coerceError,
 } from '../components/errors/stitched-error'
 import { handleClientError } from '../components/errors/use-error-handler'
 import { isNextRouterError } from '../components/is-next-router-error'
@@ -16,7 +17,7 @@ import {
 } from '../components/error-boundary'
 
 export function onCaughtError(
-  err: unknown,
+  thrownValue: unknown,
   errorInfo: ErrorInfo & { errorBoundary?: React.Component }
 ) {
   const errorBoundaryComponent = errorInfo.errorBoundary?.constructor
@@ -41,11 +42,11 @@ export function onCaughtError(
     // We don't consider errors caught unless they're caught by an explicit error
     // boundary. The built-in ones are considered implicit.
     // This mimics how the same app would behave without Next.js.
-    return onUncaughtError(err, errorInfo)
+    return onUncaughtError(thrownValue, errorInfo)
   }
 
   // Skip certain custom errors which are not expected to be reported on client
-  if (isBailoutToCSRError(err) || isNextRouterError(err)) return
+  if (isBailoutToCSRError(thrownValue) || isNextRouterError(thrownValue)) return
 
   if (process.env.NODE_ENV !== 'production') {
     const errorBoundaryName =
@@ -72,37 +73,43 @@ export function onCaughtError(
 
     const errorLocation = `${componentErrorMessage} ${errorBoundaryMessage}`
 
-    const stitchedError = getReactStitchedError(err)
+    const error = coerceError(thrownValue)
+    setOwnerStackIfAvailable(error)
     // TODO: change to passing down errorInfo later
     // In development mode, pass along the component stack to the error
     if (errorInfo.componentStack) {
-      setComponentStack(stitchedError, errorInfo.componentStack)
+      setComponentStack(error, errorInfo.componentStack)
     }
 
     // Log and report the error with location but without modifying the error stack
-    originConsoleError('%o\n\n%s', err, errorLocation)
+    originConsoleError('%o\n\n%s', thrownValue, errorLocation)
 
-    handleClientError(stitchedError)
+    handleClientError(error)
   } else {
-    originConsoleError(err)
+    originConsoleError(thrownValue)
   }
 }
 
-export function onUncaughtError(err: unknown, errorInfo: React.ErrorInfo) {
+export function onUncaughtError(
+  thrownValue: unknown,
+  errorInfo: React.ErrorInfo
+) {
   // Skip certain custom errors which are not expected to be reported on client
-  if (isBailoutToCSRError(err) || isNextRouterError(err)) return
+  if (isBailoutToCSRError(thrownValue) || isNextRouterError(thrownValue)) return
 
   if (process.env.NODE_ENV !== 'production') {
-    const stitchedError = getReactStitchedError(err)
+    const error = coerceError(thrownValue)
+    setOwnerStackIfAvailable(error)
+
     // TODO: change to passing down errorInfo later
     // In development mode, pass along the component stack to the error
     if (errorInfo.componentStack) {
-      setComponentStack(stitchedError, errorInfo.componentStack)
+      setComponentStack(error, errorInfo.componentStack)
     }
 
     // TODO: Add an adendum to the overlay telling people about custom error boundaries.
-    reportGlobalError(stitchedError)
+    reportGlobalError(error)
   } else {
-    reportGlobalError(err)
+    reportGlobalError(thrownValue)
   }
 }

packages/next/src/client/react-client-callbacks/on-recoverable-error.ts

@@ -2,25 +2,29 @@
 
 import type { HydrationOptions } from 'react-dom/client'
 import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'
-import { reportGlobalError } from './report-global-error'
 import {
-  getReactStitchedError,
+  setOwnerStackIfAvailable,
   setComponentStack,
+  coerceError,
 } from '../components/errors/stitched-error'
 import isError from '../../lib/is-error'
+import { reportGlobalError } from './report-global-error'
 
 export const onRecoverableError: HydrationOptions['onRecoverableError'] = (
   error,
   errorInfo
 ) => {
   // x-ref: https://github.com/facebook/react/pull/28736
   const cause = isError(error) && 'cause' in error ? error.cause : error
-  const stitchedError = getReactStitchedError(cause)
-  if (process.env.NODE_ENV === 'development' && errorInfo.componentStack) {
-    setComponentStack(stitchedError, errorInfo.componentStack)
-  }
   // Skip certain custom errors which are not expected to be reported on client
   if (isBailoutToCSRError(cause)) return
 
-  reportGlobalError(stitchedError)
+  const causeError = coerceError(cause)
+  setOwnerStackIfAvailable(causeError)
+
+  if (process.env.NODE_ENV === 'development' && errorInfo.componentStack) {
+    setComponentStack(causeError, errorInfo.componentStack)
+  }
+
+  reportGlobalError(causeError)
 }

test/development/app-dir/ssr-only-error/ssr-only-error.test.ts

@@ -9,7 +9,7 @@ describe('ssr-only-error', () => {
   it('should show ssr only error in error overlay', async () => {
     const browser = await next.browser('/')
 
-    // TODO(veil): Missing Owner Stack
+    // TODO(veil): Missing Owner Stack (NDX-905)
     await expect(browser).toDisplayCollapsedRedbox(`
      {
        "description": "Error: SSR only error",