From 848e802d203e531daf2b9b0edb281a1eb6c5415d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 4 Feb 2022 07:57:33 -0800 Subject: [PATCH] Add onRecoverableError option to hydrateRoot, createRoot (#23207) * [RFC] Add onHydrationError option to hydrateRoot This is not the final API but I'm pushing it for discussion purposes. When an error is thrown during hydration, we fallback to client rendering, without triggering an error boundary. This is good because, in many cases, the UI will recover and the user won't even notice that something has gone wrong behind the scenes. However, we shouldn't recover from these errors silently, because the underlying cause might be pretty serious. Server-client mismatches are not supposed to happen, even if UI doesn't break from the users perspective. Ignoring them could lead to worse problems later. De-opting from server to client rendering could also be a significant performance regression, depending on the scope of the UI it affects. So we need a way to log when hydration errors occur. This adds a new option for `hydrateRoot` called `onHydrationError`. It's symmetrical to the server renderer's `onError` option, and serves the same purpose. When no option is provided, the default behavior is to schedule a browser task and rethrow the error. This will trigger the normal browser behavior for errors, including dispatching an error event. If the app already has error monitoring, this likely will just work as expected without additional configuration. However, we can also expose additional metadata about these errors, like which Suspense boundaries were affected by the de-opt to client rendering. (I have not exposed any metadata in this commit; API needs more design work.) There are other situations besides hydration where we recover from an error without surfacing it to the user, or notifying an error boundary. For example, if an error occurs during a concurrent render, it could be due to a data race, so we try again synchronously in case that fixes it. We should probably expose a way to log these types of errors, too. (Also not implemented in this commit.) * Log all recoverable errors This expands the scope of onHydrationError to include all errors that are not surfaced to the UI (an error boundary). In addition to errors that occur during hydration, this also includes errors that recoverable by de-opting to synchronous rendering. Typically (or really, by definition) these errors are the result of a concurrent data race; blocking the main thread fixes them by prevents subsequent races. The logic for de-opting to synchronous rendering already existed. The only thing that has changed is that we now log the errors instead of silently proceeding. The logging API has been renamed from onHydrationError to onRecoverableError. * Don't log recoverable errors until commit phase If the render is interrupted and restarts, we don't want to log the errors multiple times. This change only affects errors that are recovered by de-opting to synchronous rendering; we'll have to do something else for errors during hydration, since they use a different recovery path. * Only log hydration error if client render succeeds Similar to previous step. When an error occurs during hydration, we only want to log it if falling back to client rendering _succeeds_. If client rendering fails, the error will get reported to the nearest error boundary, so there's no need for a duplicate log. To implement this, I added a list of errors to the hydration context. If the Suspense boundary successfully completes, they are added to the main recoverable errors queue (the one I added in the previous step.) * Log error with queueMicrotask instead of Scheduler If onRecoverableError is not provided, we default to rethrowing the error in a separate task. Originally, I scheduled the task with idle priority, but @sebmarkbage made the good point that if there are multiple errors logs, we want to preserve the original order. So I've switched it to a microtask. The priority can be lowered in userspace by scheduling an additional task inside onRecoverableError. * Only use host config method for default behavior Redefines the contract of the host config's logRecoverableError method to be a default implementation for onRecoverableError if a user-provided one is not provided when the root is created. * Log with reportError instead of rethrowing In modern browsers, reportError will dispatch an error event, emulating an uncaught JavaScript error. We can do this instead of rethrowing recoverable errors in a microtask, which is nice because it avoids any subtle ordering issues. In older browsers and test environments, we'll fall back to console.error. * Naming nits queueRecoverableHydrationErrors -> upgradeHydrationErrorsToRecoverable --- packages/react-art/src/ReactARTHostConfig.js | 4 + .../src/__tests__/ReactDOMFizzServer-test.js | 118 ++++++++++++++++-- ...DOMServerPartialHydration-test.internal.js | 52 +++++++- .../src/client/ReactDOMHostConfig.js | 14 +++ .../react-dom/src/client/ReactDOMLegacy.js | 1 + packages/react-dom/src/client/ReactDOMRoot.js | 12 ++ .../react-native-renderer/src/ReactFabric.js | 1 + .../src/ReactFabricHostConfig.js | 4 + .../src/ReactNativeHostConfig.js | 4 + .../src/ReactNativeRenderer.js | 1 + .../src/createReactNoop.js | 17 ++- .../src/ReactFiberCompleteWork.new.js | 7 ++ .../src/ReactFiberCompleteWork.old.js | 7 ++ .../src/ReactFiberHydrationContext.new.js | 24 ++++ .../src/ReactFiberHydrationContext.old.js | 24 ++++ .../src/ReactFiberReconciler.new.js | 2 + .../src/ReactFiberReconciler.old.js | 2 + .../src/ReactFiberRoot.new.js | 15 ++- .../src/ReactFiberRoot.old.js | 15 ++- .../src/ReactFiberThrow.new.js | 11 +- .../src/ReactFiberThrow.old.js | 11 +- .../src/ReactFiberWorkLoop.new.js | 79 ++++++++++-- .../src/ReactFiberWorkLoop.old.js | 79 ++++++++++-- .../src/ReactInternalTypes.js | 2 + .../ReactFiberHostContext-test.internal.js | 2 + .../useMutableSourceHydration-test.js | 39 +++++- .../src/forks/ReactFiberHostConfig.custom.js | 1 + packages/react-server/src/ReactFizzServer.js | 12 +- .../react-server/src/ReactFlightServer.js | 10 +- .../src/ReactTestHostConfig.js | 4 + .../src/ReactTestRenderer.js | 1 + scripts/flow/environment.js | 1 + scripts/print-warnings/print-warnings.js | 10 +- scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.cjs2015.js | 1 + scripts/rollup/validate/eslintrc.esm.js | 1 + scripts/rollup/validate/eslintrc.fb.js | 1 + scripts/rollup/validate/eslintrc.rn.js | 1 + scripts/rollup/validate/eslintrc.umd.js | 1 + 39 files changed, 530 insertions(+), 62 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index 47bced8a1274a..92417f77ae163 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -451,3 +451,7 @@ export function preparePortalMount(portalInstance: any): void { export function detachDeletedInstance(node: Instance): void { // noop } + +export function logRecoverableError(error) { + // noop +} diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 2d6f880851a26..483c588c37b9f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1723,12 +1723,22 @@ describe('ReactDOMFizzServer', () => { }); expect(Scheduler).toHaveYielded(['server']); - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Log recoverable error: ' + error.message, + ); + }, + }); if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { expect(() => { // The first paint switches to client rendering due to mismatch - expect(Scheduler).toFlushUntilNextPaint(['client']); + expect(Scheduler).toFlushUntilNextPaint([ + 'client', + 'Log recoverable error: An error occurred during hydration. ' + + 'The server HTML was replaced with client content', + ]); }).toErrorDev( 'Warning: An error occurred during hydration. The server HTML was replaced with client content', {withoutStack: true}, @@ -1805,13 +1815,23 @@ describe('ReactDOMFizzServer', () => { }); expect(Scheduler).toHaveYielded(['server']); - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Log recoverable error: ' + error.message, + ); + }, + }); if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { // The first paint uses the client due to mismatch forcing client render expect(() => { // The first paint switches to client rendering due to mismatch - expect(Scheduler).toFlushUntilNextPaint(['client']); + expect(Scheduler).toFlushUntilNextPaint([ + 'client', + 'Log recoverable error: An error occurred during hydration. ' + + 'The server HTML was replaced with client content', + ]); }).toErrorDev( 'Warning: An error occurred during hydration. The server HTML was replaced with client content', {withoutStack: true}, @@ -1897,9 +1917,15 @@ describe('ReactDOMFizzServer', () => { // Hydrate the tree. Child will throw during hydration, but not when it // falls back to client rendering. isClient = true; - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue(error.message); + }, + }); - expect(Scheduler).toFlushAndYield(['Yay!']); + // An error logged but instead of surfacing it to the UI, we switched + // to client rendering. + expect(Scheduler).toFlushAndYield(['Yay!', 'Hydration error']); expect(getVisibleChildren(container)).toEqual(
@@ -1975,8 +2001,16 @@ describe('ReactDOMFizzServer', () => { // Hydrate the tree. Child will throw during render. isClient = true; - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Log recoverable error: ' + error.message, + ); + }, + }); + // Because we failed to recover from the error, onRecoverableError + // shouldn't be called. expect(Scheduler).toFlushAndYield([]); expect(getVisibleChildren(container)).toEqual('Oops!'); }, @@ -2049,9 +2083,15 @@ describe('ReactDOMFizzServer', () => { // Hydrate the tree. Child will throw during hydration, but not when it // falls back to client rendering. isClient = true; - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue(error.message); + }, + }); - expect(Scheduler).toFlushAndYield([]); + // An error logged but instead of surfacing it to the UI, we switched + // to client rendering. + expect(Scheduler).toFlushAndYield(['Hydration error']); expect(getVisibleChildren(container)).toEqual(
@@ -2081,4 +2121,64 @@ describe('ReactDOMFizzServer', () => { expect(span3Ref.current).toBe(span3); }, ); + + it('logs regular (non-hydration) errors when the UI recovers', async () => { + let shouldThrow = true; + + function A() { + if (shouldThrow) { + Scheduler.unstable_yieldValue('Oops!'); + throw new Error('Oops!'); + } + Scheduler.unstable_yieldValue('A'); + return 'A'; + } + + function B() { + Scheduler.unstable_yieldValue('B'); + return 'B'; + } + + function App() { + return ( + <> + + + + ); + } + + const root = ReactDOM.createRoot(container, { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged a recoverable error: ' + error.message, + ); + }, + }); + React.startTransition(() => { + root.render(); + }); + + // Partially render A, but yield before the render has finished + expect(Scheduler).toFlushAndYieldThrough(['Oops!', 'Oops!']); + + // React will try rendering again synchronously. During the retry, A will + // not throw. This simulates a concurrent data race that is fixed by + // blocking the main thread. + shouldThrow = false; + expect(Scheduler).toFlushAndYield([ + // Finish initial render attempt + 'B', + + // Render again, synchronously + 'A', + 'B', + + // Log the error + 'Logged a recoverable error: Oops!', + ]); + + // UI looks normal + expect(container.textContent).toEqual('AB'); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 8f2913c15d600..e071a36737104 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -208,7 +208,11 @@ describe('ReactDOMServerPartialHydration', () => { // On the client we don't have all data yet but we want to start // hydrating anyway. suspend = true; - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue(error.message); + }, + }); if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { Scheduler.unstable_flushAll(); } else { @@ -290,7 +294,11 @@ describe('ReactDOMServerPartialHydration', () => { suspend = true; client = true; - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue(error.message); + }, + }); expect(Scheduler).toFlushAndYield([ 'Suspend', 'Component', @@ -316,12 +324,16 @@ describe('ReactDOMServerPartialHydration', () => { 'Component', 'Component', 'Component', + // second pass as client render 'Hello', 'Component', 'Component', 'Component', 'Component', + + // Hydration mismatch is logged + 'An error occurred during hydration. The server HTML was replaced with client content', ]); // Client rendered - suspense comment nodes removed @@ -573,9 +585,19 @@ describe('ReactDOMServerPartialHydration', () => { expect(() => { act(() => { - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue(error.message); + }, + }); }); }).toErrorDev('Did not expect server HTML to contain a in
'); + if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { + expect(Scheduler).toHaveYielded([ + 'An error occurred during hydration. The server HTML was replaced ' + + 'with client content', + ]); + } expect(container.innerHTML).toContain('A'); expect(container.innerHTML).not.toContain('B'); @@ -2997,7 +3019,13 @@ describe('ReactDOMServerPartialHydration', () => { const span = container.getElementsByTagName('span')[0]; expect(span.innerHTML).toBe('Hidden child'); - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Log recoverable error: ' + error.message, + ); + }, + }); Scheduler.unstable_flushAll(); expect(ref.current).toBe(span); @@ -3142,13 +3170,27 @@ describe('ReactDOMServerPartialHydration', () => { expect(() => { act(() => { - ReactDOM.hydrateRoot(container, ); + ReactDOM.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Log recoverable error: ' + error.message, + ); + }, + }); }); }).toErrorDev( 'Warning: An error occurred during hydration. ' + 'The server HTML was replaced with client content in
.', {withoutStack: true}, ); + expect(Scheduler).toHaveYielded([ + 'Log recoverable error: An error occurred during hydration. The server ' + + 'HTML was replaced with client content', + // TODO: There were multiple mismatches in a single container. Should + // we attempt to de-dupe them? + 'Log recoverable error: An error occurred during hydration. The server ' + + 'HTML was replaced with client content', + ]); // We show fallback state when mismatch happens at root expect(container.innerHTML).toEqual( diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 341d1fa7a3764..e2cbed61e5e42 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -374,6 +374,18 @@ export function getCurrentEventPriority(): * { return getEventPriority(currentEvent.type); } +/* global reportError */ +export const logRecoverableError = + typeof reportError === 'function' + ? // In modern browsers, reportError will dispatch an error event, + // emulating an uncaught JavaScript error. + reportError + : (error: mixed) => { + // In older browsers and test environments, fallback to console.error. + // eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args + console.error(error); + }; + export const isPrimaryRenderer = true; export const warnsIfNotActing = true; // This initialization code may run even on server environments @@ -1070,6 +1082,8 @@ export function didNotFindHydratableSuspenseInstance( export function errorHydratingContainer(parentContainer: Container): void { if (__DEV__) { + // TODO: This gets logged by onRecoverableError, too, so we should be + // able to remove it. console.error( 'An error occurred during hydration. The server HTML was replaced with client content in <%s>.', parentContainer.nodeName.toLowerCase(), diff --git a/packages/react-dom/src/client/ReactDOMLegacy.js b/packages/react-dom/src/client/ReactDOMLegacy.js index 05ae0d5ce4f45..cb95101401ea4 100644 --- a/packages/react-dom/src/client/ReactDOMLegacy.js +++ b/packages/react-dom/src/client/ReactDOMLegacy.js @@ -122,6 +122,7 @@ function legacyCreateRootFromDOMContainer( false, // isStrictMode false, // concurrentUpdatesByDefaultOverride, '', // identifierPrefix + null, ); markContainerAsRoot(root.current, container); diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 800eeba0d018d..fe6b6ee31f773 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -24,6 +24,7 @@ export type CreateRootOptions = { unstable_strictMode?: boolean, unstable_concurrentUpdatesByDefault?: boolean, identifierPrefix?: string, + onRecoverableError?: (error: mixed) => void, ... }; @@ -36,6 +37,7 @@ export type HydrateRootOptions = { unstable_strictMode?: boolean, unstable_concurrentUpdatesByDefault?: boolean, identifierPrefix?: string, + onRecoverableError?: (error: mixed) => void, ... }; @@ -143,6 +145,7 @@ export function createRoot( let isStrictMode = false; let concurrentUpdatesByDefaultOverride = false; let identifierPrefix = ''; + let onRecoverableError = null; if (options !== null && options !== undefined) { if (__DEV__) { if ((options: any).hydrate) { @@ -163,6 +166,9 @@ export function createRoot( if (options.identifierPrefix !== undefined) { identifierPrefix = options.identifierPrefix; } + if (options.onRecoverableError !== undefined) { + onRecoverableError = options.onRecoverableError; + } } const root = createContainer( @@ -173,6 +179,7 @@ export function createRoot( isStrictMode, concurrentUpdatesByDefaultOverride, identifierPrefix, + onRecoverableError, ); markContainerAsRoot(root.current, container); @@ -213,6 +220,7 @@ export function hydrateRoot( let isStrictMode = false; let concurrentUpdatesByDefaultOverride = false; let identifierPrefix = ''; + let onRecoverableError = null; if (options !== null && options !== undefined) { if (options.unstable_strictMode === true) { isStrictMode = true; @@ -226,6 +234,9 @@ export function hydrateRoot( if (options.identifierPrefix !== undefined) { identifierPrefix = options.identifierPrefix; } + if (options.onRecoverableError !== undefined) { + onRecoverableError = options.onRecoverableError; + } } const root = createContainer( @@ -236,6 +247,7 @@ export function hydrateRoot( isStrictMode, concurrentUpdatesByDefaultOverride, identifierPrefix, + onRecoverableError, ); markContainerAsRoot(root.current, container); // This can't be a comment node since hydration doesn't work on comment nodes anyway. diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index bf7754d6099c2..25cbd31b9fdf5 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -214,6 +214,7 @@ function render( false, null, '', + null, ); roots.set(containerTag, root); } diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 727b782efd768..bec60bff22c99 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -525,3 +525,7 @@ export function preparePortalMount(portalInstance: Instance): void { export function detachDeletedInstance(node: Instance): void { // noop } + +export function logRecoverableError(error: mixed): void { + // noop +} diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 10c5e37f41bcc..e1b1d25f5f60e 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -513,3 +513,7 @@ export function preparePortalMount(portalInstance: Instance): void { export function detachDeletedInstance(node: Instance): void { // noop } + +export function logRecoverableError(error: mixed): void { + // noop +} diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index fb539d8996811..c5d4318311c0b 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -210,6 +210,7 @@ function render( false, null, '', + null, ); roots.set(containerTag, root); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index ef76b6610617f..7e1ee5c57581b 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -466,6 +466,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, detachDeletedInstance() {}, + + logRecoverableError() { + // no-op + }, }; const hostConfig = useMutation @@ -954,7 +958,16 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { if (!root) { const container = {rootID: rootID, pendingChildren: [], children: []}; rootContainers.set(rootID, container); - root = NoopRenderer.createContainer(container, tag, false, null, null); + root = NoopRenderer.createContainer( + container, + tag, + false, + null, + null, + false, + '', + null, + ); roots.set(rootID, root); } return root.current.stateNode.containerInfo; @@ -975,6 +988,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { null, false, '', + null, ); return { _Scheduler: Scheduler, @@ -1004,6 +1018,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { null, false, '', + null, ); return { _Scheduler: Scheduler, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index e8ec41d9d81f5..39f724c76df92 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -131,6 +131,7 @@ import { resetHydrationState, getIsHydrating, hasUnhydratedTailNodes, + upgradeHydrationErrorsToRecoverable, } from './ReactFiberHydrationContext.new'; import { enableSuspenseCallback, @@ -1099,6 +1100,12 @@ function completeWork( return null; } } + + // Successfully completed this tree. If this was a forced client render, + // there may have been recoverable errors during first hydration + // attempt. If so, add them to a queue so we can log them in the + // commit phase. + upgradeHydrationErrorsToRecoverable(); } if ((workInProgress.flags & DidCapture) !== NoFlags) { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js index 0a0273470a702..c7baf36d16c47 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js @@ -131,6 +131,7 @@ import { resetHydrationState, getIsHydrating, hasUnhydratedTailNodes, + upgradeHydrationErrorsToRecoverable, } from './ReactFiberHydrationContext.old'; import { enableSuspenseCallback, @@ -1099,6 +1100,12 @@ function completeWork( return null; } } + + // Successfully completed this tree. If this was a forced client render, + // there may have been recoverable errors during first hydration + // attempt. If so, add them to a queue so we can log them in the + // commit phase. + upgradeHydrationErrorsToRecoverable(); } if ((workInProgress.flags & DidCapture) !== NoFlags) { diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index b6153eddccebb..4a460d584a53d 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -77,6 +77,7 @@ import { getSuspendedTreeContext, restoreSuspendedTreeContext, } from './ReactFiberTreeContext.new'; +import {queueRecoverableErrors} from './ReactFiberWorkLoop.new'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -84,6 +85,9 @@ let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; +// Hydration errors that were thrown inside this boundary +let hydrationErrors: Array | null = null; + function warnIfHydrating() { if (__DEV__) { if (isHydrating) { @@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean { ); hydrationParentFiber = fiber; isHydrating = true; + hydrationErrors = null; return true; } @@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( ); hydrationParentFiber = fiber; isHydrating = true; + hydrationErrors = null; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -601,10 +607,28 @@ function resetHydrationState(): void { isHydrating = false; } +export function upgradeHydrationErrorsToRecoverable(): void { + if (hydrationErrors !== null) { + // Successfully completed a forced client render. The errors that occurred + // during the hydration attempt are now recovered. We will log them in + // commit phase, once the entire tree has finished. + queueRecoverableErrors(hydrationErrors); + hydrationErrors = null; + } +} + function getIsHydrating(): boolean { return isHydrating; } +export function queueHydrationError(error: mixed): void { + if (hydrationErrors === null) { + hydrationErrors = [error]; + } else { + hydrationErrors.push(error); + } +} + export { warnIfHydrating, enterHydrationState, diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 9e2518542454a..3ee040237829a 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -77,6 +77,7 @@ import { getSuspendedTreeContext, restoreSuspendedTreeContext, } from './ReactFiberTreeContext.old'; +import {queueRecoverableErrors} from './ReactFiberWorkLoop.old'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -84,6 +85,9 @@ let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; +// Hydration errors that were thrown inside this boundary +let hydrationErrors: Array | null = null; + function warnIfHydrating() { if (__DEV__) { if (isHydrating) { @@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean { ); hydrationParentFiber = fiber; isHydrating = true; + hydrationErrors = null; return true; } @@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( ); hydrationParentFiber = fiber; isHydrating = true; + hydrationErrors = null; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -601,10 +607,28 @@ function resetHydrationState(): void { isHydrating = false; } +export function upgradeHydrationErrorsToRecoverable(): void { + if (hydrationErrors !== null) { + // Successfully completed a forced client render. The errors that occurred + // during the hydration attempt are now recovered. We will log them in + // commit phase, once the entire tree has finished. + queueRecoverableErrors(hydrationErrors); + hydrationErrors = null; + } +} + function getIsHydrating(): boolean { return isHydrating; } +export function queueHydrationError(error: mixed): void { + if (hydrationErrors === null) { + hydrationErrors = [error]; + } else { + hydrationErrors.push(error); + } +} + export { warnIfHydrating, enterHydrationState, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index 1cf200e35d56d..0c99b9ec3077d 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -245,6 +245,7 @@ export function createContainer( isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, identifierPrefix: string, + onRecoverableError: null | ((error: mixed) => void), ): OpaqueRoot { return createFiberRoot( containerInfo, @@ -254,6 +255,7 @@ export function createContainer( isStrictMode, concurrentUpdatesByDefaultOverride, identifierPrefix, + onRecoverableError, ); } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index 4b02959ab0840..202d7ce3819dc 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -245,6 +245,7 @@ export function createContainer( isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, identifierPrefix: string, + onRecoverableError: null | ((error: mixed) => void), ): OpaqueRoot { return createFiberRoot( containerInfo, @@ -254,6 +255,7 @@ export function createContainer( isStrictMode, concurrentUpdatesByDefaultOverride, identifierPrefix, + onRecoverableError, ); } diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index 9e9feb45d9b03..85820dc7a3cdf 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -30,7 +30,13 @@ import {initializeUpdateQueue} from './ReactUpdateQueue.new'; import {LegacyRoot, ConcurrentRoot} from './ReactRootTags'; import {createCache, retainCache} from './ReactFiberCacheComponent.new'; -function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) { +function FiberRootNode( + containerInfo, + tag, + hydrate, + identifierPrefix, + onRecoverableError, +) { this.tag = tag; this.containerInfo = containerInfo; this.pendingChildren = null; @@ -57,6 +63,7 @@ function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) { this.entanglements = createLaneMap(NoLanes); this.identifierPrefix = identifierPrefix; + this.onRecoverableError = onRecoverableError; if (enableCache) { this.pooledCache = null; @@ -103,13 +110,19 @@ export function createFiberRoot( hydrationCallbacks: null | SuspenseHydrationCallbacks, isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, + // TODO: We have several of these arguments that are conceptually part of the + // host config, but because they are passed in at runtime, we have to thread + // them through the root constructor. Perhaps we should put them all into a + // single type, like a DynamicHostConfig that is defined by the renderer. identifierPrefix: string, + onRecoverableError: null | ((error: mixed) => void), ): FiberRoot { const root: FiberRoot = (new FiberRootNode( containerInfo, tag, hydrate, identifierPrefix, + onRecoverableError, ): any); if (enableSuspenseCallback) { root.hydrationCallbacks = hydrationCallbacks; diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index d8d061297854f..e1eaee798bfb2 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -30,7 +30,13 @@ import {initializeUpdateQueue} from './ReactUpdateQueue.old'; import {LegacyRoot, ConcurrentRoot} from './ReactRootTags'; import {createCache, retainCache} from './ReactFiberCacheComponent.old'; -function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) { +function FiberRootNode( + containerInfo, + tag, + hydrate, + identifierPrefix, + onRecoverableError, +) { this.tag = tag; this.containerInfo = containerInfo; this.pendingChildren = null; @@ -57,6 +63,7 @@ function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) { this.entanglements = createLaneMap(NoLanes); this.identifierPrefix = identifierPrefix; + this.onRecoverableError = onRecoverableError; if (enableCache) { this.pooledCache = null; @@ -103,13 +110,19 @@ export function createFiberRoot( hydrationCallbacks: null | SuspenseHydrationCallbacks, isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, + // TODO: We have several of these arguments that are conceptually part of the + // host config, but because they are passed in at runtime, we have to thread + // them through the root constructor. Perhaps we should put them all into a + // single type, like a DynamicHostConfig that is defined by the renderer. identifierPrefix: string, + onRecoverableError: null | ((error: mixed) => void), ): FiberRoot { const root: FiberRoot = (new FiberRootNode( containerInfo, tag, hydrate, identifierPrefix, + onRecoverableError, ): any); if (enableSuspenseCallback) { root.hydrationCallbacks = hydrationCallbacks; diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index cd9931687ba5a..6a0c70f8e6d00 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -79,7 +79,10 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.new'; -import {getIsHydrating} from './ReactFiberHydrationContext.new'; +import { + getIsHydrating, + queueHydrationError, +} from './ReactFiberHydrationContext.new'; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; @@ -507,6 +510,10 @@ function throwException( root, rootRenderLanes, ); + + // Even though the user may not be affected by this error, we should + // still log it so it can be fixed. + queueHydrationError(value); return; } } else { @@ -517,7 +524,7 @@ function throwException( // We didn't find a boundary that could handle this type of exception. Start // over and traverse parent path again, this time treating the exception // as an error. - renderDidError(); + renderDidError(value); value = createCapturedValue(value, sourceFiber); let workInProgress = returnFiber; diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 8f6d18a48dea3..21ab03f4ac925 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -79,7 +79,10 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.old'; -import {getIsHydrating} from './ReactFiberHydrationContext.old'; +import { + getIsHydrating, + queueHydrationError, +} from './ReactFiberHydrationContext.old'; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; @@ -507,6 +510,10 @@ function throwException( root, rootRenderLanes, ); + + // Even though the user may not be affected by this error, we should + // still log it so it can be fixed. + queueHydrationError(value); return; } } else { @@ -517,7 +524,7 @@ function throwException( // We didn't find a boundary that could handle this type of exception. Start // over and traverse parent path again, this time treating the exception // as an error. - renderDidError(); + renderDidError(value); value = createCapturedValue(value, sourceFiber); let workInProgress = returnFiber; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index d5372999f3c46..f3efac3e74d68 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -14,6 +14,7 @@ import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; import type {StackCursor} from './ReactFiberStack.new'; import type {Flags} from './ReactFiberFlags'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; +import type {EventPriority} from './ReactEventPriorities.new'; import { warnAboutDeprecatedLifecycles, @@ -76,6 +77,7 @@ import { supportsMicrotasks, errorHydratingContainer, scheduleMicrotask, + logRecoverableError, } from './ReactFiberHostConfig'; import { @@ -296,6 +298,11 @@ let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes; let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; +// Errors that are thrown during the render phase. +let workInProgressRootConcurrentErrors: Array | null = null; +// These are errors that we recovered from without surfacing them to the UI. +// We will log them once the tree commits. +let workInProgressRootRecoverableErrors: Array | null = null; // The most recent time we committed a fallback. This lets us ensure a train // model where we don't commit new loading states in too quick succession. @@ -894,13 +901,36 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } } + const errorsFromFirstAttempt = workInProgressRootConcurrentErrors; const exitStatus = renderRootSync(root, errorRetryLanes); + if (exitStatus !== RootErrored) { + // Successfully finished rendering on retry + if (errorsFromFirstAttempt !== null) { + // The errors from the failed first attempt have been recovered. Add + // them to the collection of recoverable errors. We'll log them in the + // commit phase. + queueRecoverableErrors(errorsFromFirstAttempt); + } + } else { + // The UI failed to recover. + } executionContext = prevExecutionContext; return exitStatus; } +export function queueRecoverableErrors(errors: Array) { + if (workInProgressRootConcurrentErrors === null) { + workInProgressRootRecoverableErrors = errors; + } else { + workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( + null, + errors, + ); + } +} + function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: @@ -913,7 +943,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootErrored: { // We should have already attempted to retry this tree. If we reached // this point, it errored again. Commit it. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootSuspended: { @@ -953,14 +983,14 @@ function finishConcurrentRender(root, exitStatus, lanes) { // lower priority work to do. Instead of committing the fallback // immediately, wait for more data to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), + commitRoot.bind(null, root, workInProgressRootRecoverableErrors), msUntilTimeout, ); break; } } // The work expired. Commit immediately. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootSuspendedWithDelay: { @@ -991,7 +1021,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { // Instead of committing the fallback immediately, wait for more data // to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), + commitRoot.bind(null, root, workInProgressRootRecoverableErrors), msUntilTimeout, ); break; @@ -999,12 +1029,12 @@ function finishConcurrentRender(root, exitStatus, lanes) { } // Commit the placeholder. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootCompleted: { // The work completed. Ready to commit. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } default: { @@ -1124,7 +1154,7 @@ function performSyncWorkOnRoot(root) { const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); // Before exiting, make sure there's a callback scheduled for the next // pending level. @@ -1320,6 +1350,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootInterleavedUpdatedLanes = NoLanes; workInProgressRootRenderPhaseUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; + workInProgressRootConcurrentErrors = null; + workInProgressRootRecoverableErrors = null; enqueueInterleavedUpdates(); @@ -1474,10 +1506,15 @@ export function renderDidSuspendDelayIfPossible(): void { } } -export function renderDidError() { +export function renderDidError(error: mixed) { if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { workInProgressRootExitStatus = RootErrored; } + if (workInProgressRootConcurrentErrors === null) { + workInProgressRootConcurrentErrors = [error]; + } else { + workInProgressRootConcurrentErrors.push(error); + } } // Called during render to determine if anything has suspended. @@ -1781,7 +1818,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } } -function commitRoot(root) { +function commitRoot(root: FiberRoot, recoverableErrors: null | Array) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); @@ -1789,7 +1826,7 @@ function commitRoot(root) { try { ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); - commitRootImpl(root, previousUpdateLanePriority); + commitRootImpl(root, recoverableErrors, previousUpdateLanePriority); } finally { ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); @@ -1798,7 +1835,11 @@ function commitRoot(root) { return null; } -function commitRootImpl(root, renderPriorityLevel) { +function commitRootImpl( + root: FiberRoot, + recoverableErrors: null | Array, + renderPriorityLevel: EventPriority, +) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which // means `flushPassiveEffects` will sometimes result in additional @@ -2069,6 +2110,22 @@ function commitRootImpl(root, renderPriorityLevel) { // additional work on this root is scheduled. ensureRootIsScheduled(root, now()); + if (recoverableErrors !== null) { + // There were errors during this render, but recovered from them without + // needing to surface it to the UI. We log them here. + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + const onRecoverableError = root.onRecoverableError; + if (onRecoverableError !== null) { + onRecoverableError(recoverableError); + } else { + // No user-provided onRecoverableError. Use the default behavior + // provided by the renderer's host config. + logRecoverableError(recoverableError); + } + } + } + if (hasUncaughtError) { hasUncaughtError = false; const error = firstUncaughtError; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 92be9f0a323a9..d6e37f2785ff2 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -14,6 +14,7 @@ import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; import type {StackCursor} from './ReactFiberStack.old'; import type {Flags} from './ReactFiberFlags'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; +import type {EventPriority} from './ReactEventPriorities.old'; import { warnAboutDeprecatedLifecycles, @@ -76,6 +77,7 @@ import { supportsMicrotasks, errorHydratingContainer, scheduleMicrotask, + logRecoverableError, } from './ReactFiberHostConfig'; import { @@ -296,6 +298,11 @@ let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes; let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; +// Errors that are thrown during the render phase. +let workInProgressRootConcurrentErrors: Array | null = null; +// These are errors that we recovered from without surfacing them to the UI. +// We will log them once the tree commits. +let workInProgressRootRecoverableErrors: Array | null = null; // The most recent time we committed a fallback. This lets us ensure a train // model where we don't commit new loading states in too quick succession. @@ -894,13 +901,36 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } } + const errorsFromFirstAttempt = workInProgressRootConcurrentErrors; const exitStatus = renderRootSync(root, errorRetryLanes); + if (exitStatus !== RootErrored) { + // Successfully finished rendering on retry + if (errorsFromFirstAttempt !== null) { + // The errors from the failed first attempt have been recovered. Add + // them to the collection of recoverable errors. We'll log them in the + // commit phase. + queueRecoverableErrors(errorsFromFirstAttempt); + } + } else { + // The UI failed to recover. + } executionContext = prevExecutionContext; return exitStatus; } +export function queueRecoverableErrors(errors: Array) { + if (workInProgressRootConcurrentErrors === null) { + workInProgressRootRecoverableErrors = errors; + } else { + workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( + null, + errors, + ); + } +} + function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: @@ -913,7 +943,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootErrored: { // We should have already attempted to retry this tree. If we reached // this point, it errored again. Commit it. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootSuspended: { @@ -953,14 +983,14 @@ function finishConcurrentRender(root, exitStatus, lanes) { // lower priority work to do. Instead of committing the fallback // immediately, wait for more data to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), + commitRoot.bind(null, root, workInProgressRootRecoverableErrors), msUntilTimeout, ); break; } } // The work expired. Commit immediately. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootSuspendedWithDelay: { @@ -991,7 +1021,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { // Instead of committing the fallback immediately, wait for more data // to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), + commitRoot.bind(null, root, workInProgressRootRecoverableErrors), msUntilTimeout, ); break; @@ -999,12 +1029,12 @@ function finishConcurrentRender(root, exitStatus, lanes) { } // Commit the placeholder. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } case RootCompleted: { // The work completed. Ready to commit. - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); break; } default: { @@ -1124,7 +1154,7 @@ function performSyncWorkOnRoot(root) { const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; - commitRoot(root); + commitRoot(root, workInProgressRootRecoverableErrors); // Before exiting, make sure there's a callback scheduled for the next // pending level. @@ -1320,6 +1350,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootInterleavedUpdatedLanes = NoLanes; workInProgressRootRenderPhaseUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; + workInProgressRootConcurrentErrors = null; + workInProgressRootRecoverableErrors = null; enqueueInterleavedUpdates(); @@ -1474,10 +1506,15 @@ export function renderDidSuspendDelayIfPossible(): void { } } -export function renderDidError() { +export function renderDidError(error: mixed) { if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { workInProgressRootExitStatus = RootErrored; } + if (workInProgressRootConcurrentErrors === null) { + workInProgressRootConcurrentErrors = [error]; + } else { + workInProgressRootConcurrentErrors.push(error); + } } // Called during render to determine if anything has suspended. @@ -1781,7 +1818,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } } -function commitRoot(root) { +function commitRoot(root: FiberRoot, recoverableErrors: null | Array) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); @@ -1789,7 +1826,7 @@ function commitRoot(root) { try { ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); - commitRootImpl(root, previousUpdateLanePriority); + commitRootImpl(root, recoverableErrors, previousUpdateLanePriority); } finally { ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); @@ -1798,7 +1835,11 @@ function commitRoot(root) { return null; } -function commitRootImpl(root, renderPriorityLevel) { +function commitRootImpl( + root: FiberRoot, + recoverableErrors: null | Array, + renderPriorityLevel: EventPriority, +) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which // means `flushPassiveEffects` will sometimes result in additional @@ -2069,6 +2110,22 @@ function commitRootImpl(root, renderPriorityLevel) { // additional work on this root is scheduled. ensureRootIsScheduled(root, now()); + if (recoverableErrors !== null) { + // There were errors during this render, but recovered from them without + // needing to surface it to the UI. We log them here. + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + const onRecoverableError = root.onRecoverableError; + if (onRecoverableError !== null) { + onRecoverableError(recoverableError); + } else { + // No user-provided onRecoverableError. Use the default behavior + // provided by the renderer's host config. + logRecoverableError(recoverableError); + } + } + } + if (hasUncaughtError) { hasUncaughtError = false; const error = firstUncaughtError; diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 13965720b7cd3..bbfe70c26cfc4 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -246,6 +246,8 @@ type BaseFiberRootProperties = {| // the public createRoot object, which the fiber tree does not currently have // a reference to. identifierPrefix: string, + + onRecoverableError: null | ((error: mixed) => void), |}; // The following attributes are only used by DevTools and are only present in DEV builds. diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 4bf292df79f7a..d0c3d5b236ea4 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -76,6 +76,7 @@ describe('ReactFiberHostContext', () => { null, false, '', + null, ); act(() => { Renderer.updateContainer( @@ -139,6 +140,7 @@ describe('ReactFiberHostContext', () => { null, false, '', + null, ); act(() => { Renderer.updateContainer( diff --git a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js index 4c13ef7c35dfa..16ebb4657d28d 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js +++ b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js @@ -205,6 +205,9 @@ describe('useMutableSourceHydration', () => { act(() => { ReactDOM.hydrateRoot(container, , { mutableSources: [mutableSource], + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log error: ' + error.message); + }, }); source.value = 'two'; @@ -254,11 +257,17 @@ describe('useMutableSourceHydration', () => { React.startTransition(() => { ReactDOM.hydrateRoot(container, , { mutableSources: [mutableSource], + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log error: ' + error.message); + }, }); }); } else { ReactDOM.hydrateRoot(container, , { mutableSources: [mutableSource], + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log error: ' + error.message); + }, }); } expect(Scheduler).toFlushAndYieldThrough(['a:one']); @@ -269,7 +278,17 @@ describe('useMutableSourceHydration', () => { 'The server HTML was replaced with client content in
.', {withoutStack: true}, ); - expect(Scheduler).toHaveYielded(['a:two', 'b:two']); + expect(Scheduler).toHaveYielded([ + 'a:two', + 'b:two', + // TODO: Before onRecoverableError, this error was never surfaced to the + // user. The request to file an bug report no longer makes sense. + // However, the experimental useMutableSource API is slated for + // removal, anyway. + 'Log error: Cannot read from mutable source during the current ' + + 'render without tearing. This may be a bug in React. Please file ' + + 'an issue.', + ]); expect(source.listenerCount).toBe(2); }); @@ -328,11 +347,17 @@ describe('useMutableSourceHydration', () => { React.startTransition(() => { ReactDOM.hydrateRoot(container, fragment, { mutableSources: [mutableSource], + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log error: ' + error.message); + }, }); }); } else { ReactDOM.hydrateRoot(container, fragment, { mutableSources: [mutableSource], + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log error: ' + error.message); + }, }); } expect(Scheduler).toFlushAndYieldThrough(['0:a:one']); @@ -343,7 +368,17 @@ describe('useMutableSourceHydration', () => { 'The server HTML was replaced with client content in
.', {withoutStack: true}, ); - expect(Scheduler).toHaveYielded(['0:a:one', '1:b:two']); + expect(Scheduler).toHaveYielded([ + '0:a:one', + '1:b:two', + // TODO: Before onRecoverableError, this error was never surfaced to the + // user. The request to file an bug report no longer makes sense. + // However, the experimental useMutableSource API is slated for + // removal, anyway. + 'Log error: Cannot read from mutable source during the current ' + + 'render without tearing. This may be a bug in React. Please file ' + + 'an issue.', + ]); }); // @gate !enableSyncDefaultUpdates diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 6535d8d3fdec3..ff5bbf8035f07 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -68,6 +68,7 @@ export const prepareScopeUpdate = $$$hostConfig.preparePortalMount; export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope; export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority; export const detachDeletedInstance = $$$hostConfig.detachDeletedInstance; +export const logRecoverableError = $$$hostConfig.logRecoverableError; // ------------------- // Microtasks diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 5ce012a0f538a..2cf1e5c2fb31d 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -401,7 +401,7 @@ function popComponentStackInDEV(task: Task): void { } } -function reportError(request: Request, error: mixed): void { +function logRecoverableError(request: Request, error: mixed): void { // If this callback errors, we intentionally let that error bubble up to become a fatal error // so that someone fixes the error reporting instead of hiding it. const onError = request.onError; @@ -484,7 +484,7 @@ function renderSuspenseBoundary( } } catch (error) { contentRootSegment.status = ERRORED; - reportError(request, error); + logRecoverableError(request, error); newBoundary.forceClientRender = true; // We don't need to decrement any task numbers because we didn't spawn any new task. // We don't need to schedule any task because we know the parent has written yet. @@ -1337,7 +1337,7 @@ function erroredTask( error: mixed, ) { // Report the error to a global handler. - reportError(request, error); + logRecoverableError(request, error); if (boundary === null) { fatalError(request, error); } else { @@ -1557,7 +1557,7 @@ export function performWork(request: Request): void { flushCompletedQueues(request, request.destination); } } catch (error) { - reportError(request, error); + logRecoverableError(request, error); fatalError(request, error); } finally { setCurrentResponseState(prevResponseState); @@ -1945,7 +1945,7 @@ export function startFlowing(request: Request, destination: Destination): void { try { flushCompletedQueues(request, destination); } catch (error) { - reportError(request, error); + logRecoverableError(request, error); fatalError(request, error); } } @@ -1960,7 +1960,7 @@ export function abort(request: Request): void { flushCompletedQueues(request, request.destination); } } catch (error) { - reportError(request, error); + logRecoverableError(request, error); fatalError(request, error); } } diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 418fbb8241a66..052fa730fd91e 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -421,7 +421,7 @@ export function resolveModelToJSON( x.then(ping, ping); return serializeByRefID(newSegment.id); } else { - reportError(request, x); + logRecoverableError(request, x); // Something errored. We'll still send everything we have up until this point. // We'll replace this element with a lazy reference that throws on the client // once it gets rendered. @@ -604,7 +604,7 @@ export function resolveModelToJSON( ); } -function reportError(request: Request, error: mixed): void { +function logRecoverableError(request: Request, error: mixed): void { const onError = request.onError; onError(error); } @@ -687,7 +687,7 @@ function retrySegment(request: Request, segment: Segment): void { x.then(ping, ping); return; } else { - reportError(request, x); + logRecoverableError(request, x); // This errored, we need to serialize this error to the emitErrorChunk(request, segment.id, x); } @@ -711,7 +711,7 @@ function performWork(request: Request): void { flushCompletedChunks(request, request.destination); } } catch (error) { - reportError(request, error); + logRecoverableError(request, error); fatalError(request, error); } finally { ReactCurrentDispatcher.current = prevDispatcher; @@ -794,7 +794,7 @@ export function startFlowing(request: Request, destination: Destination): void { try { flushCompletedChunks(request, destination); } catch (error) { - reportError(request, error); + logRecoverableError(request, error); fatalError(request, error); } } diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 503b08efaf20b..840912db30886 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -314,3 +314,7 @@ export function getInstanceFromScope(scopeInstance: Object): null | Object { export function detachDeletedInstance(node: Instance): void { // noop } + +export function logRecoverableError(error: mixed): void { + // noop +} diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index de6e4beffec5f..a8121d1a14fcf 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -472,6 +472,7 @@ function create(element: React$Element, options: TestRendererOptions) { isStrictMode, concurrentUpdatesByDefault, '', + null, ); if (root == null) { diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index f44d4ed7fcd1b..4294964a74b98 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -19,6 +19,7 @@ declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: any; /*?{ };*/ declare var queueMicrotask: (fn: Function) => void; +declare var reportError: (error: mixed) => void; declare module 'create-react-class' { declare var exports: React$CreateClass; diff --git a/scripts/print-warnings/print-warnings.js b/scripts/print-warnings/print-warnings.js index 8e23dd880e92b..52fcb9630e1fa 100644 --- a/scripts/print-warnings/print-warnings.js +++ b/scripts/print-warnings/print-warnings.js @@ -67,12 +67,10 @@ function transform(file, enc, cb) { const warningMsgLiteral = evalStringConcat(node.arguments[0]); warnings.add(JSON.stringify(warningMsgLiteral)); } catch (error) { - console.error( - 'Failed to extract warning message from', - file.path - ); - console.error(astPath.node.loc); - throw error; + // Silently skip over this call. We have a lint rule to enforce + // that all calls are extractable, so if this one fails, assume + // it's intentional. + return; } } }, diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 45b7a41e7e596..802141d6bc101 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -31,6 +31,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // Flight Uint8Array: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index 7135a5a706cff..b579566145778 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -30,6 +30,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // Flight Uint8Array: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.esm.js b/scripts/rollup/validate/eslintrc.esm.js index 34cde555478ab..23eb027071a23 100644 --- a/scripts/rollup/validate/eslintrc.esm.js +++ b/scripts/rollup/validate/eslintrc.esm.js @@ -30,6 +30,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // Flight Uint8Array: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index d267ca2bec308..2f8740049236c 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -31,6 +31,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // Flight Uint8Array: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index e466784e8fcb3..21d8397487ff2 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -31,6 +31,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // jest jest: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.umd.js b/scripts/rollup/validate/eslintrc.umd.js index 2d274375cca8c..223e96d28ef58 100644 --- a/scripts/rollup/validate/eslintrc.umd.js +++ b/scripts/rollup/validate/eslintrc.umd.js @@ -36,6 +36,7 @@ module.exports = { ArrayBuffer: 'readonly', TaskController: 'readonly', + reportError: 'readonly', // Flight Uint8Array: 'readonly',