diff --git a/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js b/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js index 562853f850..f40c546735 100644 --- a/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js +++ b/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js @@ -19,12 +19,6 @@ if (global.window) { const React = require('react'); const ReactDOM = require('react-dom'); -// Unlike other tests, we want to enable error logging. -// Note this is not a real Error prototype property, -// it's only set in our Jest environment. -// eslint-disable-next-line no-extend-native -Error.prototype.suppressReactErrorLogging = false; - // Initialize JSDOM separately. // We don't use our normal JSDOM setup because we want to load React first. const {JSDOM} = require('jsdom'); diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 81b1c7366d..bde5df4205 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -21,11 +21,6 @@ export function logCapturedError(capturedError: CapturedError): void { } const error = (capturedError.error: any); - const suppressLogging = error && error.suppressReactErrorLogging; - if (suppressLogging) { - return; - } - if (__DEV__) { const { componentName, diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js deleted file mode 100644 index f651e98c30..0000000000 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js +++ /dev/null @@ -1,194 +0,0 @@ -/** - * Copyright (c) 2013-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @emails react-core - * @jest-environment node - */ - -'use strict'; - -let React; -let ReactFeatureFlags; -let ReactNoop; - -describe('ReactIncrementalErrorLogging', () => { - beforeEach(() => { - jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; - React = require('react'); - ReactNoop = require('react-noop-renderer'); - }); - - it('should log errors that occur during the begin phase', () => { - // Errors are redundantly logged in production mode by ReactFiberErrorLogger. - // It's okay to ignore them for the purpose of this test. - spyOnProd(console, 'error'); - - class ErrorThrowingComponent extends React.Component { - UNSAFE_componentWillMount() { - const error = new Error('componentWillMount error'); - // Note: it's `true` on the Error prototype our test environment. - // That lets us avoid asserting on warnings for each expected error. - // Here we intentionally shadow it to test logging, like in real apps. - error.suppressReactErrorLogging = undefined; - throw error; - } - render() { - return
; - } - } - - ReactNoop.render( -
- - - -
, - ); - - expect(() => { - expect(ReactNoop.flushDeferredPri).toWarnDev( - 'The above error occurred in the component:\n' + - ' in ErrorThrowingComponent (at **)\n' + - ' in span (at **)\n' + - ' in div (at **)\n\n' + - 'Consider adding an error boundary to your tree to customize error handling behavior.', - ); - }).toThrowError('componentWillMount error'); - }); - - it('should log errors that occur during the commit phase', () => { - // Errors are redundantly logged in production mode by ReactFiberErrorLogger. - // It's okay to ignore them for the purpose of this test. - spyOnProd(console, 'error'); - - class ErrorThrowingComponent extends React.Component { - componentDidMount() { - const error = new Error('componentDidMount error'); - // Note: it's `true` on the Error prototype our test environment. - // That lets us avoid asserting on warnings for each expected error. - // Here we intentionally shadow it to test logging, like in real apps. - error.suppressReactErrorLogging = undefined; - throw error; - } - render() { - return
; - } - } - - ReactNoop.render( -
- - - -
, - ); - - expect(() => { - expect(ReactNoop.flushDeferredPri).toWarnDev( - 'The above error occurred in the component:\n' + - ' in ErrorThrowingComponent (at **)\n' + - ' in span (at **)\n' + - ' in div (at **)\n\n' + - 'Consider adding an error boundary to your tree to customize error handling behavior.', - ); - }).toThrowError('componentDidMount error'); - }); - - it('should ignore errors thrown in log method to prevent cycle', () => { - jest.resetModules(); - jest.mock('../ReactFiberErrorLogger'); - try { - React = require('react'); - ReactNoop = require('react-noop-renderer'); - - // TODO Update this test to use toWarnDev() matcher if possible - spyOnDevAndProd(console, 'error'); - - class ErrorThrowingComponent extends React.Component { - render() { - throw new Error('render error'); - } - } - - const logCapturedErrorCalls = []; - - const ReactFiberErrorLogger = require('../ReactFiberErrorLogger'); - ReactFiberErrorLogger.logCapturedError.mockImplementation( - capturedError => { - logCapturedErrorCalls.push(capturedError); - const error = new Error('logCapturedError error'); - // Note: it's `true` on the Error prototype our test environment. - // That lets us avoid asserting on warnings for each expected error. - // Here we intentionally shadow it to test logging, like in real apps. - error.suppressReactErrorLogging = undefined; - throw error; - }, - ); - - try { - ReactNoop.render( -
- - - -
, - ); - ReactNoop.flushDeferredPri(); - } catch (error) {} - - expect(logCapturedErrorCalls.length).toBe(1); - - // The error thrown in logCapturedError should be rethrown with a clean stack - expect(() => { - jest.runAllTimers(); - }).toThrow('logCapturedError error'); - } finally { - jest.unmock('../ReactFiberErrorLogger'); - } - }); - - it('resets instance variables before unmounting failed node', () => { - class ErrorBoundary extends React.Component { - state = {error: null}; - componentDidCatch(error) { - this.setState({error}); - } - render() { - return this.state.error ? null : this.props.children; - } - } - class Foo extends React.Component { - state = {step: 0}; - componentDidMount() { - this.setState({step: 1}); - } - componentWillUnmount() { - ReactNoop.yield('componentWillUnmount: ' + this.state.step); - } - render() { - ReactNoop.yield('render: ' + this.state.step); - if (this.state.step > 0) { - throw new Error('oops'); - } - return null; - } - } - - ReactNoop.render( - - - , - ); - expect(ReactNoop.flush()).toEqual([ - 'render: 0', - 'render: 1', - 'componentWillUnmount: 0', - ]); - }); -}); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js new file mode 100644 index 0000000000..cfe14d09c9 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -0,0 +1,211 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +'use strict'; + +let React; +let ReactNoop; + +describe('ReactIncrementalErrorLogging', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactNoop = require('react-noop-renderer'); + }); + + // Note: in this test file we won't be using toWarnDev() matchers + // because they filter out precisely the messages we want to test for. + let oldConsoleError; + beforeEach(() => { + oldConsoleError = console.error; + console.error = jest.fn(); + }); + + afterEach(() => { + console.error = oldConsoleError; + oldConsoleError = null; + }); + + it('should log errors that occur during the begin phase', () => { + class ErrorThrowingComponent extends React.Component { + constructor(props) { + super(props); + throw new Error('constructor error'); + } + render() { + return
; + } + } + ReactNoop.render( +
+ + + +
, + ); + expect(ReactNoop.flushDeferredPri).toThrowError('constructor error'); + expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalledWith( + __DEV__ + ? expect.stringMatching( + new RegExp( + 'The above error occurred in the component:\n' + + '\\s+in ErrorThrowingComponent (.*)\n' + + '\\s+in span (.*)\n' + + '\\s+in div (.*)\n\n' + + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior\\.', + ), + ) + : expect.objectContaining({ + message: 'constructor error', + }), + ); + }); + + it('should log errors that occur during the commit phase', () => { + class ErrorThrowingComponent extends React.Component { + componentDidMount() { + throw new Error('componentDidMount error'); + } + render() { + return
; + } + } + ReactNoop.render( +
+ + + +
, + ); + expect(ReactNoop.flushDeferredPri).toThrowError('componentDidMount error'); + expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalledWith( + __DEV__ + ? expect.stringMatching( + new RegExp( + 'The above error occurred in the component:\n' + + '\\s+in ErrorThrowingComponent (.*)\n' + + '\\s+in span (.*)\n' + + '\\s+in div (.*)\n\n' + + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior\\.', + ), + ) + : expect.objectContaining({ + message: 'componentDidMount error', + }), + ); + }); + + it('should ignore errors thrown in log method to prevent cycle', () => { + const logCapturedErrorCalls = []; + console.error.mockImplementation(error => { + // Test what happens when logging itself is buggy. + logCapturedErrorCalls.push(error); + throw new Error('logCapturedError error'); + }); + class ErrorThrowingComponent extends React.Component { + render() { + throw new Error('render error'); + } + } + ReactNoop.render( +
+ + + +
, + ); + expect(ReactNoop.flushDeferredPri).toThrow('render error'); + expect(logCapturedErrorCalls.length).toBe(1); + expect(logCapturedErrorCalls[0]).toEqual( + __DEV__ + ? expect.stringMatching( + new RegExp( + 'The above error occurred in the component:\n' + + '\\s+in ErrorThrowingComponent (.*)\n' + + '\\s+in span (.*)\n' + + '\\s+in div (.*)\n\n' + + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior\\.', + ), + ) + : expect.objectContaining({ + message: 'render error', + }), + ); + // The error thrown in logCapturedError should be rethrown with a clean stack + expect(() => { + jest.runAllTimers(); + }).toThrow('logCapturedError error'); + }); + + it('resets instance variables before unmounting failed node', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + this.setState({error}); + } + render() { + return this.state.error ? null : this.props.children; + } + } + class Foo extends React.Component { + state = {step: 0}; + componentDidMount() { + this.setState({step: 1}); + } + componentWillUnmount() { + ReactNoop.yield('componentWillUnmount: ' + this.state.step); + } + render() { + ReactNoop.yield('render: ' + this.state.step); + if (this.state.step > 0) { + throw new Error('oops'); + } + return null; + } + } + + ReactNoop.render( + + + , + ); + expect(ReactNoop.flush()).toEqual( + [ + 'render: 0', + __DEV__ && 'render: 0', // replay + 'render: 1', + __DEV__ && 'render: 1', // replay + 'componentWillUnmount: 0', + ].filter(Boolean), + ); + + expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalledWith( + __DEV__ + ? expect.stringMatching( + new RegExp( + 'The above error occurred in the component:\n' + + '\\s+in Foo (.*)\n' + + '\\s+in ErrorBoundary (.*)\n\n' + + 'React will try to recreate this component tree from scratch ' + + 'using the error boundary you provided, ErrorBoundary.', + ), + ) + : expect.objectContaining({ + message: 'oops', + }), + ); + }); +}); diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 705ac3e859..33095ab5ec 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -2,6 +2,7 @@ const jestDiff = require('jest-diff'); const util = require('util'); +const shouldIgnoreConsoleError = require('../shouldIgnoreConsoleError'); function normalizeCodeLocInfo(str) { return str && str.replace(/at .+?:\d+/g, 'at **'); @@ -54,6 +55,15 @@ const createMatcherFor = consoleMethod => typeof message === 'string' && message.includes('\n in '); const consoleSpy = (format, ...args) => { + // Ignore uncaught errors reported by jsdom + // and React addendums because they're too noisy. + if ( + consoleMethod === 'error' && + shouldIgnoreConsoleError(format, args) + ) { + return; + } + const message = util.format(format, ...args); const normalizedMessage = normalizeCodeLocInfo(message); diff --git a/scripts/jest/setupEnvironment.js b/scripts/jest/setupEnvironment.js index cacaf1db9d..009ad89453 100644 --- a/scripts/jest/setupEnvironment.js +++ b/scripts/jest/setupEnvironment.js @@ -7,14 +7,6 @@ if (NODE_ENV !== 'development' && NODE_ENV !== 'production') { global.__DEV__ = NODE_ENV === 'development'; global.__PROFILE__ = NODE_ENV === 'development'; -// By default React console.error()'s any errors, caught or uncaught. -// However it is annoying to assert that a warning fired each time -// we assert that there is an exception in our tests. This lets us -// opt out of extra console error reporting for most tests except -// for the few that specifically test the logging by shadowing this -// property. In real apps, it would usually not be defined at all. -Error.prototype.suppressReactErrorLogging = true; - if (typeof window !== 'undefined') { global.requestAnimationFrame = function(callback) { setTimeout(callback); @@ -33,16 +25,4 @@ if (typeof window !== 'undefined') { global.cancelIdleCallback = function(callbackID) { clearTimeout(callbackID); }; - - // Same as we did with Error.prototype above. - DOMException.prototype.suppressReactErrorLogging = true; - - // Also prevent JSDOM from logging intentionally thrown errors. - // TODO: it might make sense to do it the other way around. - // https://github.com/facebook/react/issues/11098#issuecomment-355032539 - window.addEventListener('error', event => { - if (event.error != null && event.error.suppressReactErrorLogging) { - event.preventDefault(); - } - }); } diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 63bdd6e529..57c97f97b9 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -2,6 +2,7 @@ const chalk = require('chalk'); const util = require('util'); +const shouldIgnoreConsoleError = require('./shouldIgnoreConsoleError'); if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { // Inside the class equivalence tester, we have a custom environment, let's @@ -66,6 +67,12 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { ['error', 'warn'].forEach(methodName => { const unexpectedConsoleCallStacks = []; const newMethod = function(format, ...args) { + // Ignore uncaught errors reported by jsdom + // and React addendums because they're too noisy. + if (methodName === 'error' && shouldIgnoreConsoleError(format, args)) { + return; + } + // Capture the call stack now so we can warn about it later. // The call stack has helpful information for the test author. // Don't throw yet though b'c it might be accidentally caught and suppressed. diff --git a/scripts/jest/shouldIgnoreConsoleError.js b/scripts/jest/shouldIgnoreConsoleError.js new file mode 100644 index 0000000000..4ea96751c0 --- /dev/null +++ b/scripts/jest/shouldIgnoreConsoleError.js @@ -0,0 +1,31 @@ +'use strict'; + +module.exports = function shouldIgnoreConsoleError(format, args) { + if (__DEV__) { + if (typeof format === 'string') { + if (format.indexOf('Error: Uncaught [') === 0) { + // This looks like an uncaught error from invokeGuardedCallback() wrapper + // in development that is reported by jsdom. Ignore because it's noisy. + return true; + } + if (format.indexOf('The above error occurred') === 0) { + // This looks like an error addendum from ReactFiberErrorLogger. + // Ignore it too. + return true; + } + } + } else { + if ( + format != null && + typeof format.message === 'string' && + typeof format.stack === 'string' && + args.length === 0 + ) { + // In production, ReactFiberErrorLogger logs error objects directly. + // They are noisy too so we'll try to ignore them. + return true; + } + } + // Looks legit + return false; +};