diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index b001fcf3dcacc..bd80905478bf0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js index c40dff4e938fd..e75b4b5e29045 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { @@ -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 component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 4170168ddc822..284cb0599aded 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -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 component:', ); } diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index 960ba074c1ea1..bed800fb1da73 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -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 component:', ); } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8ee7165e3f645..fdd289d93280f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -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; diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 7eaff9a716923..14a1ebb352f89 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -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'; @@ -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 @@ -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. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 267ea479cd099..38937249be52a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -188,7 +188,6 @@ import { reconnectPassiveEffects, reappearLayoutEffects, disconnectPassiveEffect, - reportUncaughtErrorInDEV, invokeLayoutEffectMountInDEV, invokePassiveEffectMountInDEV, invokeLayoutEffectUnmountInDEV, @@ -3420,7 +3419,6 @@ export function captureCommitPhaseError( error: mixed, ) { if (__DEV__) { - reportUncaughtErrorInDEV(error); setIsRunningInsertionEffect(false); } if (sourceFiber.tag === HostRoot) { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index f519b0710ffa0..1613f1e048ec7 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -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 component:', ); } else { @@ -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 component', ); } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index adfeb238695dd..82edcf2db4503 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -60,22 +60,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('constructor error'); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'constructor error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'constructor error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'constructor error', + }), + ); + } }); it('should log errors that occur during the commit phase', async () => { @@ -96,22 +108,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('componentDidMount error'); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'componentDidMount error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'componentDidMount error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'componentDidMount error', + }), + ); + } }); it('should ignore errors thrown in log method to prevent cycle', async () => { @@ -135,22 +159,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('render error'); expect(logCapturedErrorCalls.length).toBe(1); - expect(logCapturedErrorCalls[0]).toEqual( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'render error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'render error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(logCapturedErrorCalls[0]).toEqual( + expect.objectContaining({ + message: 'render error', + }), + ); + } // The error thrown in logCapturedError should be rethrown with a clean stack expect(() => { jest.runAllTimers(); @@ -203,20 +239,31 @@ describe('ReactIncrementalErrorLogging', () => { ); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) Foo (.*)\n' + - '\\s+(in|at) ErrorBoundary (.*)\n\n' + - 'React will try to recreate this component tree from scratch ' + - 'using the error boundary you provided, ErrorBoundary.', - ), - ) - : expect.objectContaining({ - message: 'oops', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'oops', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)', + ), + ), + expect.stringContaining( + 'React will try to recreate this component tree from scratch ' + + 'using the error boundary you provided, ErrorBoundary.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'oops', + }), + ); + } }); }); diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index 7e5a7b0365549..5d460444440d8 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -608,6 +608,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ? [ 'Uncaught [', 'The result of getSnapshot should be cached to avoid an infinite loop', + 'The above error occurred in the', ] : [ 'The result of getSnapshot should be cached to avoid an infinite loop', diff --git a/scripts/jest/shouldIgnoreConsoleError.js b/scripts/jest/shouldIgnoreConsoleError.js index 01fe879fcf2b2..2627e927eb892 100644 --- a/scripts/jest/shouldIgnoreConsoleError.js +++ b/scripts/jest/shouldIgnoreConsoleError.js @@ -7,10 +7,13 @@ module.exports = function shouldIgnoreConsoleError( ) { if (__DEV__) { if (typeof format === 'string') { - if (format.indexOf('The above error occurred') === 0) { - // TODO: These errors should be removed. - // This looks like an error addendum from ReactFiberErrorLogger. - // Ignore it too. + if ( + args[0] != null && + typeof args[0].message === 'string' && + typeof args[0].stack === 'string' + ) { + // This looks like an error with addendum from ReactFiberErrorLogger. + // They are noisy too so we'll try to ignore them. return true; } if (