diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 75f7ca7dad567..38057599f1844 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -791,7 +791,15 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * skips will/DidUpdate when bailing unless an update was already in progress * performs batched updates at the end of the batch * can nest batchedUpdates + +src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +* catches render error in a boundary during mounting * propagates an error from a noop error boundary +* can schedule updates after uncaught error in render on mount +* can schedule updates after uncaught error in render on update +* can schedule updates after uncaught error during umounting +* continues work on other roots despite caught errors +* continues work on other roots despite uncaught errors src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js * handles isMounted even when the initial render is deferred diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index df1ceaa3bec11..712d0adca15c0 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -166,12 +166,13 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { if (typeof newInstance.componentWillMount === 'function') { newInstance.componentWillMount(); - // If we had additional state updates during this life-cycle, let's - // process them now. - const newUpdateQueue = workInProgress.updateQueue; - if (newUpdateQueue) { - newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps); - } + } + // If we had additional state updates, process them now. + // They may be from componentWillMount() or from error boundary's setState() + // during initial mounting. + const newUpdateQueue = workInProgress.updateQueue; + if (newUpdateQueue) { + newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps); } return true; } diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 7a902e20977b2..4676d772ed5e9 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -33,7 +33,7 @@ var { module.exports = function( config : HostConfig, - trapError : (fiber : Fiber, error: Error, isUnmounting : boolean) => void + trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void ) { const updateContainer = config.updateContainer; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index dfd1fe4cdcb8c..25526d3805ce4 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -60,6 +60,7 @@ var timeHeuristicForUnitOfWork = 1; type TrappedError = { boundary: Fiber | null, + root: FiberRoot | null, error: any, }; @@ -100,6 +101,9 @@ module.exports = function(config : HostConfig) { let activeErrorBoundaries : Set | null = null; let nextTrappedErrors : Array | null = null; + // Roots that have uncaught errors and should not be worked on + let rootsWithUncaughtErrors : Set | null = null; + function scheduleAnimationCallback(callback) { if (!isAnimationCallbackScheduled) { isAnimationCallbackScheduled = true; @@ -115,8 +119,11 @@ module.exports = function(config : HostConfig) { } function findNextUnitOfWork() { - // Clear out roots with no more work on them. - while (nextScheduledRoot && nextScheduledRoot.current.pendingWorkPriority === NoWork) { + // Clear out roots with no more work on them, or if they have uncaught errors + while (nextScheduledRoot && ( + nextScheduledRoot.current.pendingWorkPriority === NoWork || + (rootsWithUncaughtErrors && rootsWithUncaughtErrors.has(nextScheduledRoot)) + )) { // Unschedule this root. nextScheduledRoot.isScheduled = false; // Read the next pointer now. @@ -445,20 +452,23 @@ module.exports = function(config : HostConfig) { } isPerformingTaskWork = true; - nextUnitOfWork = findNextUnitOfWork(); - while (nextUnitOfWork && - nextPriorityLevel === TaskPriority) { - nextUnitOfWork = - performUnitOfWork(nextUnitOfWork); + try { + nextUnitOfWork = findNextUnitOfWork(); + while (nextUnitOfWork && + nextPriorityLevel === TaskPriority) { + nextUnitOfWork = + performUnitOfWork(nextUnitOfWork); - if (!nextUnitOfWork) { - nextUnitOfWork = findNextUnitOfWork(); + if (!nextUnitOfWork) { + nextUnitOfWork = findNextUnitOfWork(); + } } + if (nextUnitOfWork) { + scheduleCallbackAtPriority(nextPriorityLevel); + } + } finally { + isPerformingTaskWork = false; } - if (nextUnitOfWork) { - scheduleCallbackAtPriority(nextPriorityLevel); - } - isPerformingTaskWork = false; } function performTaskWork() { @@ -466,53 +476,76 @@ module.exports = function(config : HostConfig) { } function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) { + // Keep track of the first error we need to surface to the user. + let firstUncaughtError = null; + // The exact priority level doesn't matter, so long as it's in range of the // work (sync, animation, deferred) being performed. - try { - switch (priorityLevel) { - case SynchronousPriority: - performSynchronousWorkUnsafe(); - break; - case TaskPriority: - if (!isPerformingTaskWork) { - performTaskWorkUnsafe(); - } - break; - case AnimationPriority: - performAnimationWorkUnsafe(); - break; - case HighPriority: - case LowPriority: - case OffscreenPriority: - if (!deadline) { - throw new Error('No deadline'); - } else { - performDeferredWorkUnsafe(deadline); - } - break; - default: - break; + while (true) { + try { + switch (priorityLevel) { + case SynchronousPriority: + performSynchronousWorkUnsafe(); + break; + case TaskPriority: + if (!isPerformingTaskWork) { + performTaskWorkUnsafe(); + } + break; + case AnimationPriority: + performAnimationWorkUnsafe(); + break; + case HighPriority: + case LowPriority: + case OffscreenPriority: + if (!deadline) { + throw new Error('No deadline'); + } else { + performDeferredWorkUnsafe(deadline); + } + break; + default: + break; + } + } catch (error) { + trapError(nextUnitOfWork, error, false); + } + + // If there were errors and we aren't already handling them, handle them now + if (nextTrappedErrors && !activeErrorBoundaries) { + const nextUncaughtError = handleErrors(); + firstUncaughtError = firstUncaughtError || nextUncaughtError; + } else { + // We've done our work. + break; + } + + // An error interrupted us. Now that it is handled, we may find more work. + // It's safe because any roots with uncaught errors have been unscheduled. + nextUnitOfWork = findNextUnitOfWork(); + if (!nextUnitOfWork) { + // We found no other work we could do. + break; } - } catch (error) { - trapError(nextUnitOfWork, error, false); } - // If there were errors and we aren't already handling them, handle them now - if (nextTrappedErrors && !activeErrorBoundaries) { - handleErrors(); + // Now it's safe to surface the first uncaught error to the user. + if (firstUncaughtError) { + throw firstUncaughtError; } } - function handleErrors() : void { + function handleErrors() : Error | null { if (activeErrorBoundaries) { throw new Error('Already handling errors'); } // Start tracking active boundaries. activeErrorBoundaries = new Set(); - - // If we find unhandled errors, we'll only rethrow the first one. + // If we find unhandled errors, we'll only remember the first one. let firstUncaughtError = null; + // Keep track of which roots have fataled and need to be unscheduled. + rootsWithUncaughtErrors = new Set(); // All work created by error boundaries should have Task priority // so that it finishes before this function exits. @@ -527,8 +560,23 @@ module.exports = function(config : HostConfig) { const trappedError = nextTrappedErrors.shift(); const boundary = trappedError.boundary; const error = trappedError.error; + const root = trappedError.root; if (!boundary) { firstUncaughtError = firstUncaughtError || error; + if (root) { + // Remember to unschedule this particular root since it fataled + // and we can't do more work on it. This lets us continue working on + // other roots even if one of them fails before rethrowing the error. + rootsWithUncaughtErrors.add(root); + } else { + // Normally we should know which root caused the error, so it is + // unusual if we end up here. Since we assume this function always + // unschedules failed roots, our only resort is to completely + // unschedule all roots. Otherwise we may get into an infinite loop + // trying to resume work and finding the failing but unknown root again. + nextScheduledRoot = null; + lastScheduledRoot = null; + } continue; } // Don't visit boundaries twice. @@ -561,47 +609,23 @@ module.exports = function(config : HostConfig) { try { performTaskWorkUnsafe(); } catch (error) { - isPerformingTaskWork = false; trapError(nextUnitOfWork, error, false); } } nextTrappedErrors = null; activeErrorBoundaries = null; + rootsWithUncaughtErrors = null; priorityContext = previousPriorityContext; - if (firstUncaughtError) { - // We need to make sure any future root can get scheduled despite these errors. - // Currently after throwing, nothing gets scheduled because these fields are set. - // FIXME: this is likely a wrong fix! It's still better than ignoring updates though. - nextScheduledRoot = null; - lastScheduledRoot = null; - throw firstUncaughtError; - } + // Return the error so we can rethrow after handling other roots. + return firstUncaughtError; } - function findClosestErrorBoundary(fiber : Fiber): Fiber | null { - let maybeErrorBoundary = fiber.return; - while (maybeErrorBoundary) { - if (maybeErrorBoundary.tag === ClassComponent) { - const instance = maybeErrorBoundary.stateNode; - const isErrorBoundary = typeof instance.unstable_handleError === 'function'; - if (isErrorBoundary) { - const isHandlingAnotherError = ( - activeErrorBoundaries !== null && - activeErrorBoundaries.has(maybeErrorBoundary) - ); - if (!isHandlingAnotherError) { - return maybeErrorBoundary; - } - } - } - maybeErrorBoundary = maybeErrorBoundary.return; - } - return null; - } + function trapError(failedFiber : Fiber | null, error : any, isUnmounting : boolean) : void { + // Don't try to start here again on next flush. + nextUnitOfWork = null; - function trapError(fiber : Fiber | null, error : any, isUnmounting : boolean) : void { // It is no longer valid because we exited the user code. ReactCurrentOwner.current = null; @@ -611,12 +635,46 @@ module.exports = function(config : HostConfig) { return; } + let boundary = null; + let root = null; + + // Search for the parent error boundary and root. + let fiber = failedFiber; + while (fiber) { + const parent = fiber.return; + if (parent) { + if (parent.tag === ClassComponent && boundary === null) { + // Consider a candidate for parent boundary. + const instance = parent.stateNode; + const isBoundary = typeof instance.unstable_handleError === 'function'; + if (isBoundary) { + // Skip boundaries that are already active so errors can propagate. + const isBoundaryAlreadyHandlingAnotherError = ( + activeErrorBoundaries !== null && + activeErrorBoundaries.has(parent) + ); + if (!isBoundaryAlreadyHandlingAnotherError) { + // We found the boundary. + boundary = parent; + } + } + } + } else if (fiber.tag === HostContainer) { + // We found the root. + root = (fiber.stateNode : FiberRoot); + } else { + throw new Error('Invalid root'); + } + fiber = parent; + } + if (!nextTrappedErrors) { nextTrappedErrors = []; } nextTrappedErrors.push({ - boundary: fiber ? findClosestErrorBoundary(fiber) : null, + boundary, error, + root, }); } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 8fa9afe489d00..5df5c10caf800 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1465,27 +1465,4 @@ describe('ReactIncremental', () => { ]); expect(instance.state.n).toEqual(4); }); - - it('propagates an error from a noop error boundary', () => { - class NoopBoundary extends React.Component { - unstable_handleError() { - // Noop - } - render() { - return this.props.children; - } - } - - function RenderError() { - throw new Error('render error'); - } - - ReactNoop.render( - - - - ); - - expect(ReactNoop.flush).toThrow('render error'); - }); }); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js new file mode 100644 index 0000000000000..0f55886650227 --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -0,0 +1,278 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactNoop; + +describe('ReactIncrementalErrorHandling', () => { + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactNoop = require('ReactNoop'); + }); + + function span(prop) { + return { type: 'span', children: [], prop }; + } + + it('catches render error in a boundary during mounting', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + unstable_handleError(error) { + this.setState({error}); + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + } + + function BrokenRender(props) { + throw new Error('Hello'); + } + + ReactNoop.render( + + + + ); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello.')]); + }); + + it('propagates an error from a noop error boundary', () => { + class NoopBoundary extends React.Component { + unstable_handleError() { + // Noop + } + render() { + return this.props.children; + } + } + + function RenderError() { + throw new Error('render error'); + } + + ReactNoop.render( + + + + ); + + expect(ReactNoop.flush).toThrow('render error'); + }); + + it('can schedule updates after uncaught error in render on mount', () => { + var ops = []; + + function BrokenRender() { + ops.push('BrokenRender'); + throw new Error('Hello'); + } + + function Foo() { + ops.push('Foo'); + return null; + } + + ReactNoop.render(); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ops).toEqual(['BrokenRender']); + + ops = []; + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo']); + }); + + it('can schedule updates after uncaught error in render on update', () => { + var ops = []; + + function BrokenRender(props) { + ops.push('BrokenRender'); + if (props.throw) { + throw new Error('Hello'); + } + } + + function Foo() { + ops.push('Foo'); + return null; + } + + ReactNoop.render(); + ReactNoop.flush(); + ops = []; + + expect(() => { + ReactNoop.render(); + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ops).toEqual(['BrokenRender']); + + ops = []; + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo']); + }); + + it('can schedule updates after uncaught error during umounting', () => { + var ops = []; + + class BrokenComponentWillUnmount extends React.Component { + render() { + return
; + } + componentWillUnmount() { + throw new Error('Hello'); + } + } + + function Foo() { + ops.push('Foo'); + return null; + } + + ReactNoop.render(); + ReactNoop.flush(); + + expect(() => { + ReactNoop.render(
); + ReactNoop.flush(); + }).toThrow('Hello'); + + ops = []; + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo']); + }); + + it('continues work on other roots despite caught errors', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + unstable_handleError(error) { + this.setState({error}); + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + } + + function BrokenRender(props) { + throw new Error('Hello'); + } + + ReactNoop.renderToRootWithID( + + + , + 'a' + ); + ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.flush(); + expect(ReactNoop.getChildren('a')).toEqual([span('Caught an error: Hello.')]); + expect(ReactNoop.getChildren('b')).toEqual([span('b:1')]); + }); + + it('continues work on other roots despite uncaught errors', () => { + function BrokenRender(props) { + throw new Error('Hello'); + } + + ReactNoop.renderToRootWithID(, 'a'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ReactNoop.getChildren('a')).toEqual([]); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + + expect(ReactNoop.getChildren('a')).toEqual([]); + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ReactNoop.getChildren('a')).toEqual([span('a:3')]); + // Currently we assume previous tree stays intact for fataled trees. + // We may consider tearing it down in the future. + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.renderToRootWithID(, 'c'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ReactNoop.getChildren('a')).toEqual([span('a:4')]); + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]); + expect(ReactNoop.getChildren('c')).toEqual([span('c:4')]); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.renderToRootWithID(, 'c'); + ReactNoop.renderToRootWithID(, 'd'); + ReactNoop.renderToRootWithID(, 'e'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ReactNoop.getChildren('a')).toEqual([span('a:5')]); + expect(ReactNoop.getChildren('b')).toEqual([span('b:5')]); + expect(ReactNoop.getChildren('c')).toEqual([span('c:5')]); + expect(ReactNoop.getChildren('d')).toEqual([span('d:5')]); + expect(ReactNoop.getChildren('e')).toEqual([]); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.renderToRootWithID(, 'c'); + ReactNoop.renderToRootWithID(, 'd'); + ReactNoop.renderToRootWithID(, 'e'); + ReactNoop.renderToRootWithID(, 'f'); + expect(() => { + ReactNoop.flush(); + }).toThrow('Hello'); + expect(ReactNoop.getChildren('a')).toEqual([span('a:5')]); + expect(ReactNoop.getChildren('b')).toEqual([span('b:6')]); + expect(ReactNoop.getChildren('c')).toEqual([span('c:5')]); + expect(ReactNoop.getChildren('d')).toEqual([span('d:6')]); + expect(ReactNoop.getChildren('e')).toEqual([]); + expect(ReactNoop.getChildren('f')).toEqual([span('f:6')]); + + ReactNoop.unmountRootWithID('a'); + ReactNoop.unmountRootWithID('b'); + ReactNoop.unmountRootWithID('c'); + ReactNoop.unmountRootWithID('d'); + ReactNoop.unmountRootWithID('e'); + ReactNoop.unmountRootWithID('f'); + expect(ReactNoop.getChildren('a')).toEqual(null); + expect(ReactNoop.getChildren('b')).toEqual(null); + expect(ReactNoop.getChildren('c')).toEqual(null); + expect(ReactNoop.getChildren('d')).toEqual(null); + expect(ReactNoop.getChildren('e')).toEqual(null); + expect(ReactNoop.getChildren('f')).toEqual(null); + }); +});