Skip to content

Commit

Permalink
Print the error in DEV with addendum
Browse files Browse the repository at this point in the history
Since we no longer get the error printed by the browser automatically
we have to manually print the error with an addendum which contains
component stacks.
  • Loading branch information
sebmarkbage committed Mar 10, 2024
1 parent ddd0d91 commit bfa05cd
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('Consider adding an error boundary'),
],
]);
} else {
Expand Down Expand Up @@ -183,10 +190,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('ErrorBoundary'),
],
]);
} else {
Expand Down Expand Up @@ -236,10 +250,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('Consider adding an error boundary'),
],
]);
} else {
Expand Down Expand Up @@ -292,10 +313,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('ErrorBoundary'),
],
]);
} else {
Expand Down Expand Up @@ -345,10 +373,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('Consider adding an error boundary'),
],
]);
} else {
Expand Down Expand Up @@ -401,10 +436,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Formatting
expect.stringContaining('%o'),
expect.objectContaining({
message: 'Boom',
}),
// Addendum by React:
expect.stringContaining(
'The above error occurred in the <Foo> component',
),
expect.stringContaining('Foo'),
expect.stringContaining('ErrorBoundary'),
],
]);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ describe('ReactErrorBoundaries', () => {
});
if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error.mock.calls[0][0]).toContain(
expect(console.error.mock.calls[0][2]).toContain(
'The above error occurred in the <BrokenRender> component:',
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ describe('ReactLegacyErrorBoundaries', () => {
expect(console.error.mock.calls[0][0]).toContain(
'ReactDOM.render is no longer supported',
);
expect(console.error.mock.calls[1][0]).toContain(
expect(console.error.mock.calls[1][2]).toContain(
'The above error occurred in the <BrokenRender> component:',
);
}
Expand Down
15 changes: 0 additions & 15 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,21 +243,6 @@ function shouldProfile(current: Fiber): boolean {
);
}

export function reportUncaughtErrorInDEV(error: mixed) {
// Wrapping each small part of the commit phase into a guarded
// callback is a bit too slow (https://github.com/facebook/react/pull/21666).
// But we rely on it to surface errors to DEV tools like overlays
// (https://github.com/facebook/react/issues/21712).
// As a compromise, rethrow only caught errors in a guard.
if (__DEV__) {
// TODO: This trick no longer works. Should probably use reportError maybe.
// invokeGuardedCallback(null, () => {
// throw error;
// });
// clearCaughtError();
}
}

function callComponentWillUnmountWithTimer(current: Fiber, instance: any) {
instance.props = current.memoizedProps;
instance.state = current.memoizedState;
Expand Down
21 changes: 10 additions & 11 deletions packages/react-reconciler/src/ReactFiberErrorLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ export function logCapturedError(
}
// The error is fatal. Since the silencing might have
// been accidental, we'll surface it anyway.
// However, the browser would have silenced the original error
// so we'll print it first, and then print the stack addendum.
console['error'](error); // Don't transform to our wrapper
// For a more detailed description of this block, see:
// https://github.com/facebook/react/pull/13384
}
Expand All @@ -70,16 +67,18 @@ export function logCapturedError(
`React will try to recreate this component tree from scratch ` +
`using the error boundary you provided, ${errorBoundaryName}.`;
}
const combinedMessage =
`${componentNameMessage}\n${componentStack}\n\n` +
`${errorBoundaryMessage}`;

// TODO: The error is no longer printed by the browser.
// In development, we provide our own message with just the component stack.
// We don't include the original error message and JS stack because the browser
// has already printed it. Even if the application swallows the error, it is still
// displayed by the browser thanks to the DEV-only fake event trick in ReactErrorUtils.
console['error'](combinedMessage); // Don't transform to our wrapper
// In development, we provide our own message which includes the component stack
// in addition to the error.
console['error'](
// Don't transform to our wrapper
'%o\n\n%s\n%s\n\n%s',
error,
componentNameMessage,
componentStack,
errorBoundaryMessage,
);
} else {
// In production, we print the error directly.
// This will include the message, the JS stack, and anything the browser wants to show.
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ import {
reconnectPassiveEffects,
reappearLayoutEffects,
disconnectPassiveEffect,
reportUncaughtErrorInDEV,
invokeLayoutEffectMountInDEV,
invokePassiveEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
Expand Down Expand Up @@ -3420,7 +3419,6 @@ export function captureCommitPhaseError(
error: mixed,
) {
if (__DEV__) {
reportUncaughtErrorInDEV(error);
setIsRunningInsertionEffect(false);
}
if (sourceFiber.tag === HostRoot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,8 @@ describe('ReactIncrementalErrorHandling', () => {

if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error.mock.calls[0][0]).toContain(
expect(console.error.mock.calls[0][1]).toBe(notAnError);
expect(console.error.mock.calls[0][2]).toContain(
'The above error occurred in the <BadRender> component:',
);
} else {
Expand Down Expand Up @@ -1911,7 +1912,7 @@ describe('ReactIncrementalErrorHandling', () => {
expect(console.error.mock.calls[0][0]).toContain(
'Cannot update a component (`%s`) while rendering a different component',
);
expect(console.error.mock.calls[1][0]).toContain(
expect(console.error.mock.calls[1][2]).toContain(
'The above error occurred in the <App> component',
);
}
Expand Down
Loading

0 comments on commit bfa05cd

Please sign in to comment.