Skip to content

Commit d22841c

Browse files
committed
Don't trigger error handlers from cDC
Handling them in cDC would attach the owner stack of the error boundary. That's not helpful for a specific error though. We need to make sure these error handlers are only called from React error handlers.
1 parent 61c22ec commit d22841c

File tree

6 files changed

+26
-53
lines changed

6 files changed

+26
-53
lines changed

packages/next/src/client/app-index.tsx

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ import { setAppBuildId } from './app-build-id'
2828
import { shouldRenderRootLevelErrorOverlay } from './lib/is-error-thrown-while-rendering-rsc'
2929
import { handleClientError } from './components/errors/use-error-handler'
3030
import { isNextRouterError } from './components/is-next-router-error'
31-
import OldAppDevErrorBoundary from './components/react-dev-overlay/app/old-react-dev-overlay'
32-
import { DevOverlayErrorBoundary as AppDevErrorBoundary } from './components/react-dev-overlay/_experimental/app/error-boundary'
33-
import { ErrorBoundaryHandler } from './components/error-boundary'
3431

3532
/// <reference types="react-dom/experimental" />
3633

@@ -249,18 +246,7 @@ function Root({ children }: React.PropsWithChildren<{}>) {
249246

250247
const reactRootOptions: ReactDOMClient.RootOptions = {
251248
onRecoverableError,
252-
onCaughtError: (error, errorInfo) => {
253-
const errorBoundaryComponent = errorInfo.errorBoundary?.constructor
254-
const isImplicitErrorBoundary =
255-
(process.env.NODE_ENV !== 'production' &&
256-
(errorBoundaryComponent === OldAppDevErrorBoundary ||
257-
errorBoundaryComponent === AppDevErrorBoundary)) ||
258-
errorBoundaryComponent === ErrorBoundaryHandler
259-
// Built-in error boundaries decide whether an error is caught or not.
260-
if (!isImplicitErrorBoundary) {
261-
onCaughtError(error, errorInfo)
262-
}
263-
},
249+
onCaughtError,
264250
onUncaughtError,
265251
}
266252

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@ import { useUntrackedPathname } from './navigation-untracked'
55
import { isNextRouterError } from './is-next-router-error'
66
import { handleHardNavError } from './nav-failure-handler'
77
import { workAsyncStorage } from '../../server/app-render/work-async-storage.external'
8-
import {
9-
onCaughtError,
10-
onUncaughtError,
11-
} from '../react-client-callbacks/error-boundary-callbacks'
128

139
const styles = {
1410
error: {
@@ -122,16 +118,6 @@ export class ErrorBoundaryHandler extends React.Component<
122118
}
123119
}
124120

125-
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
126-
// We don't consider errors caught unless they're caught by an explicit error
127-
// boundary. The built-in ones are considered implicit.
128-
if (this.props.errorComponent === GlobalError) {
129-
onUncaughtError(error, errorInfo)
130-
} else {
131-
onCaughtError(error, { ...errorInfo, errorBoundary: this })
132-
}
133-
}
134-
135121
reset = () => {
136122
this.setState({ error: null })
137123
}

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type { GlobalErrorComponent } from '../../../error-boundary'
22

33
import { PureComponent } from 'react'
44
import { RuntimeErrorHandler } from '../../../errors/runtime-error-handler'
5-
import { onUncaughtError } from '../../../../react-client-callbacks/error-boundary-callbacks'
65

76
type DevOverlayErrorBoundaryProps = {
87
children: React.ReactNode
@@ -58,12 +57,8 @@ export class DevOverlayErrorBoundary extends PureComponent<
5857
}
5958
}
6059

61-
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
60+
componentDidCatch() {
6261
this.props.onError(this.state.isReactError)
63-
64-
// We don't consider errors caught unless they're caught by an explicit error
65-
// boundary. The built-in ones are considered implicit.
66-
onUncaughtError(error, errorInfo)
6762
}
6863

6964
render() {

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { RootLayoutMissingTagsError } from '../internal/container/RootLayoutMiss
1111
import type { Dispatcher } from './hot-reloader-client'
1212
import { RuntimeErrorHandler } from '../../errors/runtime-error-handler'
1313
import type { GlobalErrorComponent } from '../../error-boundary'
14-
import { onUncaughtError } from '../../../react-client-callbacks/error-boundary-callbacks'
1514

1615
function ErroredHtml({
1716
globalError: [GlobalError, globalErrorStyles],
@@ -64,12 +63,6 @@ export default class ReactDevOverlay extends React.PureComponent<
6463
}
6564
}
6665

67-
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
68-
// We don't consider errors caught unless they're caught by an explicit error
69-
// boundary. The built-in ones are considered implicit.
70-
onUncaughtError(error, errorInfo)
71-
}
72-
7366
render() {
7467
const { state, children, dispatcher, globalError } = this.props
7568
const { isReactError, reactError } = this.state

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,37 @@ import { isNextRouterError } from '../components/is-next-router-error'
77
import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'
88
import { reportGlobalError } from './report-global-error'
99
import { originConsoleError } from '../components/globals/intercept-console-error'
10+
import OldAppDevErrorBoundary from '../components/react-dev-overlay/app/old-react-dev-overlay'
11+
import { DevOverlayErrorBoundary as AppDevErrorBoundary } from '../components/react-dev-overlay/_experimental/app/error-boundary'
12+
import {
13+
ErrorBoundaryHandler,
14+
GlobalError as DefaultErrorBoundary,
15+
} from '../components/error-boundary'
1016

1117
export function onCaughtError(
1218
err: unknown,
1319
errorInfo: ErrorInfo & { errorBoundary?: React.Component }
1420
) {
21+
const errorBoundaryComponent = errorInfo.errorBoundary?.constructor
22+
23+
const isImplicitErrorBoundary =
24+
(process.env.NODE_ENV !== 'production' &&
25+
(errorBoundaryComponent === OldAppDevErrorBoundary ||
26+
errorBoundaryComponent === AppDevErrorBoundary)) ||
27+
(errorBoundaryComponent === ErrorBoundaryHandler &&
28+
(errorInfo.errorBoundary! as InstanceType<typeof ErrorBoundaryHandler>)
29+
.props.errorComponent === DefaultErrorBoundary)
30+
if (isImplicitErrorBoundary) {
31+
// We don't consider errors caught unless they're caught by an explicit error
32+
// boundary. The built-in ones are considered implicit.
33+
// This mimics how the same app would behave without Next.js.
34+
return onUncaughtError(err, errorInfo)
35+
}
36+
1537
// Skip certain custom errors which are not expected to be reported on client
1638
if (isBailoutToCSRError(err) || isNextRouterError(err)) return
1739

1840
if (process.env.NODE_ENV !== 'production') {
19-
const errorBoundaryComponent = errorInfo.errorBoundary?.constructor
2041
const errorBoundaryName =
2142
// read react component displayName
2243
(errorBoundaryComponent as any)?.displayName ||

test/development/app-dir/owner-stack/owner-stack.test.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,7 @@ describe('app-dir - owner-stack', () => {
5454
at useThrowError
5555
at useErrorHook
5656
at Page
57-
at ReactDevOverlay
58-
at HotReload
59-
at Router
60-
at AppRouter
61-
at ServerRoot"
57+
at ClientPageRoot"
6258
`)
6359
})
6460

@@ -144,11 +140,7 @@ describe('app-dir - owner-stack', () => {
144140
at useThrowError
145141
at useErrorHook
146142
at Page
147-
at ReactDevOverlay
148-
at HotReload
149-
at Router
150-
at AppRouter
151-
at ServerRoot"
143+
at ClientPageRoot"
152144
`)
153145
})
154146

0 commit comments

Comments
 (0)