From 6786563f3cbbc9b16d5a8187207b5bd904386e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 26 Mar 2024 20:44:07 -0700 Subject: [PATCH] [Fiber] Don't Rethrow Errors at the Root (#28627) Stacked on top of #28498 for test fixes. ### Don't Rethrow When we started React it was 1:1 setState calls a series of renders and if they error, it errors where the setState was called. Simple. However, then batching came and the error actually got thrown somewhere else. With concurrent mode, it's not even possible to get setState itself to throw anymore. In fact, all APIs that can rethrow out of React are executed either at the root of the scheduler or inside a DOM event handler. If you throw inside a React.startTransition callback that's sync, then that will bubble out of the startTransition but if you throw inside an async callback or a useTransition we now need to handle it at the hook site. So in 19 we need to make all React.startTransition swallow the error (and report them to reportError). The only one remaining that can throw is flushSync but it doesn't really make sense for it to throw at the callsite neither because batching. Just because something rendered in this flush doesn't mean it was rendered due to what was just scheduled and doesn't mean that it should abort any of the remaining code afterwards. setState is fire and forget. It's send an instruction elsewhere, it's not part of the current imperative code. Error boundaries never rethrow. Since you should really always have error boundaries, most of the time, it wouldn't rethrow anyway. Rethrowing also actually currently drops errors on the floor since we can only rethrow the first error, so to avoid that we'd need to call reportError anyway. This happens in RN events. The other issue with rethrowing is that it logs an extra console.error. Since we're not sure that user code will actually log it anywhere we still log it too just like we do with errors inside error boundaries which leads all of these to log twice. The goal of this PR is to never rethrow out of React instead, errors outside of error boundaries get logged to reportError. Event system errors too. ### Breaking Changes The main thing this affects is testing where you want to inspect the errors thrown. To make it easier to port, if you're inside `act` we track the error into act in an aggregate error and then rethrow it at the root of `act`. Unlike before though, if you flush synchronously inside of act it'll still continue until the end of act before rethrowing. I expect most user code breakages would be to migrate from `flushSync` to `act` if you assert on throwing. However, in the React repo we also have `internalAct` and the `waitForThrow` helpers. Since these have to use public production implementations we track these using the global onerror or process uncaughtException. Unlike regular act, includes both event handler errors and onRecoverableError by default too. Not just render/commit errors. So I had to account for that in our tests. We restore logging an extra log for uncaught errors after the main log with the component stack in it. We use `console.warn`. This is not yet ignorable if you preventDefault to the main error event. To avoid confusion if you don't end up logging the error to console I just added `An error occurred`. ### Polyfill All browsers we support really supports `reportError` but not all test and server environments do, so I implemented a polyfill for browser and node in `shared/reportGlobalError`. I don't love that this is included in all builds and gets duplicated into isomorphic even though it's not actually needed in production. Maybe in the future we can require a polyfill for this. ### Follow Ups In a follow up, I'll make caught vs uncaught error handling be configurable too. --------- Co-authored-by: Ricky Hanlon --- .../ReactInternalTestUtils.js | 71 ++++- packages/internal-test-utils/internalAct.js | 51 ++- .../src/events/DOMPluginEventSystem.js | 19 +- .../__tests__/InvalidEventListeners-test.js | 12 +- .../ReactBrowserEventEmitter-test.js | 4 +- .../__tests__/ReactCompositeComponent-test.js | 58 ++-- .../react-dom/src/__tests__/ReactDOM-test.js | 141 +++++---- .../ReactDOMConsoleErrorReporting-test.js | 136 ++++---- ...eactDOMConsoleErrorReportingLegacy-test.js | 293 +++++++++--------- .../src/__tests__/ReactDOMFiber-test.js | 10 +- .../__tests__/ReactDOMHydrationDiff-test.js | 6 + .../src/__tests__/ReactDOMLegacyFiber-test.js | 49 +-- .../src/__tests__/ReactDOMRoot-test.js | 8 +- .../src/__tests__/ReactDOMSelect-test.js | 8 +- ...DOMServerPartialHydration-test.internal.js | 19 +- .../ReactErrorBoundaries-test.internal.js | 22 +- .../ReactErrorLoggingRecovery-test.js | 32 +- ...eactLegacyErrorBoundaries-test.internal.js | 198 +++++++----- .../src/__tests__/ReactLegacyUpdates-test.js | 169 ++++++---- .../src/__tests__/ReactUpdates-test.js | 78 ++--- packages/react-dom/src/client/ReactDOMRoot.js | 16 +- .../ReactNativeEvents-test.internal.js | 16 +- .../ReactNativeMount-test.internal.js | 28 +- .../src/ReactFiberErrorLogger.js | 97 ++++-- .../src/ReactFiberRootScheduler.js | 39 +-- .../react-reconciler/src/ReactFiberThrow.js | 3 - .../src/ReactFiberWorkLoop.js | 44 +-- .../src/__tests__/ReactFlushSync-test.js | 10 +- .../ReactFlushSyncNoAggregateError-test.js | 12 +- .../src/__tests__/ReactHooks-test.internal.js | 2 +- .../ReactHooksWithNoopRenderer-test.js | 104 ++++--- ...tIncrementalErrorHandling-test.internal.js | 72 ++--- .../ReactIncrementalErrorLogging-test.js | 128 ++++---- .../src/__tests__/ReactLazy-test.internal.js | 2 +- .../ReactSuspenseWithNoopRenderer-test.js | 6 +- .../src/__tests__/ReactFresh-test.js | 190 ++++++------ packages/react/src/ReactAct.js | 52 +++- packages/react/src/ReactCurrentActQueue.js | 3 + packages/react/src/ReactStartTransition.js | 19 +- .../ReactCoffeeScriptClass-test.coffee | 18 +- .../react/src/__tests__/ReactES6Class-test.js | 31 +- .../__tests__/ReactTypeScriptClass-test.ts | 30 +- packages/shared/reportGlobalError.js | 52 ++++ .../useSyncExternalStoreShared-test.js | 152 ++++++--- scripts/jest/matchers/toWarnDev.js | 6 +- scripts/jest/setupTests.js | 2 +- scripts/jest/shouldIgnoreConsoleError.js | 7 +- scripts/rollup/validate/eslintrc.fb.js | 1 + scripts/rollup/validate/eslintrc.rn.js | 3 + scripts/rollup/validate/eslintrc.umd.js | 3 + 50 files changed, 1495 insertions(+), 1037 deletions(-) create mode 100644 packages/shared/reportGlobalError.js diff --git a/packages/internal-test-utils/ReactInternalTestUtils.js b/packages/internal-test-utils/ReactInternalTestUtils.js index df77f23448b13..c6cb39fd73e08 100644 --- a/packages/internal-test-utils/ReactInternalTestUtils.js +++ b/packages/internal-test-utils/ReactInternalTestUtils.js @@ -13,6 +13,8 @@ import simulateBrowserEventDispatch from './simulateBrowserEventDispatch'; export {act} from './internalAct'; +import {thrownErrors, actingUpdatesScopeDepth} from './internalAct'; + function assertYieldsWereCleared(caller) { const actualYields = SchedulerMock.unstable_clearLog(); if (actualYields.length !== 0) { @@ -110,6 +112,14 @@ ${diff(expectedLog, actualLog)} throw error; } +function aggregateErrors(errors: Array): mixed { + if (errors.length > 1 && typeof AggregateError === 'function') { + // eslint-disable-next-line no-undef + return new AggregateError(errors); + } + return errors[0]; +} + export async function waitForThrow(expectedError: mixed): mixed { assertYieldsWereCleared(waitForThrow); @@ -126,31 +136,72 @@ export async function waitForThrow(expectedError: mixed): mixed { error.message = 'Expected something to throw, but nothing did.'; throw error; } + + const errorHandlerDOM = function (event: ErrorEvent) { + // Prevent logs from reprinting this error. + event.preventDefault(); + thrownErrors.push(event.error); + }; + const errorHandlerNode = function (err: mixed) { + thrownErrors.push(err); + }; + // We track errors that were logged globally as if they occurred in this scope and then rethrow them. + if (actingUpdatesScopeDepth === 0) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.addEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.on('uncaughtException', errorHandlerNode); + } + } try { SchedulerMock.unstable_flushAllWithoutAsserting(); } catch (x) { + thrownErrors.push(x); + } finally { + if (actingUpdatesScopeDepth === 0) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.removeEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.off('uncaughtException', errorHandlerNode); + } + } + } + if (thrownErrors.length > 0) { + const thrownError = aggregateErrors(thrownErrors); + thrownErrors.length = 0; + if (expectedError === undefined) { // If no expected error was provided, then assume the caller is OK with // any error being thrown. We're returning the error so they can do // their own checks, if they wish. - return x; + return thrownError; } - if (equals(x, expectedError)) { - return x; + if (equals(thrownError, expectedError)) { + return thrownError; } if ( typeof expectedError === 'string' && - typeof x === 'object' && - x !== null && - typeof x.message === 'string' + typeof thrownError === 'object' && + thrownError !== null && + typeof thrownError.message === 'string' ) { - if (x.message.includes(expectedError)) { - return x; + if (thrownError.message.includes(expectedError)) { + return thrownError; } else { error.message = ` Expected error was not thrown. -${diff(expectedError, x.message)} +${diff(expectedError, thrownError.message)} `; throw error; } @@ -158,7 +209,7 @@ ${diff(expectedError, x.message)} error.message = ` Expected error was not thrown. -${diff(expectedError, x)} +${diff(expectedError, thrownError)} `; throw error; } diff --git a/packages/internal-test-utils/internalAct.js b/packages/internal-test-utils/internalAct.js index 082be156dc89b..7b22c44697e36 100644 --- a/packages/internal-test-utils/internalAct.js +++ b/packages/internal-test-utils/internalAct.js @@ -20,7 +20,9 @@ import * as Scheduler from 'scheduler/unstable_mock'; import enqueueTask from './enqueueTask'; -let actingUpdatesScopeDepth: number = 0; +export let actingUpdatesScopeDepth: number = 0; + +export const thrownErrors: Array = []; async function waitForMicrotasks() { return new Promise(resolve => { @@ -28,6 +30,14 @@ async function waitForMicrotasks() { }); } +function aggregateErrors(errors: Array): mixed { + if (errors.length > 1 && typeof AggregateError === 'function') { + // eslint-disable-next-line no-undef + return new AggregateError(errors); + } + return errors[0]; +} + export async function act(scope: () => Thenable): Thenable { if (Scheduler.unstable_flushUntilNextPaint === undefined) { throw Error( @@ -63,6 +73,28 @@ export async function act(scope: () => Thenable): Thenable { // public version of `act`, though we maybe should in the future. await waitForMicrotasks(); + const errorHandlerDOM = function (event: ErrorEvent) { + // Prevent logs from reprinting this error. + event.preventDefault(); + thrownErrors.push(event.error); + }; + const errorHandlerNode = function (err: mixed) { + thrownErrors.push(err); + }; + // We track errors that were logged globally as if they occurred in this scope and then rethrow them. + if (actingUpdatesScopeDepth === 1) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.addEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.on('uncaughtException', errorHandlerNode); + } + } + try { const result = await scope(); @@ -106,10 +138,27 @@ export async function act(scope: () => Thenable): Thenable { Scheduler.unstable_flushUntilNextPaint(); } while (true); + if (thrownErrors.length > 0) { + // Rethrow any errors logged by the global error handling. + const thrownError = aggregateErrors(thrownErrors); + thrownErrors.length = 0; + throw thrownError; + } + return result; } finally { const depth = actingUpdatesScopeDepth; if (depth === 1) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.removeEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.off('uncaughtException', errorHandlerNode); + } global.IS_REACT_ACT_ENVIRONMENT = previousIsActEnvironment; } actingUpdatesScopeDepth = depth - 1; diff --git a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js index f26ad485590d6..7e7221a6d393f 100644 --- a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js @@ -68,6 +68,8 @@ import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; import * as FormActionEventPlugin from './plugins/FormActionEventPlugin'; +import reportGlobalError from 'shared/reportGlobalError'; + type DispatchListener = { instance: null | Fiber, listener: Function, @@ -226,9 +228,6 @@ export const nonDelegatedEvents: Set = new Set([ ...mediaEventTypes, ]); -let hasError: boolean = false; -let caughtError: mixed = null; - function executeDispatch( event: ReactSyntheticEvent, listener: Function, @@ -238,12 +237,7 @@ function executeDispatch( try { listener(event); } catch (error) { - if (!hasError) { - hasError = true; - caughtError = error; - } else { - // TODO: Make sure this error gets logged somehow. - } + reportGlobalError(error); } event.currentTarget = null; } @@ -285,13 +279,6 @@ export function processDispatchQueue( processDispatchQueueItemsInOrder(event, listeners, inCapturePhase); // event system doesn't use pooling. } - // This would be a good time to rethrow if any of the event handlers threw. - if (hasError) { - const error = caughtError; - hasError = false; - caughtError = null; - throw error; - } } function dispatchEventsForPlugins( diff --git a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js index e412256ab2678..d7045d2756ad4 100644 --- a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js +++ b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js @@ -51,13 +51,11 @@ describe('InvalidEventListeners', () => { } window.addEventListener('error', handleWindowError); try { - await act(() => { - node.dispatchEvent( - new MouseEvent('click', { - bubbles: true, - }), - ); - }); + node.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + }), + ); } finally { window.removeEventListener('error', handleWindowError); } diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index 9c023f06d9d2a..965c20dbddf53 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -195,9 +195,7 @@ describe('ReactBrowserEventEmitter', () => { }); window.addEventListener('error', errorHandler); try { - await act(() => { - CHILD.click(); - }); + CHILD.click(); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index bf082369e0610..561928b24faf3 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -223,12 +223,12 @@ describe('ReactCompositeComponent', () => { const el = document.createElement('div'); const root = ReactDOMClient.createRoot(el); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow( + }).rejects.toThrow( 'Objects are not valid as a React child (found: object with keys {render}).', ); }).toErrorDev( @@ -526,12 +526,12 @@ describe('ReactCompositeComponent', () => { } } const root = ReactDOMClient.createRoot(container); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(TypeError); + }).rejects.toThrow(TypeError); }).toErrorDev( 'Warning: The component appears to have a render method, ' + "but doesn't extend React.Component. This is likely to cause errors. " + @@ -539,11 +539,11 @@ describe('ReactCompositeComponent', () => { ); // Test deduplication - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(TypeError); + }).rejects.toThrow(TypeError); }); it('should warn about `setState` in render', async () => { @@ -596,11 +596,11 @@ describe('ReactCompositeComponent', () => { expect(ReactCurrentOwner.current).toBe(null); const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(instance); }); - }).toThrow(); + }).rejects.toThrow(); expect(ReactCurrentOwner.current).toBe(null); }); @@ -884,7 +884,7 @@ describe('ReactCompositeComponent', () => { ); }); - it('should only call componentWillUnmount once', () => { + it('should only call componentWillUnmount once', async () => { let app; let count = 0; @@ -919,14 +919,14 @@ describe('ReactCompositeComponent', () => { }; const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(); }); - ReactDOM.flushSync(() => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); expect(count).toBe(1); }); @@ -1211,7 +1211,7 @@ describe('ReactCompositeComponent', () => { assertLog(['setState callback called']); }); - it('should return a meaningful warning when constructor is returned', () => { + it('should return a meaningful warning when constructor is returned', async () => { class RenderTextInvalidConstructor extends React.Component { constructor(props) { super(props); @@ -1224,12 +1224,12 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); }).toErrorDev([ 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + 'did you accidentally return an object from the constructor?', @@ -1260,16 +1260,16 @@ describe('ReactCompositeComponent', () => { ); }); - it('should return error if render is not defined', () => { + it('should return error if render is not defined', async () => { class RenderTestUndefinedRender extends React.Component {} const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); }).toErrorDev([ 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + 'you may have forgotten to define `render`.', diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 83e3e0229d7b1..e72f726c16503 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -166,6 +166,9 @@ describe('ReactDOM', () => { // @gate !disableLegacyMode it('throws in render() if the mount callback in legacy roots is not a function', async () => { + spyOnDev(console, 'warn'); + spyOnDev(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -180,40 +183,55 @@ describe('ReactDOM', () => { } const myDiv = document.createElement('div'); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, 'no'); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: no.', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', + }).toErrorDev( + [ + 'Warning: Expected the last optional `callback` argument to be a function. Instead received: no.', + 'Warning: Expected the last optional `callback` argument to be a function. Instead received: no.', + ], + {withoutStack: 2}, ); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, new Foo()); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); }); @@ -234,42 +252,57 @@ describe('ReactDOM', () => { const myDiv = document.createElement('div'); ReactDOM.render(, myDiv); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, 'no'); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: no.', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: no.', + 'Expected the last optional `callback` argument to be a function. Instead received: no.', + ], + {withoutStack: 2}, ); ReactDOM.render(, myDiv); // Re-mount - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); ReactDOM.render(, myDiv); // Re-mount - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, new Foo()); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index bd80905478bf0..65aa67d342579 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -16,16 +16,14 @@ describe('ReactDOMConsoleErrorReporting', () => { let NoError; let container; let windowOnError; - let waitForThrow; + let Scheduler; beforeEach(() => { jest.resetModules(); act = require('internal-test-utils').act; React = require('react'); ReactDOMClient = require('react-dom/client'); - - const InternalTestUtils = require('internal-test-utils'); - waitForThrow = InternalTestUtils.waitForThrow; + Scheduler = require('scheduler'); ErrorBoundary = class extends React.Component { state = {error: null}; @@ -46,6 +44,8 @@ describe('ReactDOMConsoleErrorReporting', () => { document.body.appendChild(container); windowOnError = jest.fn(); window.addEventListener('error', windowOnError); + spyOnDevAndProd(console, 'error').mockImplementation(() => {}); + spyOnDevAndProd(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { @@ -54,11 +54,14 @@ describe('ReactDOMConsoleErrorReporting', () => { jest.restoreAllMocks(); }); + async function fakeAct(cb) { + // We don't use act/waitForThrow here because we want to observe how errors are reported for real. + await cb(); + Scheduler.unstable_flushAll(); + } + describe('ReactDOMClient.createRoot', () => { it('logs errors during event handlers', async () => { - const originalError = console.error; - console.error = jest.fn(); - function Foo() { return (