Skip to content

Commit 6090cab

Browse files
sebmarkbagesebmarkbage
authored
Use a Wrapper Error for onRecoverableError with a "cause" Field for the real Error (#28736)
We basically have four kinds of recoverable errors: - Hydration mismatches. - Server errored but client didn't. - Hydration render errored but client render didn't (in Root or Suspense boundary). - Concurrent render errored but synchronous render didn't. For the first three we log an additional error that the root or Suspense boundary didn't error. This provides some context about what happened. However, the problem is that for hydration mismatches that's unnecessary extra context that is confusing. We also don't log any additional context for concurrent render errors that could recover. This used to be the only recoverable error so it didn't need extra context but now we need to distinguish them. When we log these to `reportError` it's confusing to just see the error because you didn't see anything error on the page. It's also hard to group them together as one. In this PR, I remove the unnecessary context for hydration mismatches. For hydration and concurrent errors, I now wrap them in an error that describes that what happened but then use the new `cause` field to link the original error so we can keep that as the cause. The error that happened was that hydration client rendered or you deopted to sync render, the cause of that error is some other error. For server errors, we control the Error object so I already had added some context to that error object's message. Since we hide the message in prod, it's nice not to have the raw message in DEV neither. We could potentially split these into two errors for parity though.
1 parent 583eb67 commit 6090cab

16 files changed

+437
-286
lines changed

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

Lines changed: 97 additions & 73 deletions
Large diffs are not rendered by default.

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,9 @@ describe('ReactDOMFizzShellHydration', () => {
397397
},
398398
onRecoverableError(error) {
399399
Scheduler.log('onRecoverableError: ' + error.message);
400+
if (error.cause) {
401+
Scheduler.log('Cause: ' + error.cause.message);
402+
}
400403
},
401404
});
402405
});
@@ -462,6 +465,9 @@ describe('ReactDOMFizzShellHydration', () => {
462465
},
463466
onRecoverableError(error) {
464467
Scheduler.log('onRecoverableError: ' + error.message);
468+
if (error.cause) {
469+
Scheduler.log('Cause: ' + error.cause.message);
470+
}
465471
},
466472
});
467473
});
@@ -529,13 +535,16 @@ describe('ReactDOMFizzShellHydration', () => {
529535
},
530536
onRecoverableError(error) {
531537
Scheduler.log('onRecoverableError: ' + error.message);
538+
if (error.cause) {
539+
Scheduler.log('Cause: ' + error.cause.message);
540+
}
532541
},
533542
});
534543
});
535544

536545
assertLog([
537-
'onRecoverableError: plain error',
538-
'onRecoverableError: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.',
546+
'onRecoverableError: There was an error while hydrating but React was able to recover by instead client rendering the entire root.',
547+
'Cause: plain error',
539548
]);
540549
expect(container.textContent).toBe('Hello world');
541550
});

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

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
165165
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
166166
onRecoverableError(error) {
167167
// Don't miss a hydration error. There should be none.
168-
Scheduler.log(normalizeError(error.message));
168+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
169+
if (error.cause) {
170+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
171+
}
169172
},
170173
});
171174
await waitForAll([]);
@@ -205,7 +208,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
205208
);
206209
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
207210
onRecoverableError(error) {
208-
Scheduler.log(normalizeError(error.message));
211+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
212+
if (error.cause) {
213+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
214+
}
209215
},
210216
});
211217
await waitForAll([]);
@@ -246,12 +252,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
246252
);
247253
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
248254
onRecoverableError(error) {
249-
Scheduler.log(normalizeError(error.message));
255+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
256+
if (error.cause) {
257+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
258+
}
250259
},
251260
});
252261
await waitForAll([
253-
"Hydration failed because the server rendered HTML didn't match the client.",
254-
'There was an error while hydrating.',
262+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
255263
]);
256264
expect(getVisibleChildren(container)).toEqual(
257265
<div>
@@ -283,7 +291,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
283291
);
284292
const root = ReactDOMClient.hydrateRoot(container, <App text="Client" />, {
285293
onRecoverableError(error) {
286-
Scheduler.log(normalizeError(error.message));
294+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
295+
if (error.cause) {
296+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
297+
}
287298
},
288299
});
289300
await waitForAll([]);
@@ -327,12 +338,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
327338
);
328339
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
329340
onRecoverableError(error) {
330-
Scheduler.log(normalizeError(error.message));
341+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
342+
if (error.cause) {
343+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
344+
}
331345
},
332346
});
333347
await waitForAll([
334-
"Hydration failed because the server rendered HTML didn't match the client.",
335-
'There was an error while hydrating.',
348+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
336349
]);
337350
expect(getVisibleChildren(container)).toEqual(
338351
<div>
@@ -367,12 +380,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
367380
);
368381
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
369382
onRecoverableError(error) {
370-
Scheduler.log(normalizeError(error.message));
383+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
384+
if (error.cause) {
385+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
386+
}
371387
},
372388
});
373389
await waitForAll([
374-
"Hydration failed because the server rendered HTML didn't match the client.",
375-
'There was an error while hydrating.',
390+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
376391
]);
377392
expect(getVisibleChildren(container)).toEqual(
378393
<div>
@@ -410,12 +425,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
410425
);
411426
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
412427
onRecoverableError(error) {
413-
Scheduler.log(normalizeError(error.message));
428+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
429+
if (error.cause) {
430+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
431+
}
414432
},
415433
});
416434
await waitForAll([
417-
"Hydration failed because the server rendered HTML didn't match the client.",
418-
'There was an error while hydrating.',
435+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
419436
]);
420437
expect(getVisibleChildren(container)).toEqual(
421438
<div>
@@ -451,12 +468,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
451468
);
452469
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
453470
onRecoverableError(error) {
454-
Scheduler.log(normalizeError(error.message));
471+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
472+
if (error.cause) {
473+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
474+
}
455475
},
456476
});
457477
await waitForAll([
458-
"Hydration failed because the server rendered HTML didn't match the client.",
459-
'There was an error while hydrating.',
478+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
460479
]);
461480
expect(getVisibleChildren(container)).toEqual(
462481
<div>
@@ -496,7 +515,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
496515
);
497516
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
498517
onRecoverableError(error) {
499-
Scheduler.log(normalizeError(error.message));
518+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
519+
if (error.cause) {
520+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
521+
}
500522
},
501523
});
502524
await waitForAll([]);
@@ -533,7 +555,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
533555
);
534556
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
535557
onRecoverableError(error) {
536-
Scheduler.log(normalizeError(error.message));
558+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
559+
if (error.cause) {
560+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
561+
}
537562
},
538563
});
539564
await waitForAll([]);
@@ -566,12 +591,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
566591
);
567592
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
568593
onRecoverableError(error) {
569-
Scheduler.log(normalizeError(error.message));
594+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
595+
if (error.cause) {
596+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
597+
}
570598
},
571599
});
572600
await waitForAll([
573-
"Hydration failed because the server rendered HTML didn't match the client.",
574-
'There was an error while hydrating.',
601+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
575602
]);
576603
expect(getVisibleChildren(container)).toEqual(
577604
<div>
@@ -604,12 +631,14 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
604631
);
605632
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
606633
onRecoverableError(error) {
607-
Scheduler.log(normalizeError(error.message));
634+
Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
635+
if (error.cause) {
636+
Scheduler.log('Cause: ' + normalizeError(error.cause.message));
637+
}
608638
},
609639
});
610640
await waitForAll([
611-
"Hydration failed because the server rendered HTML didn't match the client.",
612-
'There was an error while hydrating.',
641+
"onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.",
613642
]);
614643
expect(getVisibleChildren(container)).toEqual(
615644
<div>

0 commit comments

Comments
 (0)