@samitouri / QOS-React / commits / c80f390823

Make error handling more resilient

* Ensure that errors in one root don't prevent work on another root * Fix an issue where boundary state change would get ignored in incremental mode

Dan Abramov committed Nov 4, 2016 at 22:44 UTC c80f390823266385df75fe52be397210e23c0301
5 files changed +421 -81
scripts/fiber/tests-passing.txt
+8
@@ -791,7 +791,15 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
791 * skips will/DidUpdate when bailing unless an update was already in progress
792 * performs batched updates at the end of the batch
793 * can nest batchedUpdates
794 +
795 +src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
796 +* catches render error in a boundary during mounting
797 * propagates an error from a noop error boundary
798 +* can schedule updates after uncaught error in render on mount
799 +* can schedule updates after uncaught error in render on update
800 +* can schedule updates after uncaught error during umounting
801 +* continues work on other roots despite caught errors
802 +* continues work on other roots despite uncaught errors
803
804 src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
805 * handles isMounted even when the initial render is deferred
src/renderers/shared/fiber/ReactFiberClassComponent.js
+7 -6
@@ -166,12 +166,13 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
166
167 if (typeof newInstance.componentWillMount === 'function') {
168 newInstance.componentWillMount();
169 - // If we had additional state updates during this life-cycle, let's
170 - // process them now.
171 - const newUpdateQueue = workInProgress.updateQueue;
172 - if (newUpdateQueue) {
173 - newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps);
174 - }
169 + }
170 + // If we had additional state updates, process them now.
171 + // They may be from componentWillMount() or from error boundary's setState()
172 + // during initial mounting.
173 + const newUpdateQueue = workInProgress.updateQueue;
174 + if (newUpdateQueue) {
175 + newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps);
176 }
177 return true;
178 }
src/renderers/shared/fiber/ReactFiberCommitWork.js
+1 -1
@@ -33,7 +33,7 @@ var {
33
34 module.exports = function<T, P, I, TI, C>(
35 config : HostConfig<T, P, I, TI, C>,
36 - trapError : (fiber : Fiber, error: Error, isUnmounting : boolean) => void
36 + trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void
37 ) {
38
39 const updateContainer = config.updateContainer;
src/renderers/shared/fiber/ReactFiberScheduler.js
+173 -74
@@ -60,6 +60,7 @@ var timeHeuristicForUnitOfWork = 1;
60
61 type TrappedError = {
62 boundary: Fiber | null,
63 + root: FiberRoot | null,
64 error: any,
65 };
66
@@ -445,20 +446,23 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
446 }
447
448 isPerformingTaskWork = true;
448 - nextUnitOfWork = findNextUnitOfWork();
449 - while (nextUnitOfWork &&
450 - nextPriorityLevel === TaskPriority) {
451 - nextUnitOfWork =
452 - performUnitOfWork(nextUnitOfWork);
449 + try {
450 + nextUnitOfWork = findNextUnitOfWork();
451 + while (nextUnitOfWork &&
452 + nextPriorityLevel === TaskPriority) {
453 + nextUnitOfWork =
454 + performUnitOfWork(nextUnitOfWork);
455
454 - if (!nextUnitOfWork) {
455 - nextUnitOfWork = findNextUnitOfWork();
456 + if (!nextUnitOfWork) {
457 + nextUnitOfWork = findNextUnitOfWork();
458 + }
459 }
460 + if (nextUnitOfWork) {
461 + scheduleCallbackAtPriority(nextPriorityLevel);
462 + }
463 + } finally {
464 + isPerformingTaskWork = false;
465 }
458 - if (nextUnitOfWork) {
459 - scheduleCallbackAtPriority(nextPriorityLevel);
460 - }
461 - isPerformingTaskWork = false;
466 }
467
468 function performTaskWork() {
@@ -466,53 +470,76 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
470 }
471
472 function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) {
473 + // Keep track of the first error we need to surface to the user.
474 + let firstUncaughtError = null;
475 +
476 // The exact priority level doesn't matter, so long as it's in range of the
477 // work (sync, animation, deferred) being performed.
471 - try {
472 - switch (priorityLevel) {
473 - case SynchronousPriority:
474 - performSynchronousWorkUnsafe();
475 - break;
476 - case TaskPriority:
477 - if (!isPerformingTaskWork) {
478 - performTaskWorkUnsafe();
479 - }
480 - break;
481 - case AnimationPriority:
482 - performAnimationWorkUnsafe();
483 - break;
484 - case HighPriority:
485 - case LowPriority:
486 - case OffscreenPriority:
487 - if (!deadline) {
488 - throw new Error('No deadline');
489 - } else {
490 - performDeferredWorkUnsafe(deadline);
491 - }
492 - break;
493 - default:
494 - break;
478 + while (true) {
479 + try {
480 + switch (priorityLevel) {
481 + case SynchronousPriority:
482 + performSynchronousWorkUnsafe();
483 + break;
484 + case TaskPriority:
485 + if (!isPerformingTaskWork) {
486 + performTaskWorkUnsafe();
487 + }
488 + break;
489 + case AnimationPriority:
490 + performAnimationWorkUnsafe();
491 + break;
492 + case HighPriority:
493 + case LowPriority:
494 + case OffscreenPriority:
495 + if (!deadline) {
496 + throw new Error('No deadline');
497 + } else {
498 + performDeferredWorkUnsafe(deadline);
499 + }
500 + break;
501 + default:
502 + break;
503 + }
504 + } catch (error) {
505 + trapError(nextUnitOfWork, error, false);
506 + }
507 +
508 + // If there were errors and we aren't already handling them, handle them now
509 + if (nextTrappedErrors && !activeErrorBoundaries) {
510 + const nextUncaughtError = handleErrors();
511 + firstUncaughtError = firstUncaughtError || nextUncaughtError;
512 + } else {
513 + // We've done our work.
514 + break;
515 + }
516 +
517 + // An error interrupted us. Now that it is handled, we may find more work.
518 + // It's safe because any roots with uncaught errors have been unscheduled.
519 + nextUnitOfWork = findNextUnitOfWork();
520 + if (!nextUnitOfWork) {
521 + // We found no other work we could do.
522 + break;
523 }
496 - } catch (error) {
497 - trapError(nextUnitOfWork, error, false);
524 }
525
500 - // If there were errors and we aren't already handling them, handle them now
501 - if (nextTrappedErrors && !activeErrorBoundaries) {
502 - handleErrors();
526 + // Now it's safe to surface the first uncaught error to the user.
527 + if (firstUncaughtError) {
528 + throw firstUncaughtError;
529 }
530 }
531
506 - function handleErrors() : void {
532 + function handleErrors() : Error | null {
533 if (activeErrorBoundaries) {
534 throw new Error('Already handling errors');
535 }
536
537 // Start tracking active boundaries.
538 activeErrorBoundaries = new Set();
513 -
514 - // If we find unhandled errors, we'll only rethrow the first one.
539 + // If we find unhandled errors, we'll only remember the first one.
540 let firstUncaughtError = null;
541 + // Keep track of which roots have fataled and need to be unscheduled.
542 + const rootsWithUncaughtErrors = new Set();
543
544 // All work created by error boundaries should have Task priority
545 // so that it finishes before this function exits.
@@ -527,8 +554,23 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
554 const trappedError = nextTrappedErrors.shift();
555 const boundary = trappedError.boundary;
556 const error = trappedError.error;
557 + const root = trappedError.root;
558 if (!boundary) {
559 firstUncaughtError = firstUncaughtError || error;
560 + if (root) {
561 + // Remember to unschedule this particular root since it fataled
562 + // and we can't do more work on it. This lets us continue working on
563 + // other roots even if one of them fails before rethrowing the error.
564 + rootsWithUncaughtErrors.add(root);
565 + } else {
566 + // Normally we should know which root caused the error, so it is
567 + // unusual if we end up here. Since we assume this function always
568 + // unschedules failed roots, our only resort is to completely
569 + // unschedule all roots. Otherwise we may get into an infinite loop
570 + // trying to resume work and finding the failing but unknown root again.
571 + nextScheduledRoot = null;
572 + lastScheduledRoot = null;
573 + }
574 continue;
575 }
576 // Don't visit boundaries twice.
@@ -561,47 +603,28 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
603 try {
604 performTaskWorkUnsafe();
605 } catch (error) {
564 - isPerformingTaskWork = false;
606 trapError(nextUnitOfWork, error, false);
607 }
608 }
609
610 + // Unschedule the roots that fataled so that we can later schedule
611 + // other updates to the same roots and potentially replace them.
612 + if (rootsWithUncaughtErrors.size) {
613 + unscheduleWork(rootsWithUncaughtErrors);
614 + }
615 +
616 nextTrappedErrors = null;
617 activeErrorBoundaries = null;
618 priorityContext = previousPriorityContext;
619
573 - if (firstUncaughtError) {
574 - // We need to make sure any future root can get scheduled despite these errors.
575 - // Currently after throwing, nothing gets scheduled because these fields are set.
576 - // FIXME: this is likely a wrong fix! It's still better than ignoring updates though.
577 - nextScheduledRoot = null;
578 - lastScheduledRoot = null;
579 - throw firstUncaughtError;
580 - }
620 + // Return the error so we can rethrow after handling other roots.
621 + return firstUncaughtError;
622 }
623
583 - function findClosestErrorBoundary(fiber : Fiber): Fiber | null {
584 - let maybeErrorBoundary = fiber.return;
585 - while (maybeErrorBoundary) {
586 - if (maybeErrorBoundary.tag === ClassComponent) {
587 - const instance = maybeErrorBoundary.stateNode;
588 - const isErrorBoundary = typeof instance.unstable_handleError === 'function';
589 - if (isErrorBoundary) {
590 - const isHandlingAnotherError = (
591 - activeErrorBoundaries !== null &&
592 - activeErrorBoundaries.has(maybeErrorBoundary)
593 - );
594 - if (!isHandlingAnotherError) {
595 - return maybeErrorBoundary;
596 - }
597 - }
598 - }
599 - maybeErrorBoundary = maybeErrorBoundary.return;
600 - }
601 - return null;
602 - }
624 + function trapError(failedFiber : Fiber | null, error : any, isUnmounting : boolean) : void {
625 + // Don't try to start here again on next flush.
626 + nextUnitOfWork = null;
627
604 - function trapError(fiber : Fiber | null, error : any, isUnmounting : boolean) : void {
628 // It is no longer valid because we exited the user code.
629 ReactCurrentOwner.current = null;
630
@@ -611,15 +634,91 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
634 return;
635 }
636
637 + let boundary = null;
638 + let root = null;
639 +
640 + // Search for the parent error boundary and root.
641 + let fiber = failedFiber;
642 + while (fiber) {
643 + const parent = fiber.return;
644 + if (parent) {
645 + if (parent.tag === ClassComponent && boundary === null) {
646 + // Consider a candidate for parent boundary.
647 + const instance = parent.stateNode;
648 + const isBoundary = typeof instance.unstable_handleError === 'function';
649 + if (isBoundary) {
650 + // Skip boundaries that are already active so errors can propagate.
651 + const isBoundaryAlreadyHandlingAnotherError = (
652 + activeErrorBoundaries !== null &&
653 + activeErrorBoundaries.has(parent)
654 + );
655 + if (!isBoundaryAlreadyHandlingAnotherError) {
656 + // We found the boundary.
657 + boundary = parent;
658 + }
659 + }
660 + }
661 + } else if (fiber.tag === HostContainer) {
662 + // We found the root.
663 + root = (fiber.stateNode : FiberRoot);
664 + } else {
665 + throw new Error('Invalid root');
666 + }
667 + fiber = parent;
668 + }
669 +
670 if (!nextTrappedErrors) {
671 nextTrappedErrors = [];
672 }
673 nextTrappedErrors.push({
618 - boundary: fiber ? findClosestErrorBoundary(fiber) : null,
674 + boundary,
675 error,
676 + root,
677 });
678 }
679
680 + function unscheduleWork(unscheduledRoots : Set<FiberRoot>) {
681 + // New pointers for the new linked list
682 + let newNextScheduledRoot = null;
683 + let newLastScheduledRoot = null;
684 +
685 + // Search for the first first root that should remain scheduled
686 + let root = nextScheduledRoot;
687 + while (root) {
688 + if (unscheduledRoots.has(root)) {
689 + root.isScheduled = false;
690 + // Read the next pointer now.
691 + // We need to clear it in case this root gets scheduled again later.
692 + const next = root.nextScheduledRoot;
693 + root.nextScheduledRoot = null;
694 + root = next;
695 + } else {
696 + newNextScheduledRoot = newNextScheduledRoot || root;
697 + newLastScheduledRoot = root;
698 + break;
699 + }
700 + }
701 +
702 + // Find the other roots that should remain scheduled
703 + while (root) {
704 + const next = root.nextScheduledRoot;
705 + if (next && unscheduledRoots.has(next)) {
706 + // Skip the next root.
707 + next.isScheduled = false;
708 + // Move forward.
709 + root.nextScheduledRoot = next.nextScheduledRoot;
710 + } else {
711 + // Keep the root and move forward.
712 + newLastScheduledRoot = root;
713 + root = next;
714 + }
715 + }
716 +
717 + // We're done filtering
718 + nextScheduledRoot = newNextScheduledRoot;
719 + lastScheduledRoot = newLastScheduledRoot;
720 + }
721 +
722 function scheduleWork(root : FiberRoot) {
723 let priorityLevel = priorityContext;
724
src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
+232
@@ -21,6 +21,37 @@ describe('ReactIncrementalErrorHandling', () => {
21 ReactNoop = require('ReactNoop');
22 });
23
24 + function span(prop) {
25 + return { type: 'span', children: [], prop };
26 + }
27 +
28 + it('catches render error in a boundary during mounting', () => {
29 + class ErrorBoundary extends React.Component {
30 + state = {error: null};
31 + unstable_handleError(error) {
32 + this.setState({error});
33 + }
34 + render() {
35 + if (this.state.error) {
36 + return <span prop={`Caught an error: ${this.state.error.message}.`} />;
37 + }
38 + return this.props.children;
39 + }
40 + }
41 +
42 + function BrokenRender(props) {
43 + throw new Error('Hello');
44 + }
45 +
46 + ReactNoop.render(
47 + <ErrorBoundary>
48 + <BrokenRender />
49 + </ErrorBoundary>
50 + );
51 + ReactNoop.flush();
52 + expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello.')]);
53 + });
54 +
55 it('propagates an error from a noop error boundary', () => {
56 class NoopBoundary extends React.Component {
57 unstable_handleError() {
@@ -43,4 +74,205 @@ describe('ReactIncrementalErrorHandling', () => {
74
75 expect(ReactNoop.flush).toThrow('render error');
76 });
77 +
78 + it('can schedule updates after uncaught error in render on mount', () => {
79 + var ops = [];
80 +
81 + function BrokenRender() {
82 + ops.push('BrokenRender');
83 + throw new Error('Hello');
84 + }
85 +
86 + function Foo() {
87 + ops.push('Foo');
88 + return null;
89 + }
90 +
91 + ReactNoop.render(<BrokenRender />);
92 + expect(() => {
93 + ReactNoop.flush();
94 + }).toThrow('Hello');
95 + expect(ops).toEqual(['BrokenRender']);
96 +
97 + ops = [];
98 + ReactNoop.render(<Foo />);
99 + ReactNoop.flush();
100 + expect(ops).toEqual(['Foo']);
101 + });
102 +
103 + it('can schedule updates after uncaught error in render on update', () => {
104 + var ops = [];
105 +
106 + function BrokenRender(props) {
107 + ops.push('BrokenRender');
108 + if (props.throw) {
109 + throw new Error('Hello');
110 + }
111 + }
112 +
113 + function Foo() {
114 + ops.push('Foo');
115 + return null;
116 + }
117 +
118 + ReactNoop.render(<BrokenRender throw={false} />);
119 + ReactNoop.flush();
120 + ops = [];
121 +
122 + expect(() => {
123 + ReactNoop.render(<BrokenRender throw={true} />);
124 + ReactNoop.flush();
125 + }).toThrow('Hello');
126 + expect(ops).toEqual(['BrokenRender']);
127 +
128 + ops = [];
129 + ReactNoop.render(<Foo />);
130 + ReactNoop.flush();
131 + expect(ops).toEqual(['Foo']);
132 + });
133 +
134 + it('can schedule updates after uncaught error during umounting', () => {
135 + var ops = [];
136 +
137 + class BrokenComponentWillUnmount extends React.Component {
138 + render() {
139 + return <div />;
140 + }
141 + componentWillUnmount() {
142 + throw new Error('Hello');
143 + }
144 + }
145 +
146 + function Foo() {
147 + ops.push('Foo');
148 + return null;
149 + }
150 +
151 + ReactNoop.render(<BrokenComponentWillUnmount />);
152 + ReactNoop.flush();
153 +
154 + expect(() => {
155 + ReactNoop.render(<div />);
156 + ReactNoop.flush();
157 + }).toThrow('Hello');
158 +
159 + ops = [];
160 + ReactNoop.render(<Foo />);
161 + ReactNoop.flush();
162 + expect(ops).toEqual(['Foo']);
163 + });
164 +
165 + it('continues work on other roots despite caught errors', () => {
166 + class ErrorBoundary extends React.Component {
167 + state = {error: null};
168 + unstable_handleError(error) {
169 + this.setState({error});
170 + }
171 + render() {
172 + if (this.state.error) {
173 + return <span prop={`Caught an error: ${this.state.error.message}.`} />;
174 + }
175 + return this.props.children;
176 + }
177 + }
178 +
179 + function BrokenRender(props) {
180 + throw new Error('Hello');
181 + }
182 +
183 + ReactNoop.renderToRootWithID(
184 + <ErrorBoundary>
185 + <BrokenRender />
186 + </ErrorBoundary>,
187 + 'a'
188 + );
189 + ReactNoop.renderToRootWithID(<span prop="b:1" />, 'b');
190 + ReactNoop.flush();
191 + expect(ReactNoop.getChildren('a')).toEqual([span('Caught an error: Hello.')]);
192 + expect(ReactNoop.getChildren('b')).toEqual([span('b:1')]);
193 + });
194 +
195 + it('continues work on other roots despite uncaught errors', () => {
196 + function BrokenRender(props) {
197 + throw new Error('Hello');
198 + }
199 +
200 + ReactNoop.renderToRootWithID(<BrokenRender />, 'a');
201 + expect(() => {
202 + ReactNoop.flush();
203 + }).toThrow('Hello');
204 + expect(ReactNoop.getChildren('a')).toEqual([]);
205 +
206 + ReactNoop.renderToRootWithID(<BrokenRender />, 'a');
207 + ReactNoop.renderToRootWithID(<span prop="b:2" />, 'b');
208 + expect(() => {
209 + ReactNoop.flush();
210 + }).toThrow('Hello');
211 +
212 + expect(ReactNoop.getChildren('a')).toEqual([]);
213 + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]);
214 +
215 + ReactNoop.renderToRootWithID(<span prop="a:3" />, 'a');
216 + ReactNoop.renderToRootWithID(<BrokenRender />, 'b');
217 + expect(() => {
218 + ReactNoop.flush();
219 + }).toThrow('Hello');
220 + expect(ReactNoop.getChildren('a')).toEqual([span('a:3')]);
221 + // Currently we assume previous tree stays intact for fataled trees.
222 + // We may consider tearing it down in the future.
223 + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]);
224 +
225 + ReactNoop.renderToRootWithID(<span prop="a:4" />, 'a');
226 + ReactNoop.renderToRootWithID(<BrokenRender />, 'b');
227 + ReactNoop.renderToRootWithID(<span prop="c:4" />, 'c');
228 + expect(() => {
229 + ReactNoop.flush();
230 + }).toThrow('Hello');
231 + expect(ReactNoop.getChildren('a')).toEqual([span('a:4')]);
232 + expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]);
233 + expect(ReactNoop.getChildren('c')).toEqual([span('c:4')]);
234 +
235 + ReactNoop.renderToRootWithID(<span prop="a:5" />, 'a');
236 + ReactNoop.renderToRootWithID(<span prop="b:5" />, 'b');
237 + ReactNoop.renderToRootWithID(<span prop="c:5" />, 'c');
238 + ReactNoop.renderToRootWithID(<span prop="d:5" />, 'd');
239 + ReactNoop.renderToRootWithID(<BrokenRender />, 'e');
240 + expect(() => {
241 + ReactNoop.flush();
242 + }).toThrow('Hello');
243 + expect(ReactNoop.getChildren('a')).toEqual([span('a:5')]);
244 + expect(ReactNoop.getChildren('b')).toEqual([span('b:5')]);
245 + expect(ReactNoop.getChildren('c')).toEqual([span('c:5')]);
246 + expect(ReactNoop.getChildren('d')).toEqual([span('d:5')]);
247 + expect(ReactNoop.getChildren('e')).toEqual([]);
248 +
249 + ReactNoop.renderToRootWithID(<BrokenRender />, 'a');
250 + ReactNoop.renderToRootWithID(<span prop="b:6" />, 'b');
251 + ReactNoop.renderToRootWithID(<BrokenRender />, 'c');
252 + ReactNoop.renderToRootWithID(<span prop="d:6" />, 'd');
253 + ReactNoop.renderToRootWithID(<BrokenRender />, 'e');
254 + ReactNoop.renderToRootWithID(<span prop="f:6" />, 'f');
255 + expect(() => {
256 + ReactNoop.flush();
257 + }).toThrow('Hello');
258 + expect(ReactNoop.getChildren('a')).toEqual([span('a:5')]);
259 + expect(ReactNoop.getChildren('b')).toEqual([span('b:6')]);
260 + expect(ReactNoop.getChildren('c')).toEqual([span('c:5')]);
261 + expect(ReactNoop.getChildren('d')).toEqual([span('d:6')]);
262 + expect(ReactNoop.getChildren('e')).toEqual([]);
263 + expect(ReactNoop.getChildren('f')).toEqual([span('f:6')]);
264 +
265 + ReactNoop.unmountRootWithID('a');
266 + ReactNoop.unmountRootWithID('b');
267 + ReactNoop.unmountRootWithID('c');
268 + ReactNoop.unmountRootWithID('d');
269 + ReactNoop.unmountRootWithID('e');
270 + ReactNoop.unmountRootWithID('f');
271 + expect(ReactNoop.getChildren('a')).toEqual(null);
272 + expect(ReactNoop.getChildren('b')).toEqual(null);
273 + expect(ReactNoop.getChildren('c')).toEqual(null);
274 + expect(ReactNoop.getChildren('d')).toEqual(null);
275 + expect(ReactNoop.getChildren('e')).toEqual(null);
276 + expect(ReactNoop.getChildren('f')).toEqual(null);
277 + });
278 });