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 11, 2024
1 parent 944275e commit aae244d
Show file tree
Hide file tree
Showing 11 changed files with 219 additions and 122 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 @@ -159,10 +159,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
[
// 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 @@ -215,12 +222,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
// TODO: There is no log for this error now.
[
// 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 @@ -271,12 +284,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
// TODO: There should be an error reported here.
[
// 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 @@ -332,12 +351,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
// TODO: There should be a log here.
[
// 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 @@ -389,12 +414,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
// TODO: We should have logged an error.
[
// 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 @@ -450,12 +481,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[expect.stringContaining('ReactDOM.render is no longer supported')],
// TODO: There should be a log here.
[
// 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
41 changes: 12 additions & 29 deletions packages/react-reconciler/src/ReactFiberErrorLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {Fiber} from './ReactInternalTypes';
import type {CapturedValue} from './ReactCapturedValue';

import {showErrorDialog} from './ReactFiberErrorDialog';
import {ClassComponent} from './ReactWorkTags';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import {HostRoot} from 'react-reconciler/src/ReactWorkTags';

Expand All @@ -33,25 +32,8 @@ export function logCapturedError(
const source = errorInfo.source;
const stack = errorInfo.stack;
const componentStack = stack !== null ? stack : '';
// Browsers support silencing uncaught errors by calling
// `preventDefault()` in window `error` handler.
// We record this information as an expando on the error.
// TODO: We no longer track this.
if (error != null && error._suppressLogging) {
if (boundary.tag === ClassComponent) {
// The error is recoverable and was silenced.
// Ignore it and don't print the stack addendum.
// This is handy for testing error boundaries without noise.
return;
}
// 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
}
// TODO: There's no longer a way to silence these warnings e.g. for tests.
// See https://github.com/facebook/react/pull/13384

const componentName = source ? getComponentNameFromFiber(source) : null;
const componentNameMessage = componentName
Expand All @@ -70,16 +52,17 @@ 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 @@ -186,7 +186,6 @@ import {
reconnectPassiveEffects,
reappearLayoutEffects,
disconnectPassiveEffect,
reportUncaughtErrorInDEV,
invokeLayoutEffectMountInDEV,
invokePassiveEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
Expand Down Expand Up @@ -3384,7 +3383,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 aae244d

Please sign in to comment.