Skip to content

Commit

Permalink
Add onRecoverableError option to hydrateRoot, createRoot (#23207)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
acdlite authored Feb 4, 2022
1 parent 5318971 commit 848e802
Show file tree
Hide file tree
Showing 39 changed files with 530 additions and 62 deletions.
4 changes: 4 additions & 0 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,7 @@ export function preparePortalMount(portalInstance: any): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logRecoverableError(error) {
// noop
}
118 changes: 109 additions & 9 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1723,12 +1723,22 @@ describe('ReactDOMFizzServer', () => {
});
expect(Scheduler).toHaveYielded(['server']);

ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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},
Expand Down Expand Up @@ -1805,13 +1815,23 @@ describe('ReactDOMFizzServer', () => {
});
expect(Scheduler).toHaveYielded(['server']);

ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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},
Expand Down Expand Up @@ -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, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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(
<div>
<span />
Expand Down Expand Up @@ -1975,8 +2001,16 @@ describe('ReactDOMFizzServer', () => {

// Hydrate the tree. Child will throw during render.
isClient = true;
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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!');
},
Expand Down Expand Up @@ -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, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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(
<div>
<span />
Expand Down Expand Up @@ -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 (
<>
<A />
<B />
</>
);
}

const root = ReactDOM.createRoot(container, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Logged a recoverable error: ' + error.message,
);
},
});
React.startTransition(() => {
root.render(<App />);
});

// 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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) {
Scheduler.unstable_flushAll();
} else {
Expand Down Expand Up @@ -290,7 +294,11 @@ describe('ReactDOMServerPartialHydration', () => {
suspend = true;
client = true;

ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
expect(Scheduler).toFlushAndYield([
'Suspend',
'Component',
Expand All @@ -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
Expand Down Expand Up @@ -573,9 +585,19 @@ describe('ReactDOMServerPartialHydration', () => {

expect(() => {
act(() => {
ReactDOM.hydrateRoot(container, <App hasB={false} />);
ReactDOM.hydrateRoot(container, <App hasB={false} />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
});
}).toErrorDev('Did not expect server HTML to contain a <span> in <div>');
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('<span>A</span>');
expect(container.innerHTML).not.toContain('<span>B</span>');
Expand Down Expand Up @@ -2997,7 +3019,13 @@ describe('ReactDOMServerPartialHydration', () => {
const span = container.getElementsByTagName('span')[0];
expect(span.innerHTML).toBe('Hidden child');

ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Log recoverable error: ' + error.message,
);
},
});

Scheduler.unstable_flushAll();
expect(ref.current).toBe(span);
Expand Down Expand Up @@ -3142,13 +3170,27 @@ describe('ReactDOMServerPartialHydration', () => {

expect(() => {
act(() => {
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
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 <div>.',
{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(
Expand Down
14 changes: 14 additions & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ function legacyCreateRootFromDOMContainer(
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
null,
);
markContainerAsRoot(root.current, container);

Expand Down
12 changes: 12 additions & 0 deletions packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type CreateRootOptions = {
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
identifierPrefix?: string,
onRecoverableError?: (error: mixed) => void,
...
};

Expand All @@ -36,6 +37,7 @@ export type HydrateRootOptions = {
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
identifierPrefix?: string,
onRecoverableError?: (error: mixed) => void,
...
};

Expand Down Expand Up @@ -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) {
Expand All @@ -163,6 +166,9 @@ export function createRoot(
if (options.identifierPrefix !== undefined) {
identifierPrefix = options.identifierPrefix;
}
if (options.onRecoverableError !== undefined) {
onRecoverableError = options.onRecoverableError;
}
}

const root = createContainer(
Expand All @@ -173,6 +179,7 @@ export function createRoot(
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
onRecoverableError,
);
markContainerAsRoot(root.current, container);

Expand Down Expand Up @@ -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;
Expand All @@ -226,6 +234,9 @@ export function hydrateRoot(
if (options.identifierPrefix !== undefined) {
identifierPrefix = options.identifierPrefix;
}
if (options.onRecoverableError !== undefined) {
onRecoverableError = options.onRecoverableError;
}
}

const root = createContainer(
Expand All @@ -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.
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ function render(
false,
null,
'',
null,
);
roots.set(containerTag, root);
}
Expand Down
Loading

0 comments on commit 848e802

Please sign in to comment.