Error boundaries can only capture errors once per batch
Subsequent failures should propagate the error to the next boundary. The conceptual model for this is try { render(); } catch (error) { setStateToRecover(); render(); } Closes #8485
Andrew Clark committed
Dec 5, 2016 at 11:16 UTC
7e8221471795033b098d90ace6083ab652ee3e9e
2 files changed
+160
-160
src/renderers/shared/fiber/ReactFiberScheduler.js
+131
-136
@@ -99,13 +99,19 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
99
let isAnimationCallbackScheduled : boolean = false;
100
let isDeferredCallbackScheduled : boolean = false;
101
102
+ // Keep track of which fibers have captured an error that need to be handled.
103
+ // Work is removed from this collection after unstable_handleError is called.
104
let capturedErrors : Map<Fiber, Error> | null = null;
103
- let firstUncaughtError : Error | null = null;
104
-
105
- let isCommitting : boolean = false;
105
+ // Keep track of which fibers have failed during the current batch of work.
106
+ // This is a different set than capturedErrors, because it is not reset until
107
+ // the end of the batch. This is needed to propagate errors correctly if a
108
+ // subtree fails more than once.
109
+ let failedBoundaries : Set<Fiber> | null = null;
110
// Error boundaries that captured an error during the current commit.
111
let commitPhaseBoundaries : Set<Fiber> | null = null;
112
+ let firstUncaughtError : Error | null = null;
113
114
+ let isCommitting : boolean = false;
115
let isUnmounting : boolean = false;
116
117
function scheduleAnimationCallback(callback) {
@@ -508,126 +514,83 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
514
performWork(AnimationPriority);
515
}
516
511
- function workLoop(priorityLevel) {
512
- outer: while (true) {
513
- if (!nextUnitOfWork) {
517
+ function errorLoop() {
518
+ // Keep performing work until there are no more errors
519
+ while (capturedErrors && capturedErrors.size) {
520
+ if (!nextUnitOfWork && !pendingCommit) {
521
nextUnitOfWork = findNextUnitOfWork();
522
}
516
-
517
- // If there are errors, use a forked version of performUnitOfWork
518
- if (capturedErrors && capturedErrors.size) {
519
- while (true) {
520
- if (!nextUnitOfWork && !pendingCommit) {
521
- nextUnitOfWork = findNextUnitOfWork();
522
- }
523
-
524
- // If we're performing Task work and we're inside a deferred work batch,
525
- // then we shouldn't commit any work yet. Defer it to the next frame.
526
- // TODO: Is there a better way check if the pending commit should be
527
- // deferred? It's not as simple as retaining work priority because
528
- // the root may receive a lower priority update before the completed
529
- // work is flushed, causing the new update to be up-prioritized.
530
- if (
531
- pendingCommit &&
532
- !(priorityLevel === TaskPriority && isPerformingDeferredWork)
533
- ) {
534
- commitAllWork(pendingCommit);
535
- } else if (nextUnitOfWork && nextPriorityLevel !== NoWork && nextPriorityLevel <= priorityLevel) {
536
- if (isFailedWork(nextUnitOfWork)) {
537
- nextUnitOfWork = performFailedUnitOfWork(nextUnitOfWork);
538
- } else {
539
- nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
540
- }
541
- } else {
542
- return;
543
- }
544
- }
545
- }
546
-
547
- // Otherwise, handle the normal, faster case where there are no errors:
548
- while (true) {
549
- if (!nextUnitOfWork && !pendingCommit) {
550
- nextUnitOfWork = findNextUnitOfWork();
551
- }
552
-
553
- // If we're performing Task work and we're inside a deferred work batch,
554
- // then we shouldn't commit any work yet. Defer it to the next frame.
555
- // TODO: Is there a better way check if the pending commit should be
556
- // deferred? It's not as simple as retaining work priority because
557
- // the root may receive a lower priority update before the completed
558
- // work is flushed, causing the new update to be up-prioritized.
559
- if (
560
- pendingCommit &&
561
- !(priorityLevel === TaskPriority && isPerformingDeferredWork)
562
- ) {
563
- const noError = commitAllWork(pendingCommit);
564
- if (!noError) {
565
- // There was an error during the commit phase. Restart the outer
566
- // loop so we can switch to the slower version of the work loop that
567
- // checks for failed work.
568
- continue outer;
569
- }
570
- } else if (nextUnitOfWork && nextPriorityLevel !== NoWork && nextPriorityLevel <= priorityLevel) {
571
- nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
523
+ if (pendingCommit) {
524
+ commitAllWork(pendingCommit);
525
+ } else if (nextUnitOfWork) {
526
+ if (hasCapturedError(nextUnitOfWork)) {
527
+ // Use a forked version of performUnitOfWork
528
+ nextUnitOfWork = performFailedUnitOfWork(nextUnitOfWork);
529
} else {
573
- return;
530
+ nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
531
}
532
+ } else {
533
+ return;
534
}
535
}
536
}
537
579
- function deferredWorkLoop(deadline) {
580
- outer: while (true) {
581
- if (!nextUnitOfWork) {
538
+ function workLoop(priorityLevel) {
539
+ while (true) {
540
+ if (!nextUnitOfWork && !pendingCommit) {
541
nextUnitOfWork = findNextUnitOfWork();
542
}
543
585
- // If there are errors, use a forked version of performUnitOfWork
586
- if (capturedErrors && capturedErrors.size) {
587
- while (true) {
588
- if (deadline.timeRemaining() > timeHeuristicForUnitOfWork) {
589
- if (!nextUnitOfWork && !pendingCommit) {
590
- nextUnitOfWork = findNextUnitOfWork();
591
- }
592
- if (pendingCommit) {
593
- commitAllWork(pendingCommit);
594
- } else if (nextUnitOfWork) {
595
- if (isFailedWork(nextUnitOfWork)) {
596
- nextUnitOfWork = performFailedUnitOfWork(nextUnitOfWork);
597
- } else {
598
- nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
599
- }
600
- } else {
601
- return;
602
- }
603
- } else {
604
- return;
605
- }
544
+ // If we're performing Task work and we're inside a deferred work batch,
545
+ // then we shouldn't commit any work yet. Defer it to the next frame.
546
+ // TODO: Is there a better way check if the pending commit should be
547
+ // deferred? It's not as simple as retaining work priority because
548
+ // the root may receive a lower priority update before the completed
549
+ // work is flushed, causing the new update to be up-prioritized.
550
+ if (
551
+ pendingCommit &&
552
+ !(priorityLevel === TaskPriority && isPerformingDeferredWork)
553
+ ) {
554
+ const noError = commitAllWork(pendingCommit);
555
+ if (!noError) {
556
+ // There was an error during the commit phase. Clear them before
557
+ // continuing.
558
+ errorLoop();
559
+ continue;
560
}
561
+ } else if (nextUnitOfWork && nextPriorityLevel !== NoWork && nextPriorityLevel <= priorityLevel) {
562
+ nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
563
+ } else {
564
+ return;
565
}
566
+ }
567
+ }
568
609
- // Otherwise, handle the normal, faster case where there are no errors:
610
- while (true) {
611
- if (deadline.timeRemaining() > timeHeuristicForUnitOfWork) {
612
- if (!nextUnitOfWork && !pendingCommit) {
613
- nextUnitOfWork = findNextUnitOfWork();
614
- }
615
- if (pendingCommit) {
616
- const noError = commitAllWork(pendingCommit);
617
- if (!noError) {
618
- // There was an error during the commit phase. Restart the outer
619
- // loop so we can switch to the slower version of the work loop that
620
- // checks for failed work.
621
- continue outer;
622
- }
623
- } else if (nextUnitOfWork) {
624
- nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
625
- } else {
626
- return;
569
+ function deferredWorkLoop(deadline) {
570
+ if (!nextUnitOfWork) {
571
+ nextUnitOfWork = findNextUnitOfWork();
572
+ }
573
+
574
+ while (true) {
575
+ if (deadline.timeRemaining() > timeHeuristicForUnitOfWork) {
576
+ if (!nextUnitOfWork && !pendingCommit) {
577
+ nextUnitOfWork = findNextUnitOfWork();
578
+ }
579
+ if (pendingCommit) {
580
+ const noError = commitAllWork(pendingCommit);
581
+ if (!noError) {
582
+ // There was an error during the commit phase. Clear them before
583
+ // continuing.
584
+ errorLoop();
585
+ continue;
586
}
587
+ } else if (nextUnitOfWork) {
588
+ nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
589
} else {
590
return;
591
}
592
+ } else {
593
+ return;
594
}
595
}
596
}
@@ -649,6 +612,11 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
612
// work. The loop we're in now runs infrequently: to flush task work at
613
// the end of a frame, or to restart after an error.
614
while (priorityLevel !== NoWork) {
615
+ // If there are errors, clear them first.
616
+ if (capturedErrors && capturedErrors.size) {
617
+ errorLoop();
618
+ }
619
+
620
if (priorityLevel >= HighPriority) {
621
if (!deadline) {
622
throw new Error('Cannot perform deferred work without a deadline.');
@@ -724,9 +692,8 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
692
// We're done performing work. Time to clean up.
693
isPerformingWork = false;
694
isPerformingDeferredWork = false;
727
- if (capturedErrors && !capturedErrors.size) {
728
- capturedErrors = null;
729
- }
695
+ capturedErrors = null;
696
+ failedBoundaries = null;
697
698
// It's now safe to throw the first uncaught error.
699
if (firstUncaughtError) {
@@ -743,14 +710,6 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
710
// It is no longer valid because this unit of work failed.
711
nextUnitOfWork = null;
712
746
- // Ignore this error if it's the result of unmounting a failed boundary
747
- if (failedWork &&
748
- isUnmounting &&
749
- capturedErrors &&
750
- capturedErrors.has(failedWork)) {
751
- return null;
752
- }
753
-
713
// Search for the nearest error boundary.
714
let boundary : Fiber | null = null;
715
if (failedWork) {
@@ -765,8 +724,33 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
724
if (node.tag === ClassComponent) {
725
const instance = node.stateNode;
726
if (typeof instance.unstable_handleError === 'function') {
768
- // Found an error boundary!
769
- boundary = node;
727
+ if (isFailedBoundary(node)) {
728
+ // This boundary is already in a failed state. The error should
729
+ // propagate to the next boundary — except in the
730
+ // following cases:
731
+
732
+ // If we're currently unmounting, that means this error was
733
+ // thrown while unmounting a failed subtree. We should ignore
734
+ // the error.
735
+ if (isUnmounting) {
736
+ return null;
737
+ }
738
+
739
+ // If we're in the commit phase, we should check to see if
740
+ // this boundary already captured an error during this commit.
741
+ // This case exists because multiple errors can be thrown during
742
+ // a single commit without interruption.
743
+ if (commitPhaseBoundaries && (
744
+ commitPhaseBoundaries.has(node) ||
745
+ (node.alternate) && commitPhaseBoundaries.has(node.alternate)
746
+ )) {
747
+ // If so, we should ignore this error.
748
+ return null;
749
+ }
750
+ } else {
751
+ // Found an error boundary!
752
+ boundary = node;
753
+ }
754
}
755
} else if (node.tag === HostRoot) {
756
// Treat the root like a no-op error boundary.
@@ -778,6 +762,13 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
762
}
763
764
if (boundary) {
765
+ // Add to the collection of failed boundaries. This lets us know that
766
+ // subsequent errors in this subtree should propagate to the next boundary.
767
+ if (!failedBoundaries) {
768
+ failedBoundaries = new Set();
769
+ }
770
+ failedBoundaries.add(boundary);
771
+
772
// Add to the collection of captured errors. This is stored as a global
773
// map of errors keyed by the boundaries that capture them. We mostly
774
// use this Map as a Set; it's a Map only to avoid adding a field to Fiber
@@ -786,24 +777,20 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
777
capturedErrors = new Map();
778
}
779
789
- // Ensure that neither this boundary nor its alternate has captured an
790
- // error already.
791
- if (!isFailedWork(boundary)) {
792
- capturedErrors.set(boundary, error);
793
- // If we're in the commit phase, defer scheduling an update on the
794
- // boundary until after the commit is complete
795
- if (isCommitting) {
796
- if (!commitPhaseBoundaries) {
797
- commitPhaseBoundaries = new Set();
798
- }
799
- commitPhaseBoundaries.add(boundary);
800
- } else {
801
- // Otherwise, schedule an update now. Error recovery has Task priority.
802
- const previousPriorityContext = priorityContext;
803
- priorityContext = TaskPriority;
804
- scheduleUpdate(boundary);
805
- priorityContext = previousPriorityContext;
780
+ capturedErrors.set(boundary, error);
781
+ // If we're in the commit phase, defer scheduling an update on the
782
+ // boundary until after the commit is complete
783
+ if (isCommitting) {
784
+ if (!commitPhaseBoundaries) {
785
+ commitPhaseBoundaries = new Set();
786
}
787
+ commitPhaseBoundaries.add(boundary);
788
+ } else {
789
+ // Otherwise, schedule an update now. Error recovery has Task priority.
790
+ const previousPriorityContext = priorityContext;
791
+ priorityContext = TaskPriority;
792
+ scheduleUpdate(boundary);
793
+ priorityContext = previousPriorityContext;
794
}
795
return boundary;
796
} else if (!firstUncaughtError) {
@@ -813,13 +800,21 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
800
return null;
801
}
802
816
- function isFailedWork(fiber : Fiber) : boolean {
803
+ function hasCapturedError(fiber : Fiber) : boolean {
804
return Boolean(
805
capturedErrors &&
806
(capturedErrors.has(fiber) || (fiber.alternate && capturedErrors.has(fiber.alternate)))
807
);
808
}
809
810
+ function isFailedBoundary(fiber : Fiber) : boolean {
811
+ const res = Boolean(
812
+ failedBoundaries &&
813
+ (failedBoundaries.has(fiber) || (fiber.alternate && failedBoundaries.has(fiber.alternate)))
814
+ );
815
+ return res;
816
+ }
817
+
818
function commitErrorHandling(effectfulFiber : Fiber) {
819
let error;
820
if (capturedErrors) {
src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
+29
-24
@@ -33,7 +33,7 @@ describe('ReactErrorBoundaries', () => {
33
var ErrorBoundary;
34
var ErrorMessage;
35
var NoopErrorBoundary;
36
- var RethrowErrorBoundary;
36
+ var RetryErrorBoundary;
37
var Normal;
38
39
beforeEach(() => {
@@ -484,32 +484,33 @@ describe('ReactErrorBoundaries', () => {
484
},
485
};
486
487
- RethrowErrorBoundary = class extends React.Component {
487
+ RetryErrorBoundary = class extends React.Component {
488
constructor(props) {
489
super(props);
490
- log.push('RethrowErrorBoundary constructor');
490
+ log.push('RetryErrorBoundary constructor');
491
}
492
render() {
493
- log.push('RethrowErrorBoundary render');
493
+ log.push('RetryErrorBoundary render');
494
return <BrokenRender />;
495
}
496
componentWillMount() {
497
- log.push('RethrowErrorBoundary componentWillMount');
497
+ log.push('RetryErrorBoundary componentWillMount');
498
}
499
componentDidMount() {
500
- log.push('RethrowErrorBoundary componentDidMount');
500
+ log.push('RetryErrorBoundary componentDidMount');
501
}
502
componentWillUnmount() {
503
- log.push('RethrowErrorBoundary componentWillUnmount');
503
+ log.push('RetryErrorBoundary componentWillUnmount');
504
}
505
unstable_handleError(error) {
506
- if (!ReactDOMFeatureFlags.useFiber) {
507
- log.push('RethrowErrorBoundary unstable_handleError [*]');
506
+ if (ReactDOMFeatureFlags.useFiber) {
507
+ log.push('RetryErrorBoundary unstable_handleError [!]');
508
+ // In Fiber, calling setState() (and failing) is treated as a rethrow.
509
+ this.setState({});
510
+ } else {
511
+ log.push('RetryErrorBoundary unstable_handleError [*]');
512
// In Stack, not calling setState() is treated as a rethrow.
509
- return;
513
}
511
- log.push('RethrowErrorBoundary unstable_handleError [!]');
512
- throw error;
514
}
515
};
516
@@ -788,9 +789,9 @@ describe('ReactErrorBoundaries', () => {
789
var container = document.createElement('div');
790
ReactDOM.render(
791
<ErrorBoundary>
791
- <RethrowErrorBoundary>
792
+ <RetryErrorBoundary>
793
<BrokenRender />
793
- </RethrowErrorBoundary>
794
+ </RetryErrorBoundary>
795
</ErrorBoundary>,
796
container
797
);
@@ -799,20 +800,24 @@ describe('ReactErrorBoundaries', () => {
800
'ErrorBoundary constructor',
801
'ErrorBoundary componentWillMount',
802
'ErrorBoundary render success',
802
- 'RethrowErrorBoundary constructor',
803
- 'RethrowErrorBoundary componentWillMount',
804
- 'RethrowErrorBoundary render',
803
+ 'RetryErrorBoundary constructor',
804
+ 'RetryErrorBoundary componentWillMount',
805
+ 'RetryErrorBoundary render',
806
'BrokenRender constructor',
807
'BrokenRender componentWillMount',
808
'BrokenRender render [!]',
809
...(ReactDOMFeatureFlags.useFiber ? [
809
- // In Fiber, noop error boundaries render null
810
- 'RethrowErrorBoundary componentDidMount',
811
- 'RethrowErrorBoundary unstable_handleError [!]',
810
+ // In Fiber, failed error boundaries render null before attempting to recover
811
+ 'RetryErrorBoundary componentDidMount',
812
+ 'RetryErrorBoundary unstable_handleError [!]',
813
'ErrorBoundary componentDidMount',
813
- // The error got rethrown here.
814
+ // Retry
815
+ 'RetryErrorBoundary render',
816
+ 'BrokenRender constructor',
817
+ 'BrokenRender componentWillMount',
818
+ 'BrokenRender render [!]',
819
// This time, the error propagates to the higher boundary
815
- 'RethrowErrorBoundary componentWillUnmount',
820
+ 'RetryErrorBoundary componentWillUnmount',
821
'ErrorBoundary unstable_handleError',
822
// Render the error
823
'ErrorBoundary componentWillUpdate',
@@ -821,8 +826,8 @@ describe('ReactErrorBoundaries', () => {
826
] : [
827
// The first error boundary catches the error.
828
// However, it doesn't adjust its state so next render will also fail.
824
- 'RethrowErrorBoundary unstable_handleError [*]',
825
- 'RethrowErrorBoundary render',
829
+ 'RetryErrorBoundary unstable_handleError [*]',
830
+ 'RetryErrorBoundary render',
831
'BrokenRender constructor',
832
'BrokenRender componentWillMount',
833
'BrokenRender render [!]',