@samitouri / QOS-React-2 / commits / 4d9f850065

Re-throw errors thrown by the renderer at the root in the complete phase (#18029)

* Re-throw errors thrown by the renderer at the root React treats errors thrown at the root as a fatal because there's no parent component that can capture it. (This is distinct from an "uncaught error" that isn't wrapped in an error boundary, because in that case we can fall back to deleting the whole tree -- not great, but at least the error is contained to a single root, and React is left in a consistent state.) It turns out we didn't have a test case for this path. The only way it can happen is if the renderer's host config throws. We had similar test cases for host components, but none for the host root. This adds a new test case and fixes a bug where React would keep retrying the root because the `workInProgress` pointer was not advanced to the next fiber. (Which in this case is `null`, since it's the root.) We could consider in the future trying to gracefully exit from certain types of root errors without leaving React in an inconsistent state. For example, we should be able to gracefully exit from errors thrown in the begin phase. For now, I'm treating it like an internal invariant and immediately exiting. * Add comment

Andrew Clark committed Feb 18, 2020 at 15:55 UTC 4d9f8500651c5d1e19d8ec9a2359d5476a53814b
3 files changed +26
packages/react-noop-renderer/src/createReactNoop.js
+7
@@ -533,6 +533,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
533 newChildren: Array<Instance | TextInstance>,
534 ): void {
535 container.pendingChildren = newChildren;
536 + if (
537 + newChildren.length === 1 &&
538 + newChildren[0].text === 'Error when completing root'
539 + ) {
540 + // Trigger an error for testing purposes
541 + throw Error('Error when completing root');
542 + }
543 },
544
545 replaceContainerChildren(
packages/react-reconciler/src/ReactFiberWorkLoop.js
+7
@@ -1285,6 +1285,13 @@ function handleError(root, thrownValue) {
1285 // boundary.
1286 workInProgressRootExitStatus = RootFatalErrored;
1287 workInProgressRootFatalError = thrownValue;
1288 + // Set `workInProgress` to null. This represents advancing to the next
1289 + // sibling, or the parent if there are no siblings. But since the root
1290 + // has no siblings nor a parent, we set it to null. Usually this is
1291 + // handled by `completeUnitOfWork` or `unwindWork`, but since we're
1292 + // interntionally not calling those, we need set it here.
1293 + // TODO: Consider calling `unwindWork` to pop the contexts.
1294 + workInProgress = null;
1295 return null;
1296 }
1297
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+12
@@ -1658,4 +1658,16 @@ describe('ReactIncrementalErrorHandling', () => {
1658 'Please update the following components: Provider',
1659 ]);
1660 });
1661 +
1662 + if (global.__PERSISTENT__) {
1663 + it('regression test: should fatal if error is thrown at the root', () => {
1664 + const root = ReactNoop.createRoot();
1665 + root.render('Error when completing root');
1666 + expect(Scheduler).toFlushAndThrow('Error when completing root');
1667 +
1668 + const blockingRoot = ReactNoop.createBlockingRoot();
1669 + blockingRoot.render('Error when completing root');
1670 + expect(Scheduler).toFlushAndThrow('Error when completing root');
1671 + });
1672 + }
1673 });