Don't log onRecoverableError if the current commit fail (#28665)
We didn't recover after all. Currently we might log a recoverable error in the recovery pass. E.g. the SSR server had an error. Then the client component fails to render which errors again. This ends up double logging. So if we fail to actually complete a fully successful commit, we ignore any recoverable errors because we'll get real errors logged. It's possible that this might cover up some other error that happened at the same time.
Sebastian Markbåge committed
Mar 28, 2024 at 10:39 UTC
e10a7b5cd541882a78ff659147c1a0294413ccb0
2 files changed
+17
-10
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+9
-9
@@ -6004,23 +6004,23 @@ describe('ReactDOMFizzServer', () => {
6004
expect(reportedServerErrors.length).toBe(1);
6005
expect(reportedServerErrors[0].message).toBe('Oops!');
6006
6007
+ const reportedCaughtErrors = [];
6008
const reportedClientErrors = [];
6009
ReactDOMClient.hydrateRoot(container, <App />, {
6010
+ onCaughtError(error) {
6011
+ reportedCaughtErrors.push(error);
6012
+ },
6013
onRecoverableError(error) {
6014
reportedClientErrors.push(error);
6015
},
6016
});
6017
await waitForAll([]);
6018
expect(getVisibleChildren(container)).toEqual('Oops!');
6015
- expect(reportedClientErrors.length).toBe(1);
6016
- if (__DEV__) {
6017
- expect(reportedClientErrors[0].message).toBe('Oops!');
6018
- } else {
6019
- expect(reportedClientErrors[0].message).toBe(
6020
- 'The server could not finish this Suspense boundary, likely due to ' +
6021
- 'an error during server rendering. Switched to client rendering.',
6022
- );
6023
- }
6019
+ // Because this is rethrown on the client, it is not a recoverable error.
6020
+ expect(reportedClientErrors.length).toBe(0);
6021
+ // It is caught by the error boundary.
6022
+ expect(reportedCaughtErrors.length).toBe(1);
6023
+ expect(reportedCaughtErrors[0].message).toBe('Oops!');
6024
});
6025
6026
it("use a promise that's already been instrumented and resolved", async () => {
packages/react-reconciler/src/ReactFiberWorkLoop.js
+8
-1
@@ -1099,7 +1099,14 @@ function finishConcurrentRender(
1099
// Commit the placeholder.
1100
break;
1101
}
1102
- case RootErrored:
1102
+ case RootErrored: {
1103
+ // This render errored. Ignore any recoverable errors because we weren't actually
1104
+ // able to recover. Instead, whatever the final errors were is the ones we log.
1105
+ // This ensures that we only log the actual client side error if it's just a plain
1106
+ // error thrown from a component on the server and the client.
1107
+ workInProgressRootRecoverableErrors = null;
1108
+ break;
1109
+ }
1110
case RootSuspended:
1111
case RootCompleted: {
1112
break;