Skip to content

Commit fe8c95f

Browse files
acdliteacdlite
committed
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.
1 parent 2a28e76 commit fe8c95f

15 files changed

+225
-50
lines changed

‎packages/react-art/src/ReactARTHostConfig.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,6 @@ export function detachDeletedInstance(node: Instance): void {
452452
// noop
453453
}
454454

455-
export function logHydrationError(config, error) {
455+
export function logRecoverableError(config, error) {
456456
// noop
457457
}

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,7 @@ describe('ReactDOMFizzServer', () => {
18981898
// falls back to client rendering.
18991899
isClient = true;
19001900
ReactDOM.hydrateRoot(container, <App />, {
1901-
onHydrationError(error) {
1901+
onRecoverableError(error) {
19021902
Scheduler.unstable_yieldValue(error.message);
19031903
},
19041904
});
@@ -1982,7 +1982,7 @@ describe('ReactDOMFizzServer', () => {
19821982
// Hydrate the tree. Child will throw during render.
19831983
isClient = true;
19841984
ReactDOM.hydrateRoot(container, <App />, {
1985-
onHydrationError(error) {
1985+
onRecoverableError(error) {
19861986
// TODO: We logged a hydration error, but the same error ends up
19871987
// being thrown during the fallback to client rendering, too. Maybe
19881988
// we should only log if the client render succeeds.
@@ -2063,7 +2063,7 @@ describe('ReactDOMFizzServer', () => {
20632063
// falls back to client rendering.
20642064
isClient = true;
20652065
ReactDOM.hydrateRoot(container, <App />, {
2066-
onHydrationError(error) {
2066+
onRecoverableError(error) {
20672067
Scheduler.unstable_yieldValue(error.message);
20682068
},
20692069
});
@@ -2100,4 +2100,65 @@ describe('ReactDOMFizzServer', () => {
21002100
expect(span3Ref.current).toBe(span3);
21012101
},
21022102
);
2103+
2104+
// @gate experimental
2105+
it('logs regular (non-hydration) errors when the UI recovers', async () => {
2106+
let shouldThrow = true;
2107+
2108+
function A() {
2109+
if (shouldThrow) {
2110+
Scheduler.unstable_yieldValue('Oops!');
2111+
throw new Error('Oops!');
2112+
}
2113+
Scheduler.unstable_yieldValue('A');
2114+
return 'A';
2115+
}
2116+
2117+
function B() {
2118+
Scheduler.unstable_yieldValue('B');
2119+
return 'B';
2120+
}
2121+
2122+
function App() {
2123+
return (
2124+
<>
2125+
<A />
2126+
<B />
2127+
</>
2128+
);
2129+
}
2130+
2131+
const root = ReactDOM.createRoot(container, {
2132+
onRecoverableError(error) {
2133+
Scheduler.unstable_yieldValue(
2134+
'Logged a recoverable error: ' + error.message,
2135+
);
2136+
},
2137+
});
2138+
React.startTransition(() => {
2139+
root.render(<App />);
2140+
});
2141+
2142+
// Partially render A, but yield before the render has finished
2143+
expect(Scheduler).toFlushAndYieldThrough(['Oops!', 'Oops!']);
2144+
2145+
// React will try rendering again synchronously. During the retry, A will
2146+
// not throw. This simulates a concurrent data race that is fixed by
2147+
// blocking the main thread.
2148+
shouldThrow = false;
2149+
expect(Scheduler).toFlushAndYield([
2150+
// Finish initial render attempt
2151+
'B',
2152+
2153+
// Render again, synchronously
2154+
'A',
2155+
'B',
2156+
2157+
// Log the error
2158+
'Logged a recoverable error: Oops!',
2159+
]);
2160+
2161+
// UI looks normal
2162+
expect(container.textContent).toEqual('AB');
2163+
});
21032164
});

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ describe('ReactDOMServerPartialHydration', () => {
209209
// hydrating anyway.
210210
suspend = true;
211211
ReactDOM.hydrateRoot(container, <App />, {
212-
onHydrationError(error) {
212+
onRecoverableError(error) {
213213
Scheduler.unstable_yieldValue(error.message);
214214
},
215215
});
@@ -299,7 +299,7 @@ describe('ReactDOMServerPartialHydration', () => {
299299
client = true;
300300

301301
ReactDOM.hydrateRoot(container, <App />, {
302-
onHydrationError(error) {
302+
onRecoverableError(error) {
303303
Scheduler.unstable_yieldValue(error.message);
304304
},
305305
});
@@ -3052,13 +3052,27 @@ describe('ReactDOMServerPartialHydration', () => {
30523052

30533053
expect(() => {
30543054
act(() => {
3055-
ReactDOM.hydrateRoot(container, <App />);
3055+
ReactDOM.hydrateRoot(container, <App />, {
3056+
onRecoverableError(error) {
3057+
Scheduler.unstable_yieldValue(
3058+
'Log recoverable error: ' + error.message,
3059+
);
3060+
},
3061+
});
30563062
});
30573063
}).toErrorDev(
30583064
'Warning: An error occurred during hydration. ' +
30593065
'The server HTML was replaced with client content in <div>.',
30603066
{withoutStack: true},
30613067
);
3068+
expect(Scheduler).toHaveYielded([
3069+
'Log recoverable error: An error occurred during hydration. The server ' +
3070+
'HTML was replaced with client content',
3071+
// TODO: There were multiple mismatches in a single container. Should
3072+
// we attempt to de-dupe them?
3073+
'Log recoverable error: An error occurred during hydration. The server ' +
3074+
'HTML was replaced with client content',
3075+
]);
30623076

30633077
// We show fallback state when mismatch happens at root
30643078
expect(container.innerHTML).toEqual(

‎packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,15 +379,15 @@ export function getCurrentEventPriority(): * {
379379
return getEventPriority(currentEvent.type);
380380
}
381381

382-
export function logHydrationError(
382+
export function logRecoverableError(
383383
config: ErrorLoggingConfig,
384384
error: mixed,
385385
): void {
386-
const onHydrationError = config;
387-
if (onHydrationError !== null) {
386+
const onRecoverableError = config;
387+
if (onRecoverableError !== null) {
388388
// Schedule a callback to invoke the user-provided logging function.
389389
scheduleCallback(IdlePriority, () => {
390-
onHydrationError(error);
390+
onRecoverableError(error);
391391
});
392392
} else {
393393
// Default behavior is to rethrow the error in a separate task. This will
@@ -1094,6 +1094,8 @@ export function didNotFindHydratableSuspenseInstance(
10941094

10951095
export function errorHydratingContainer(parentContainer: Container): void {
10961096
if (__DEV__) {
1097+
// TODO: This gets logged by onRecoverableError, too, so we should be
1098+
// able to remove it.
10971099
console.error(
10981100
'An error occurred during hydration. The server HTML was replaced with client content in <%s>.',
10991101
parentContainer.nodeName.toLowerCase(),

‎packages/react-dom/src/client/ReactDOMRoot.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export type CreateRootOptions = {
2424
unstable_strictMode?: boolean,
2525
unstable_concurrentUpdatesByDefault?: boolean,
2626
identifierPrefix?: string,
27+
onRecoverableError?: (error: mixed) => void,
2728
...
2829
};
2930

@@ -36,7 +37,7 @@ export type HydrateRootOptions = {
3637
unstable_strictMode?: boolean,
3738
unstable_concurrentUpdatesByDefault?: boolean,
3839
identifierPrefix?: string,
39-
onHydrationError?: (error: mixed) => void,
40+
onRecoverableError?: (error: mixed) => void,
4041
...
4142
};
4243

@@ -144,6 +145,7 @@ export function createRoot(
144145
let isStrictMode = false;
145146
let concurrentUpdatesByDefaultOverride = false;
146147
let identifierPrefix = '';
148+
let onRecoverableError = null;
147149
if (options !== null && options !== undefined) {
148150
if (__DEV__) {
149151
if ((options: any).hydrate) {
@@ -164,6 +166,9 @@ export function createRoot(
164166
if (options.identifierPrefix !== undefined) {
165167
identifierPrefix = options.identifierPrefix;
166168
}
169+
if (options.onRecoverableError !== undefined) {
170+
onRecoverableError = options.onRecoverableError;
171+
}
167172
}
168173

169174
const root = createContainer(
@@ -174,7 +179,7 @@ export function createRoot(
174179
isStrictMode,
175180
concurrentUpdatesByDefaultOverride,
176181
identifierPrefix,
177-
null,
182+
onRecoverableError,
178183
);
179184
markContainerAsRoot(root.current, container);
180185

@@ -215,7 +220,7 @@ export function hydrateRoot(
215220
let isStrictMode = false;
216221
let concurrentUpdatesByDefaultOverride = false;
217222
let identifierPrefix = '';
218-
let onHydrationError = null;
223+
let onRecoverableError = null;
219224
if (options !== null && options !== undefined) {
220225
if (options.unstable_strictMode === true) {
221226
isStrictMode = true;
@@ -229,8 +234,8 @@ export function hydrateRoot(
229234
if (options.identifierPrefix !== undefined) {
230235
identifierPrefix = options.identifierPrefix;
231236
}
232-
if (options.onHydrationError !== undefined) {
233-
onHydrationError = options.onHydrationError;
237+
if (options.onRecoverableError !== undefined) {
238+
onRecoverableError = options.onRecoverableError;
234239
}
235240
}
236241

@@ -242,7 +247,7 @@ export function hydrateRoot(
242247
isStrictMode,
243248
concurrentUpdatesByDefaultOverride,
244249
identifierPrefix,
245-
onHydrationError,
250+
onRecoverableError,
246251
);
247252
markContainerAsRoot(root.current, container);
248253
// This can't be a comment node since hydration doesn't work on comment nodes anyway.

‎packages/react-native-renderer/src/ReactFabricHostConfig.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ export function detachDeletedInstance(node: Instance): void {
528528
// noop
529529
}
530530

531-
export function logHydrationError(
531+
export function logRecoverableError(
532532
config: ErrorLoggingConfig,
533533
error: mixed,
534534
): void {

‎packages/react-native-renderer/src/ReactNativeHostConfig.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ export function detachDeletedInstance(node: Instance): void {
516516
// noop
517517
}
518518

519-
export function logHydrationError(
519+
export function logRecoverableError(
520520
config: ErrorLoggingConfig,
521521
error: mixed,
522522
): void {

‎packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
467467

468468
detachDeletedInstance() {},
469469

470-
logHydrationError() {
470+
logRecoverableError() {
471471
// no-op
472472
},
473473
};

‎packages/react-reconciler/src/ReactFiberThrow.new.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {
3737
import {
3838
supportsPersistence,
3939
getOffscreenContainerProps,
40-
logHydrationError,
40+
logRecoverableError,
4141
} from './ReactFiberHostConfig';
4242
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
4343
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -515,7 +515,7 @@ function throwException(
515515
// probably want to log any error that is recovered from without
516516
// triggering an error boundary — or maybe even those, too. Need to
517517
// figure out the right API.
518-
logHydrationError(root.errorLoggingConfig, value);
518+
logRecoverableError(root.errorLoggingConfig, value);
519519
return;
520520
}
521521
} else {
@@ -526,7 +526,7 @@ function throwException(
526526
// We didn't find a boundary that could handle this type of exception. Start
527527
// over and traverse parent path again, this time treating the exception
528528
// as an error.
529-
renderDidError();
529+
renderDidError(value);
530530

531531
value = createCapturedValue(value, sourceFiber);
532532
let workInProgress = returnFiber;

‎packages/react-reconciler/src/ReactFiberThrow.old.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {
3737
import {
3838
supportsPersistence,
3939
getOffscreenContainerProps,
40-
logHydrationError,
40+
logRecoverableError,
4141
} from './ReactFiberHostConfig';
4242
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
4343
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -515,7 +515,7 @@ function throwException(
515515
// probably want to log any error that is recovered from without
516516
// triggering an error boundary — or maybe even those, too. Need to
517517
// figure out the right API.
518-
logHydrationError(root.errorLoggingConfig, value);
518+
logRecoverableError(root.errorLoggingConfig, value);
519519
return;
520520
}
521521
} else {
@@ -526,7 +526,7 @@ function throwException(
526526
// We didn't find a boundary that could handle this type of exception. Start
527527
// over and traverse parent path again, this time treating the exception
528528
// as an error.
529-
renderDidError();
529+
renderDidError(value);
530530

531531
value = createCapturedValue(value, sourceFiber);
532532
let workInProgress = returnFiber;

0 commit comments

Comments
 (0)