Allow the user to opt out of seeing "The above error..." addendum (#1…

…3384)

* Remove e.suppressReactErrorLogging check before last resort throw

It's unnecessary here. It was here because this method called console.error().
But we now rethrow with a clean stack, and that's worth doing regardless of whether the logging is silenced.

* Don't print error addendum if 'error' event got preventDefault()

* Add fixtures

* Use an expando property instead of a WeakSet

* Make it a bit less fragile

* Clarify comments

fixtures/dom/src/components/fixtures/error-handling/index.js

@@ -7,9 +7,21 @@ const ReactDOM = window.ReactDOM;
 function BadRender(props) {
   props.doThrow();
 }
+
+class BadDidMount extends React.Component {
+  componentDidMount() {
+    this.props.doThrow();
+  }
+
+  render() {
+    return null;
+  }
+}
+
 class ErrorBoundary extends React.Component {
   static defaultProps = {
     buttonText: 'Trigger error',
+    badChildType: BadRender,
   };
   state = {
     shouldThrow: false,
@@ -33,7 +45,8 @@ class ErrorBoundary extends React.Component {
       }
     }
     if (this.state.shouldThrow) {
-      return <BadRender doThrow={this.props.doThrow} />;
+      const BadChild = this.props.badChildType;
+      return <BadChild doThrow={this.props.doThrow} />;
     }
     return <button onClick={this.triggerError}>{this.props.buttonText}</button>;
   }
@@ -84,6 +97,112 @@ class TriggerErrorAndCatch extends React.Component {
   }
 }
 
+function silenceWindowError(event) {
+  event.preventDefault();
+}
+
+class SilenceErrors extends React.Component {
+  state = {
+    silenceErrors: false,
+  };
+  componentDidMount() {
+    if (this.state.silenceErrors) {
+      window.addEventListener('error', silenceWindowError);
+    }
+  }
+  componentDidUpdate(prevProps, prevState) {
+    if (!prevState.silenceErrors && this.state.silenceErrors) {
+      window.addEventListener('error', silenceWindowError);
+    } else if (prevState.silenceErrors && !this.state.silenceErrors) {
+      window.removeEventListener('error', silenceWindowError);
+    }
+  }
+  componentWillUnmount() {
+    if (this.state.silenceErrors) {
+      window.removeEventListener('error', silenceWindowError);
+    }
+  }
+  render() {
+    return (
+      <div>
+        <label>
+          <input
+            type="checkbox"
+            value={this.state.silenceErrors}
+            onChange={() =>
+              this.setState(state => ({
+                silenceErrors: !state.silenceErrors,
+              }))
+            }
+          />
+          Silence errors
+        </label>
+        {this.state.silenceErrors && (
+          <div>
+            {this.props.children}
+            <br />
+            <hr />
+            <b style={{color: 'red'}}>
+              Don't forget to uncheck "Silence errors" when you're done with
+              this test!
+            </b>
+          </div>
+        )}
+      </div>
+    );
+  }
+}
+
+class SilenceRecoverableError extends React.Component {
+  render() {
+    return (
+      <SilenceErrors>
+        <ErrorBoundary
+          badChildType={BadRender}
+          buttonText={'Throw (render phase)'}
+          doThrow={() => {
+            throw new Error('Silenced error (render phase)');
+          }}
+        />
+        <ErrorBoundary
+          badChildType={BadDidMount}
+          buttonText={'Throw (commit phase)'}
+          doThrow={() => {
+            throw new Error('Silenced error (commit phase)');
+          }}
+        />
+      </SilenceErrors>
+    );
+  }
+}
+
+class TrySilenceFatalError extends React.Component {
+  container = document.createElement('div');
+
+  triggerErrorAndCatch = () => {
+    try {
+      ReactDOM.flushSync(() => {
+        ReactDOM.render(
+          <BadRender
+            doThrow={() => {
+              throw new Error('Caught error');
+            }}
+          />,
+          this.container
+        );
+      });
+    } catch (e) {}
+  };
+
+  render() {
+    return (
+      <SilenceErrors>
+        <button onClick={this.triggerErrorAndCatch}>Throw fatal error</button>
+      </SilenceErrors>
+    );
+  }
+}
+
 export default class ErrorHandlingTestCases extends React.Component {
   render() {
     return (
@@ -103,6 +222,12 @@ export default class ErrorHandlingTestCases extends React.Component {
             the BadRender component. After resuming, the "Trigger error" button
             should be replaced with "Captured an error: Oops!" Clicking reset
             should reset the test case.
+            <br />
+            <br />
+            In the console, you should see <b>two</b> messages: the actual error
+            ("Oops") printed natively by the browser with its JavaScript stack,
+            and our addendum ("The above error occurred in BadRender component")
+            with a React component stack.
           </TestCase.ExpectedResult>
           <Example
             doThrow={() => {
@@ -155,10 +280,44 @@ export default class ErrorHandlingTestCases extends React.Component {
           </TestCase.Steps>
           <TestCase.ExpectedResult>
             Open the console. "Uncaught Error: Caught error" should have been
-            logged by the browser.
+            logged by the browser. You should also see our addendum ("The above
+            error...").
           </TestCase.ExpectedResult>
           <TriggerErrorAndCatch />
         </TestCase>
+        <TestCase
+          title="Recoverable errors can be silenced with preventDefault (development mode only)"
+          description="">
+          <TestCase.Steps>
+            <li>Check the "Silence errors" checkbox below</li>
+            <li>Click the "Throw (render phase)" button</li>
+            <li>Click the "Throw (commit phase)" button</li>
+            <li>Uncheck the "Silence errors" checkbox</li>
+          </TestCase.Steps>
+          <TestCase.ExpectedResult>
+            Open the console. You shouldn't see <b>any</b> messages in the
+            console: neither the browser error, nor our "The above error"
+            addendum, from either of the buttons. The buttons themselves should
+            get replaced by two labels: "Captured an error: Silenced error
+            (render phase)" and "Captured an error: Silenced error (commit
+            phase)".
+          </TestCase.ExpectedResult>
+          <SilenceRecoverableError />
+        </TestCase>
+        <TestCase
+          title="Fatal errors cannot be silenced with preventDefault (development mode only)"
+          description="">
+          <TestCase.Steps>
+            <li>Check the "Silence errors" checkbox below</li>
+            <li>Click the "Throw fatal error" button</li>
+            <li>Uncheck the "Silence errors" checkbox</li>
+          </TestCase.Steps>
+          <TestCase.ExpectedResult>
+            Open the console. "Error: Caught error" should have been logged by
+            React. You should also see our addendum ("The above error...").
+          </TestCase.ExpectedResult>
+          <TrySilenceFatalError />
+        </TestCase>
       </FixtureSet>
     );
   }

fixtures/dom/src/style.css

@@ -158,7 +158,7 @@ li {
 }
 
 .test-case__body {
-  padding: 0 15px;
+  padding: 10px;
 }
 
 .test-case__desc {

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -112,17 +112,13 @@ export function logError(boundary: Fiber, errorInfo: CapturedValue<mixed>) {
   try {
     logCapturedError(capturedError);
   } catch (e) {
-    // Prevent cycle if logCapturedError() throws.
-    // A cycle may still occur if logCapturedError renders a component that throws.
-    const suppressLogging = e && e.suppressReactErrorLogging;
-    if (!suppressLogging) {
-      // Rethrow it from a clean stack because this function is assumed to never throw.
-      // We can't safely call console.error() here because it could *also* throw if overridden.
-      // https://github.com/facebook/react/issues/13188
-      setTimeout(() => {
-        throw e;
-      });
-    }
+    // This method must not throw, or React internal state will get messed up.
+    // If console.error is overridden, or logCapturedError() shows a dialog that throws,
+    // we want to report this error outside of the normal stack as a last resort.
+    // https://github.com/facebook/react/issues/13188
+    setTimeout(() => {
+      throw e;
+    });
   }
 }
 

packages/react-reconciler/src/ReactFiberErrorLogger.js

@@ -35,6 +35,25 @@ export function logCapturedError(capturedError: CapturedError): void {
       willRetry,
     } = capturedError;
 
+    // Browsers support silencing uncaught errors by calling
+    // `preventDefault()` in window `error` handler.
+    // We record this information as an expando on the error.
+    if (error != null && error._suppressLogging) {
+      if (errorBoundaryFound && willRetry) {
+        // The error is recoverable and was silenced.
+        // Ignore it and don't print the stack addendum.
+        // This is handy for testing error boundaries without noise.
+        return;
+      }
+      // The error is fatal. Since the silencing might have
+      // been accidental, we'll surface it anyway.
+      // However, the browser would have silenced the original error
+      // so we'll print it first, and then print the stack addendum.
+      console.error(error);
+      // For a more detailed description of this block, see:
+      // https://github.com/facebook/react/pull/13384
+    }
+
     const componentNameMessage = componentName
       ? `The above error occurred in the <${componentName}> component:`
       : 'The above error occurred in one of your React components:';

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -305,7 +305,19 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
     isReplayingFailedUnitOfWork = false;
     originalReplayError = null;
     if (hasCaughtError()) {
-      clearCaughtError();
+      const replayError = clearCaughtError();
+      if (replayError != null && thrownValue != null) {
+        try {
+          // Reading the expando property is intentionally
+          // inside `try` because it might be a getter or Proxy.
+          if (replayError._suppressLogging) {
+            // Also suppress logging for the original error.
+            (thrownValue: any)._suppressLogging = true;
+          }
+        } catch (inner) {
+          // Ignore.
+        }
+      }
     } else {
       // If the begin phase did not fail the second time, set this pointer
       // back to the original value.

packages/shared/invokeGuardedCallback.js

@@ -132,6 +132,18 @@ if (__DEV__) {
         if (error === null && event.colno === 0 && event.lineno === 0) {
           isCrossOriginError = true;
         }
+        if (event.defaultPrevented) {
+          // Some other error handler has prevented default.
+          // Browsers silence the error report if this happens.
+          // We'll remember this to later decide whether to log it or not.
+          if (error != null && typeof error === 'object') {
+            try {
+              error._suppressLogging = true;
+            } catch (inner) {
+              // Ignore.
+            }
+          }
+        }
       }
 
       // Create a fake event type.