Handles risky callbacks on setState. Fixes #8238 (#8242)
* Handles risky callbacks on setState. Fixes #8238 * Updates try-catch to cover `callback` when context is not present. * Updates code to trapErrors instead of swallowing them. * Fixes flow errors * Incorporates review comments * Traps only the first error. Removes `callbackWasCalled` and updates fiber tests.
Ankeet Maini committed
Nov 13, 2016 at 02:21 UTC
077822e9d0f8a26bd505ee38e6353449ac8a845b
4 files changed
+68
-8
scripts/fiber/tests-passing.txt
+1
@@ -806,6 +806,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
806
* skips will/DidUpdate when bailing unless an update was already in progress
807
* performs batched updates at the end of the batch
808
* can nest batchedUpdates
809
+* can handle if setState callback throws
810
811
src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
812
* catches render error in a boundary during mounting
src/renderers/shared/fiber/ReactFiberCommitWork.js
+11
-2
@@ -321,7 +321,11 @@ module.exports = function<T, P, I, TI, C>(
321
}
322
if (finishedWork.effectTag & Callback) {
323
if (finishedWork.callbackList) {
324
- callCallbacks(finishedWork.callbackList, instance);
324
+ const callbackError = callCallbacks(finishedWork.callbackList, instance);
325
+ // since we only want to keep the first error
326
+ if (!error) {
327
+ error = callbackError;
328
+ }
329
finishedWork.callbackList = null;
330
}
331
}
@@ -332,10 +336,15 @@ module.exports = function<T, P, I, TI, C>(
336
}
337
case HostContainer: {
338
const rootFiber = finishedWork.stateNode;
339
+ let error = null;
340
if (rootFiber.callbackList) {
341
const { callbackList } = rootFiber;
342
rootFiber.callbackList = null;
338
- callCallbacks(callbackList, rootFiber.current.child.stateNode);
343
+ error = callCallbacks(callbackList, rootFiber.current.child.stateNode);
344
+ }
345
+
346
+ if (error) {
347
+ trapError(rootFiber, error, false);
348
}
349
return;
350
}
src/renderers/shared/fiber/ReactFiberUpdateQueue.js
+12
-6
@@ -69,20 +69,26 @@ exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) :
69
return queue;
70
};
71
72
-exports.callCallbacks = function(queue : UpdateQueue, context : any) {
72
+exports.callCallbacks = function(queue : UpdateQueue, context : any) : Error | null {
73
let node : ?UpdateQueueNode = queue;
74
+ let error = null;
75
while (node) {
76
const callback = node.callback;
77
if (callback && !node.callbackWasCalled) {
77
- node.callbackWasCalled = true;
78
- if (typeof context !== 'undefined') {
79
- callback.call(context);
80
- } else {
81
- callback();
78
+ try {
79
+ node.callbackWasCalled = true;
80
+ if (typeof context !== 'undefined') {
81
+ callback.call(context);
82
+ } else {
83
+ callback();
84
+ }
85
+ } catch (e) {
86
+ error = e;
87
}
88
}
89
node = node.next;
90
}
91
+ return error;
92
};
93
94
exports.mergeUpdateQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any {
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+44
@@ -1465,4 +1465,48 @@ describe('ReactIncremental', () => {
1465
]);
1466
expect(instance.state.n).toEqual(4);
1467
});
1468
+
1469
+ it('can handle if setState callback throws', () => {
1470
+ var ops = [];
1471
+ var instance;
1472
+
1473
+ class Foo extends React.Component {
1474
+ state = { n: 0 };
1475
+ render() {
1476
+ instance = this;
1477
+ return <div />;
1478
+ }
1479
+ }
1480
+
1481
+ ReactNoop.render(<Foo />);
1482
+ ReactNoop.flush();
1483
+ ops = [];
1484
+
1485
+ expect(instance.state.n).toEqual(0);
1486
+
1487
+ // first good callback
1488
+ instance.setState({ n: 1 }, () => ops.push('first good callback'));
1489
+ ReactNoop.flush();
1490
+
1491
+ // callback throws
1492
+ instance.setState({ n: 2 }, () => {
1493
+ throw new Error('Bail');
1494
+ });
1495
+ expect(() => {
1496
+ ReactNoop.flush();
1497
+ }).toThrow('Bail');
1498
+
1499
+ // should set state to 2 even if callback throws up
1500
+ expect(instance.state.n).toEqual(2);
1501
+
1502
+ // another good callback
1503
+ instance.setState({ n: 3 }, () => ops.push('second good callback'));
1504
+ ReactNoop.flush();
1505
+
1506
+ expect(ops).toEqual([
1507
+ 'first good callback',
1508
+ 'second good callback',
1509
+ ]);
1510
+ expect(instance.state.n).toEqual(3);
1511
+ });
1512
});