diff --git a/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js b/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js new file mode 100644 index 0000000000..562853f850 --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactErrorLoggingRecovery-test.js @@ -0,0 +1,79 @@ +/** + * 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 + */ + +// This is a regression test for https://github.com/facebook/react/issues/13188. +// It reproduces a combination of conditions that led to a problem. + +if (global.window) { + throw new Error('This test must run in a Node environment.'); +} + +// The issue only reproduced when React was loaded before JSDOM. +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'); +global.requestAnimationFrame = setTimeout; +global.cancelAnimationFrame = clearTimeout; +const jsdom = new JSDOM(`
`); +global.window = jsdom.window; +global.document = jsdom.window.document; +global.navigator = jsdom.window.navigator; + +class Bad extends React.Component { + componentDidUpdate() { + throw new Error('no'); + } + render() { + return null; + } +} + +describe('ReactErrorLoggingRecovery', () => { + let originalConsoleError = console.error; + + beforeEach(() => { + console.error = error => { + throw new Error('Buggy console.error'); + }; + }); + + afterEach(() => { + console.error = originalConsoleError; + }); + + it('should recover from errors in console.error', function() { + const div = document.createElement('div'); + let didCatch = false; + try { + ReactDOM.render(, div); + ReactDOM.render(, div); + } catch (e) { + expect(e.message).toBe('no'); + didCatch = true; + } + expect(didCatch).toBe(true); + ReactDOM.render(Hello, div); + expect(div.firstChild.textContent).toBe('Hello'); + + // Verify the console.error bug is surfaced + expect(() => { + jest.runAllTimers(); + }).toThrow('Buggy console.error'); + }); +}); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index d5fb55b492..16653ea928 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -116,7 +116,12 @@ export function logError(boundary: Fiber, errorInfo: CapturedValue) { // A cycle may still occur if logCapturedError renders a component that throws. const suppressLogging = e && e.suppressReactErrorLogging; if (!suppressLogging) { - console.error(e); + // 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 overriden. + // https://github.com/facebook/react/issues/13188 + setTimeout(() => { + throw e; + }); } } } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js index 9b417309a6..f651e98c30 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js @@ -144,11 +144,10 @@ describe('ReactIncrementalErrorLogging', () => { expect(logCapturedErrorCalls.length).toBe(1); - // The error thrown in logCapturedError should also be logged - expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error.calls.argsFor(0)[0].message).toContain( - 'logCapturedError error', - ); + // The error thrown in logCapturedError should be rethrown with a clean stack + expect(() => { + jest.runAllTimers(); + }).toThrow('logCapturedError error'); } finally { jest.unmock('../ReactFiberErrorLogger'); } diff --git a/scripts/jest/config.build.js b/scripts/jest/config.build.js index 7ad1cf68b6..bada59a627 100644 --- a/scripts/jest/config.build.js +++ b/scripts/jest/config.build.js @@ -10,6 +10,11 @@ const packages = readdirSync(packagesRoot).filter(dir => { if (dir.charAt(0) === '.') { return false; } + if (dir === 'events') { + // There's an actual Node package called "events" + // that's used by jsdom so we don't want to alias that. + return false; + } const packagePath = join(packagesRoot, dir, 'package.json'); return statSync(packagePath).isFile(); });