Handle errors thrown when committing root
Previously this caused an infinite loop.
Andrew Clark committed
Dec 20, 2016 at 15:34 UTC
15acbaa8e147f6da64d11393c412d392274a0464
3 files changed
+91
-83
scripts/fiber/tests-passing.txt
+1
@@ -1184,6 +1184,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
1184
* unmounts components with uncaught errors
1185
* does not interrupt unmounting if detaching a ref throws
1186
* handles error thrown by host config while working on failed root
1187
+* handles error thrown by top-level callback
1188
1189
src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
1190
* handles isMounted even when the initial render is deferred
src/renderers/shared/fiber/ReactFiberScheduler.js
+83
-83
@@ -117,7 +117,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
117
let nextUnitOfWork : ?Fiber = null;
118
let nextPriorityLevel : PriorityLevel = NoWork;
119
120
- // The next fiber with an effect, during the commit phase.
120
+ // The next fiber with an effect that we're currently committing.
121
let nextEffect : ?Fiber = null;
122
123
let pendingCommit : ?Fiber = null;
@@ -210,7 +210,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
210
return null;
211
}
212
213
- function commitAllHostEffects(finishedWork : Fiber) {
213
+ function commitAllHostEffects() {
214
while (nextEffect) {
215
if (__DEV__) {
216
ReactDebugCurrentFiber.current = nextEffect;
@@ -267,16 +267,9 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
267
if (__DEV__) {
268
ReactDebugCurrentFiber.current = null;
269
}
270
-
271
- // If the root itself had an effect, we perform that since it is
272
- // not part of the effect list.
273
- if (finishedWork.effectTag !== NoEffect) {
274
- const current = finishedWork.alternate;
275
- commitWork(current, finishedWork);
276
- }
270
}
271
279
- function commitAllLifeCycles(finishedWork : Fiber) {
272
+ function commitAllLifeCycles() {
273
while (nextEffect) {
274
const current = nextEffect.alternate;
275
// Use Task priority for lifecycle updates
@@ -298,16 +291,6 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
291
// tags to reason about the current life-cycle.
292
nextEffect = next;
293
}
301
-
302
- // If the root itself had an effect, we perform that since it is
303
- // not part of the effect list.
304
- if (finishedWork.effectTag !== NoEffect) {
305
- const current = finishedWork.alternate;
306
- commitLifeCycles(current, finishedWork);
307
- if (finishedWork.effectTag & Err) {
308
- commitErrorHandling(finishedWork);
309
- }
310
- }
294
}
295
296
function commitAllWork(finishedWork : Fiber) {
@@ -331,25 +314,42 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
314
const previousPriorityContext = priorityContext;
315
priorityContext = TaskPriority;
316
317
+ let firstEffect;
318
+ if (finishedWork.effectTag !== NoEffect) {
319
+ // A fiber's effect list consists only of its children, not itself. So if
320
+ // the root has an effect, we need to add it to the end of the list. The
321
+ // resulting list is the set that would belong to the root's parent, if
322
+ // it had one; that is, all the effects in the tree including the root.
323
+ if (finishedWork.lastEffect) {
324
+ finishedWork.lastEffect.nextEffect = finishedWork;
325
+ firstEffect = finishedWork.firstEffect;
326
+ } else {
327
+ firstEffect = finishedWork;
328
+ }
329
+ } else {
330
+ // There is no effect on the root.
331
+ firstEffect = finishedWork.firstEffect;
332
+ }
333
+
334
prepareForCommit();
335
336
// Commit all the side-effects within a tree. We'll do this in two passes.
337
// The first pass performs all the host insertions, updates, deletions and
338
// ref unmounts.
339
- nextEffect = finishedWork.firstEffect;
340
- while (true) {
339
+ nextEffect = firstEffect;
340
+ while (nextEffect) {
341
try {
342
commitAllHostEffects(finishedWork);
343
} catch (error) {
344
+ if (!nextEffect) {
345
+ throw new Error('Should have nextEffect.');
346
+ }
347
captureError(nextEffect, error);
348
// Clean-up
346
- isUnmounting = false;
349
if (nextEffect) {
350
nextEffect = nextEffect.nextEffect;
349
- continue;
351
}
352
}
352
- break;
353
}
354
355
resetAfterCommit();
@@ -360,20 +360,19 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
360
// In the second pass we'll perform all life-cycles and ref callbacks.
361
// Life-cycles happen as a separate pass so that all placements, updates,
362
// and deletions in the entire tree have already been invoked.
363
- nextEffect = finishedWork.firstEffect;
364
- while (true) {
363
+ nextEffect = firstEffect;
364
+ while (nextEffect) {
365
try {
366
commitAllLifeCycles(finishedWork, nextEffect);
367
} catch (error) {
368
- captureError(nextEffect || null, error);
368
+ if (!nextEffect) {
369
+ throw new Error('Should have nextEffect.');
370
+ }
371
+ captureError(nextEffect, error);
372
if (nextEffect) {
370
- const next = nextEffect.nextEffect;
371
- nextEffect.nextEffect = null;
372
- nextEffect = next;
373
+ nextEffect = nextEffect.nextEffect;
374
}
374
- continue;
375
}
376
- break;
376
}
377
378
isCommitting = false;
@@ -694,6 +693,9 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
693
} catch (error) {
694
// We caught an error during either the begin or complete phases.
695
const failedWork = nextUnitOfWork;
696
+ if (!failedWork) {
697
+ throw new Error('Should have nextUnitOfWork.');
698
+ }
699
700
// Reset the priority context to its value before reconcilation.
701
priorityContext = priorityContextBeforeReconciliation;
@@ -785,7 +787,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
787
}
788
789
// Returns the boundary that captured the error, or null if the error is ignored
788
- function captureError(failedWork : ?Fiber, error : Error) : ?Fiber {
790
+ function captureError(failedWork : Fiber, error : Error) : ?Fiber {
791
// It is no longer valid because we exited the user code.
792
ReactCurrentOwner.current = null;
793
if (__DEV__) {
@@ -796,59 +798,57 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
798
799
// Search for the nearest error boundary.
800
let boundary : ?Fiber = null;
799
- if (failedWork) {
800
- // Host containers are a special case. If the failed work itself is a host
801
- // container, then it acts as its own boundary. In all other cases, we
802
- // ignore the work itself and only search through the parents.
803
- if (failedWork.tag === HostRoot) {
804
- boundary = failedWork;
805
-
806
- if (isFailedBoundary(failedWork)) {
807
- // If this root already failed, there must have been an error when
808
- // attempting to unmount it. This is a worst-case scenario and
809
- // should only be possible if there's a bug in the renderer.
810
- fatalError = error;
811
- }
812
- } else {
813
- let node = failedWork.return;
814
- while (node && !boundary) {
815
- if (node.tag === ClassComponent) {
816
- const instance = node.stateNode;
817
- if (typeof instance.unstable_handleError === 'function') {
818
- if (isFailedBoundary(node)) {
819
- // This boundary is already in a failed state. The error should
820
- // propagate to the next boundary — except in the
821
- // following cases:
822
-
823
- // If we're currently unmounting, that means this error was
824
- // thrown while unmounting a failed subtree. We should ignore
825
- // the error.
826
- if (isUnmounting) {
827
- return null;
828
- }
829
-
830
- // If we're in the commit phase, we should check to see if
831
- // this boundary already captured an error during this commit.
832
- // This case exists because multiple errors can be thrown during
833
- // a single commit without interruption.
834
- if (commitPhaseBoundaries && (
835
- commitPhaseBoundaries.has(node) ||
836
- (node.alternate) && commitPhaseBoundaries.has(node.alternate)
837
- )) {
838
- // If so, we should ignore this error.
839
- return null;
840
- }
841
- } else {
842
- // Found an error boundary!
843
- boundary = node;
801
+ // Host containers are a special case. If the failed work itself is a host
802
+ // container, then it acts as its own boundary. In all other cases, we
803
+ // ignore the work itself and only search through the parents.
804
+ if (failedWork.tag === HostRoot) {
805
+ boundary = failedWork;
806
+
807
+ if (isFailedBoundary(failedWork)) {
808
+ // If this root already failed, there must have been an error when
809
+ // attempting to unmount it. This is a worst-case scenario and
810
+ // should only be possible if there's a bug in the renderer.
811
+ fatalError = error;
812
+ }
813
+ } else {
814
+ let node = failedWork.return;
815
+ while (node && !boundary) {
816
+ if (node.tag === ClassComponent) {
817
+ const instance = node.stateNode;
818
+ if (typeof instance.unstable_handleError === 'function') {
819
+ if (isFailedBoundary(node)) {
820
+ // This boundary is already in a failed state. The error should
821
+ // propagate to the next boundary — except in the
822
+ // following cases:
823
+
824
+ // If we're currently unmounting, that means this error was
825
+ // thrown while unmounting a failed subtree. We should ignore
826
+ // the error.
827
+ if (isUnmounting) {
828
+ return null;
829
}
830
+
831
+ // If we're in the commit phase, we should check to see if
832
+ // this boundary already captured an error during this commit.
833
+ // This case exists because multiple errors can be thrown during
834
+ // a single commit without interruption.
835
+ if (commitPhaseBoundaries && (
836
+ commitPhaseBoundaries.has(node) ||
837
+ (node.alternate) && commitPhaseBoundaries.has(node.alternate)
838
+ )) {
839
+ // If so, we should ignore this error.
840
+ return null;
841
+ }
842
+ } else {
843
+ // Found an error boundary!
844
+ boundary = node;
845
}
846
- } else if (node.tag === HostRoot) {
847
- // Treat the root like a no-op error boundary.
848
- boundary = node;
846
}
850
- node = node.return;
847
+ } else if (node.tag === HostRoot) {
848
+ // Treat the root like a no-op error boundary.
849
+ boundary = node;
850
}
851
+ node = node.return;
852
}
853
}
854
src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
+7
@@ -924,4 +924,11 @@ describe('ReactIncrementalErrorHandling', () => {
924
expect(() => ReactNoop.flush()).toThrow('Error in host config.');
925
});
926
});
927
+
928
+ it('handles error thrown by top-level callback', () => {
929
+ ReactNoop.render(<div />, () => {
930
+ throw new Error('Error!');
931
+ });
932
+ expect(() => ReactNoop.flush()).toThrow('Error!');
933
+ });
934
});