Ensure first error is thrown if HostRoot catches two errors in commit phase (#8923)
Previously, we were overwriting capturedErrors with the second error and throwing that one.
Ben Alpert committed
Feb 3, 2017 at 11:12 UTC
fa1087b42c9eb4c769f6d094a22377f9055c5a69
3 files changed
+74
-28
scripts/fiber/tests-passing.txt
+1
@@ -1484,6 +1484,7 @@ src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
1484
* lets different boundaries catch their own first errors
1485
* discards a bad root if the root component fails
1486
* renders empty output if error boundary does not handle the error
1487
+* passes first error when two errors happen in commit
1488
1489
src/renderers/shared/shared/__tests__/ReactIdentity-test.js
1490
* should allow key property to express identity
src/renderers/shared/fiber/ReactFiberScheduler.js
+31
-28
@@ -866,39 +866,42 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
866
errorBoundaryFound = true;
867
errorBoundaryName = getComponentName(node);
868
869
- if (isFailedBoundary(node)) {
870
- // This boundary is already in a failed state. The error should
871
- // propagate to the next boundary — except in the
872
- // following cases:
873
-
874
- // If we're currently unmounting, that means this error was
875
- // thrown while unmounting a failed subtree. We should ignore
876
- // the error.
877
- if (isUnmounting) {
878
- return null;
879
- }
880
-
881
- // If we're in the commit phase, we should check to see if
882
- // this boundary already captured an error during this commit.
883
- // This case exists because multiple errors can be thrown during
884
- // a single commit without interruption.
885
- if (commitPhaseBoundaries && (
886
- commitPhaseBoundaries.has(node) ||
887
- (node.alternate) && commitPhaseBoundaries.has(node.alternate)
888
- )) {
889
- // If so, we should ignore this error.
890
- return null;
891
- }
892
- } else {
893
- // Found an error boundary!
894
- boundary = node;
895
- willRetry = true;
896
- }
869
+ // Found an error boundary!
870
+ boundary = node;
871
+ willRetry = true;
872
}
873
} else if (node.tag === HostRoot) {
874
// Treat the root like a no-op error boundary.
875
boundary = node;
876
}
877
+
878
+ if (isFailedBoundary(node)) {
879
+ // This boundary is already in a failed state.
880
+
881
+ // If we're currently unmounting, that means this error was
882
+ // thrown while unmounting a failed subtree. We should ignore
883
+ // the error.
884
+ if (isUnmounting) {
885
+ return null;
886
+ }
887
+
888
+ // If we're in the commit phase, we should check to see if
889
+ // this boundary already captured an error during this commit.
890
+ // This case exists because multiple errors can be thrown during
891
+ // a single commit without interruption.
892
+ if (commitPhaseBoundaries && (
893
+ commitPhaseBoundaries.has(node) ||
894
+ (node.alternate) && commitPhaseBoundaries.has(node.alternate)
895
+ )) {
896
+ // If so, we should ignore this error.
897
+ return null;
898
+ }
899
+
900
+ // The error should propagate to the next boundary -— we keep looking.
901
+ boundary = null;
902
+ willRetry = false;
903
+ }
904
+
905
node = node.return;
906
}
907
}
src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
+42
@@ -2178,6 +2178,48 @@ describe('ReactErrorBoundaries', () => {
2178
'NoopErrorBoundary componentWillUnmount',
2179
]);
2180
});
2181
+
2182
+ it('passes first error when two errors happen in commit', () => {
2183
+ const errors = [];
2184
+ let caughtError;
2185
+ class Parent extends React.Component {
2186
+ render() {
2187
+ return <Child />;
2188
+ }
2189
+ componentDidMount() {
2190
+ errors.push('parent sad');
2191
+ throw new Error('parent sad');
2192
+ }
2193
+ }
2194
+ class Child extends React.Component {
2195
+ render() {
2196
+ return <div />;
2197
+ }
2198
+ componentDidMount() {
2199
+ errors.push('child sad');
2200
+ throw new Error('child sad');
2201
+ }
2202
+ }
2203
+
2204
+ var container = document.createElement('div');
2205
+ try {
2206
+ // Here, we test the behavior where there is no error boundary and we
2207
+ // delegate to the host root.
2208
+ ReactDOM.render(<Parent />, container);
2209
+ } catch (e) {
2210
+ if (e.message !== 'parent sad' && e.message !== 'child sad') {
2211
+ throw e;
2212
+ }
2213
+ caughtError = e;
2214
+ }
2215
+
2216
+ expect(errors).toEqual([
2217
+ 'child sad',
2218
+ 'parent sad',
2219
+ ]);
2220
+ // Error should be the first thrown
2221
+ expect(caughtError.message).toBe('child sad');
2222
+ });
2223
}
2224
2225
});