Don't log onRecoverableError if the current commit fail (facebook#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.

Author: sebmarkbage

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

@@ -6004,23 +6004,23 @@ describe('ReactDOMFizzServer', () => {
     expect(reportedServerErrors.length).toBe(1);
     expect(reportedServerErrors[0].message).toBe('Oops!');
 
+    const reportedCaughtErrors = [];
     const reportedClientErrors = [];
     ReactDOMClient.hydrateRoot(container, <App />, {
+      onCaughtError(error) {
+        reportedCaughtErrors.push(error);
+      },
       onRecoverableError(error) {
         reportedClientErrors.push(error);
       },
     });
     await waitForAll([]);
     expect(getVisibleChildren(container)).toEqual('Oops!');
-    expect(reportedClientErrors.length).toBe(1);
-    if (__DEV__) {
-      expect(reportedClientErrors[0].message).toBe('Oops!');
-    } else {
-      expect(reportedClientErrors[0].message).toBe(
-        'The server could not finish this Suspense boundary, likely due to ' +
-          'an error during server rendering. Switched to client rendering.',
-      );
-    }
+    // Because this is rethrown on the client, it is not a recoverable error.
+    expect(reportedClientErrors.length).toBe(0);
+    // It is caught by the error boundary.
+    expect(reportedCaughtErrors.length).toBe(1);
+    expect(reportedCaughtErrors[0].message).toBe('Oops!');
   });
 
   it("use a promise that's already been instrumented and resolved", async () => {

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -1099,7 +1099,14 @@ function finishConcurrentRender(
       // Commit the placeholder.
       break;
     }
-    case RootErrored:
+    case RootErrored: {
+      // This render errored. Ignore any recoverable errors because we weren't actually
+      // able to recover. Instead, whatever the final errors were is the ones we log.
+      // This ensures that we only log the actual client side error if it's just a plain
+      // error thrown from a component on the server and the client.
+      workInProgressRootRecoverableErrors = null;
+      break;
+    }
     case RootSuspended:
     case RootCompleted: {
       break;