Skip to content

Commit

Permalink
[Fiber] New error boundary semantics (facebook#8304)
Browse files Browse the repository at this point in the history
* Exit early in scheduleUpdate if a node's priority matches

This is a performance optimization and is unobservable. However, it
helps protect against regressions on the following invariants on which
it relies:

- The priority of a fiber is greater than or equal to the priority of
all its descendent fibers.
- If a tree has pending work priority, its root is scheduled.

* New error boundary semantics

- Recovering from an error boundary no longer uses Task priority by
default. The work is scheduled using whatever priority created the
error.
- Error handling is now a side-effect that happens during the
commit phase.
- The default behavior of an error boundary is to render null. Errors
do not propagate except when an boundary fails. Conceptually, this would
be like throwing an error from a catch block.
- A host container is treated like a no-op error boundary. The first
error captured by a host container is thrown at the end of the batch.
Like with normal error boundaries, the entire tree is unmounted.

* Fix broken setState callback test

* Add test for "unwinding" context when an error interrupts rendering

* Switch over primary effect types only

This avoids the need to create an export for every combination of bits.

* Only continue the work loop if the error was successfully captured

* Add more tests for incremental error handling

These tests are currently failing:

  ✕ catches render error in a boundary during partial deferred mounting
  ✕ catches render error in a boundary during animation mounting
  ✕ propagates an error from a noop error boundary during full deferred mounting
  ✕ propagates an error from a noop error boundary during partial deferred mounting
  ✕ propagates an error from a noop error boundary during animation mounting

The observed behavior is that unstable_handleError() unexpected gets called twice:

      "ErrorBoundary render success",
      "BrokenRender",
      "ErrorBoundary unstable_handleError",
  +   "ErrorBoundary render success",
  +   "BrokenRender",
  +   "ErrorBoundary unstable_handleError",
      "ErrorBoundary render error"

* Verify batched updates get scheduled despite errors

* Add try/catch/finally blocks around commit phase passes

We'll consolidate all these blocks in a future PR that refactors the
commit phase to be separate from the perform work loop.

* NoopBoundary -> RethrowBoundary

* Only throw uncaught error once there is no more work to perform

* Remove outdated comment

It was fixed in facebook#8451.

* Record tests

* Always reset nextUnitOfWork on error

This is important so that the test at the end of performAndHandleErrors() knows it's safe to rethrow.

* Add a passing test for unmounting behavior on crashed tree

* Top-level errors

An error thrown from a host container should be "captured" by the host
container itself

* Remove outdated comment

* Separate Rethrow and Noop scenarios in boundary tests

* Move try block outside the commit loops
  • Loading branch information
acdlite authored and acusti committed Mar 15, 2017
1 parent 4db3ff6 commit 6eb50d2
Show file tree
Hide file tree
Showing 9 changed files with 1,161 additions and 482 deletions.
19 changes: 17 additions & 2 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1080,16 +1080,29 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* reads context when setState is above the provider

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
* catches render error in a boundary during mounting
* propagates an error from a noop error boundary
* catches render error in a boundary during full deferred mounting
* catches render error in a boundary during partial deferred mounting
* catches render error in a boundary during animation mounting
* catches render error in a boundary during synchronous mounting
* catches render error in a boundary during batched mounting
* propagates an error from a noop error boundary during full deferred mounting
* propagates an error from a noop error boundary during partial deferred mounting
* propagates an error from a noop error boundary during animation mounting
* propagates an error from a noop error boundary during synchronous mounting
* propagates an error from a noop error boundary during batched mounting
* applies batched updates regardless despite errors in scheduling
* applies nested batched updates despite errors in scheduling
* applies sync updates regardless despite errors in scheduling
* 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
* unwinds the context stack correctly on error
* catches reconciler errors in a boundary during mounting
* catches reconciler errors in a boundary during update
* recovers from uncaught reconciler errors
* unmounts components with uncaught errors

src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
* handles isMounted even when the initial render is deferred
Expand Down Expand Up @@ -1323,6 +1336,8 @@ src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
* catches errors in componentDidUpdate
* propagates errors inside boundary during componentDidMount
* lets different boundaries catch their own first errors
* discards a bad root if the root component fails
* renders empty output if error boundary does not handle the error

src/renderers/shared/shared/__tests__/ReactIdentity-test.js
* should allow key property to express identity
Expand Down
23 changes: 23 additions & 0 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,31 @@ module.exports = function<T, P, I, TI, C>(
}
}

function beginFailedWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) {
if (workInProgress.tag !== ClassComponent &&
workInProgress.tag !== HostContainer) {
throw new Error('Invalid type of work');
}

if (workInProgress.pendingWorkPriority === NoWork ||
workInProgress.pendingWorkPriority > priorityLevel) {
return bailoutOnLowPriority(current, workInProgress);
}

// If we don't bail out, we're going be recomputing our children so we need
// to drop our effect list.
workInProgress.firstEffect = null;
workInProgress.lastEffect = null;

// Unmount the current children as if the component rendered null
const nextChildren = null;
reconcileChildren(current, workInProgress, nextChildren);
return workInProgress.child;
}

return {
beginWork,
beginFailedWork,
};

};
12 changes: 8 additions & 4 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var {

module.exports = function<T, P, I, TI, C>(
config : HostConfig<T, P, I, TI, C>,
trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void
captureError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => Fiber | null
) {

const commitUpdate = config.commitUpdate;
Expand Down Expand Up @@ -133,6 +133,10 @@ module.exports = function<T, P, I, TI, C>(
}

function commitPlacement(finishedWork : Fiber) : void {
// Clear effect from effect tag before any errors can be thrown, so that
// we don't attempt to do this again
finishedWork.effectTag &= ~Placement;

// Recursively insert all host nodes into the parent.
const parent = getHostParent(finishedWork);
const before = getHostSibling(finishedWork);
Expand Down Expand Up @@ -272,7 +276,7 @@ module.exports = function<T, P, I, TI, C>(
if (typeof instance.componentWillUnmount === 'function') {
const error = tryCallComponentWillUnmount(instance);
if (error) {
trapError(current, error, true);
captureError(current, error, true);
}
}
return;
Expand Down Expand Up @@ -362,7 +366,7 @@ module.exports = function<T, P, I, TI, C>(
}
}
if (firstError) {
trapError(finishedWork, firstError, false);
captureError(finishedWork, firstError, false);
}
return;
}
Expand All @@ -375,7 +379,7 @@ module.exports = function<T, P, I, TI, C>(
firstError = callCallbacks(callbackList, rootFiber.current.child.stateNode);
}
if (firstError) {
trapError(rootFiber, firstError, false);
captureError(rootFiber, firstError, false);
}
return;
}
Expand Down
17 changes: 15 additions & 2 deletions src/renderers/shared/fiber/ReactFiberContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,19 @@ exports.hasContextChanged = function() : boolean {
function isContextProvider(fiber : Fiber) : boolean {
return (
fiber.tag === ClassComponent &&
// Instance might be null, if the fiber errored during construction
fiber.stateNode &&
typeof fiber.stateNode.getChildContext === 'function'
);
}
exports.isContextProvider = isContextProvider;

exports.popContextProvider = function() : void {
function popContextProvider() : void {
contextStack[index] = emptyObject;
didPerformWorkStack[index] = false;
index--;
};
}
exports.popContextProvider = popContextProvider;

exports.pushTopLevelContextObject = function(context : Object, didChange : boolean) : void {
invariant(index === -1, 'Unexpected context found on stack');
Expand Down Expand Up @@ -149,3 +152,13 @@ exports.findCurrentUnmaskedContext = function(fiber: Fiber) : Object {
}
return node.stateNode.context;
};

exports.unwindContext = function(from : Fiber, to: Fiber) {
let node = from;
while (node && (node !== to) && (node.alternate !== to)) {
if (isContextProvider(node)) {
popContextProvider();
}
node = node.return;
}
};
Loading

0 comments on commit 6eb50d2

Please sign in to comment.