flushSync: Exhaust queue even if something throws (#26366)
If something throws as a result of `flushSync`, and there's remaining work left in the queue, React should keep working until all the work is complete. If multiple errors are thrown, React will combine them into an AggregateError object and throw that. In environments where AggregateError is not available, React will rethrow in an async task. (All the evergreen runtimes support AggregateError.) The scenario where this happens is relatively rare, because `flushSync` will only throw if there's no error boundary to capture the error.
Andrew Clark committed
Mar 10, 2023 at 17:21 UTC
93c10dfa6b0848c12189b773b59c77d74cad2a1a
12 files changed
+211
-41
packages/react-reconciler/src/ReactFiberSyncTaskQueue.js
+52
-22
@@ -52,37 +52,67 @@ export function flushSyncCallbacks(): null {
52
if (!isFlushingSyncQueue && syncQueue !== null) {
53
// Prevent re-entrance.
54
isFlushingSyncQueue = true;
55
- let i = 0;
55
+
56
+ // Set the event priority to discrete
57
+ // TODO: Is this necessary anymore? The only user code that runs in this
58
+ // queue is in the render or commit phases, which already set the
59
+ // event priority. Should be able to remove.
60
const previousUpdatePriority = getCurrentUpdatePriority();
57
- try {
58
- const isSync = true;
59
- const queue = syncQueue;
60
- // TODO: Is this necessary anymore? The only user code that runs in this
61
- // queue is in the render or commit phases.
62
- setCurrentUpdatePriority(DiscreteEventPriority);
61
+ setCurrentUpdatePriority(DiscreteEventPriority);
62
+
63
+ let errors: Array<mixed> | null = null;
64
+
65
+ const queue = syncQueue;
66
+ // $FlowFixMe[incompatible-use] found when upgrading Flow
67
+ for (let i = 0; i < queue.length; i++) {
68
// $FlowFixMe[incompatible-use] found when upgrading Flow
64
- for (; i < queue.length; i++) {
65
- // $FlowFixMe[incompatible-use] found when upgrading Flow
66
- let callback: SchedulerCallback = queue[i];
69
+ let callback: SchedulerCallback = queue[i];
70
+ try {
71
do {
72
+ const isSync = true;
73
// $FlowFixMe[incompatible-type] we bail out when we get a null
74
callback = callback(isSync);
75
} while (callback !== null);
76
+ } catch (error) {
77
+ // Collect errors so we can rethrow them at the end
78
+ if (errors === null) {
79
+ errors = [error];
80
+ } else {
81
+ errors.push(error);
82
+ }
83
}
72
- syncQueue = null;
73
- includesLegacySyncCallbacks = false;
74
- } catch (error) {
75
- // If something throws, leave the remaining callbacks on the queue.
76
- if (syncQueue !== null) {
77
- syncQueue = syncQueue.slice(i + 1);
84
+ }
85
+
86
+ syncQueue = null;
87
+ includesLegacySyncCallbacks = false;
88
+ setCurrentUpdatePriority(previousUpdatePriority);
89
+ isFlushingSyncQueue = false;
90
+
91
+ if (errors !== null) {
92
+ if (errors.length > 1) {
93
+ if (typeof AggregateError === 'function') {
94
+ // eslint-disable-next-line no-undef
95
+ throw new AggregateError(errors);
96
+ } else {
97
+ for (let i = 1; i < errors.length; i++) {
98
+ scheduleCallback(
99
+ ImmediatePriority,
100
+ throwError.bind(null, errors[i]),
101
+ );
102
+ }
103
+ const firstError = errors[0];
104
+ throw firstError;
105
+ }
106
+ } else {
107
+ const error = errors[0];
108
+ throw error;
109
}
79
- // Resume flushing in the next tick
80
- scheduleCallback(ImmediatePriority, flushSyncCallbacks);
81
- throw error;
82
- } finally {
83
- setCurrentUpdatePriority(previousUpdatePriority);
84
- isFlushingSyncQueue = false;
110
}
111
}
112
+
113
return null;
114
}
115
+
116
+function throwError(error: mixed) {
117
+ throw error;
118
+}
packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
+46
@@ -248,4 +248,50 @@ describe('ReactFlushSync', () => {
248
// Now the effects have fired.
249
assertLog(['Effect']);
250
});
251
+
252
+ test('completely exhausts synchronous work queue even if something throws', async () => {
253
+ function Throws({error}) {
254
+ throw error;
255
+ }
256
+
257
+ const root1 = ReactNoop.createRoot();
258
+ const root2 = ReactNoop.createRoot();
259
+ const root3 = ReactNoop.createRoot();
260
+
261
+ await act(async () => {
262
+ root1.render(<Text text="Hi" />);
263
+ root2.render(<Text text="Andrew" />);
264
+ root3.render(<Text text="!" />);
265
+ });
266
+ assertLog(['Hi', 'Andrew', '!']);
267
+
268
+ const aahh = new Error('AAHH!');
269
+ const nooo = new Error('Noooooooooo!');
270
+
271
+ let error;
272
+ try {
273
+ ReactNoop.flushSync(() => {
274
+ root1.render(<Throws error={aahh} />);
275
+ root2.render(<Throws error={nooo} />);
276
+ root3.render(<Text text="aww" />);
277
+ });
278
+ } catch (e) {
279
+ error = e;
280
+ }
281
+
282
+ // The update to root 3 should have finished synchronously, even though the
283
+ // earlier updates errored.
284
+ assertLog(['aww']);
285
+ // Roots 1 and 2 were unmounted.
286
+ expect(root1).toMatchRenderedOutput(null);
287
+ expect(root2).toMatchRenderedOutput(null);
288
+ expect(root3).toMatchRenderedOutput('aww');
289
+
290
+ // Because there were multiple errors, React threw an AggregateError.
291
+ // eslint-disable-next-line no-undef
292
+ expect(error).toBeInstanceOf(AggregateError);
293
+ expect(error.errors.length).toBe(2);
294
+ expect(error.errors[0]).toBe(aahh);
295
+ expect(error.errors[1]).toBe(nooo);
296
+ });
297
});
packages/react-reconciler/src/__tests__/ReactFlushSyncNoAggregateError-test.js
new
+79
@@ -0,0 +1,79 @@
1
+let React;
2
+let ReactNoop;
3
+let Scheduler;
4
+let act;
5
+let assertLog;
6
+let waitForThrow;
7
+
8
+// TODO: Migrate tests to React DOM instead of React Noop
9
+
10
+describe('ReactFlushSync (AggregateError not available)', () => {
11
+ beforeEach(() => {
12
+ jest.resetModules();
13
+
14
+ global.AggregateError = undefined;
15
+
16
+ React = require('react');
17
+ ReactNoop = require('react-noop-renderer');
18
+ Scheduler = require('scheduler');
19
+ act = require('internal-test-utils').act;
20
+
21
+ const InternalTestUtils = require('internal-test-utils');
22
+ assertLog = InternalTestUtils.assertLog;
23
+ waitForThrow = InternalTestUtils.waitForThrow;
24
+ });
25
+
26
+ function Text({text}) {
27
+ Scheduler.log(text);
28
+ return text;
29
+ }
30
+
31
+ test('completely exhausts synchronous work queue even if something throws', async () => {
32
+ function Throws({error}) {
33
+ throw error;
34
+ }
35
+
36
+ const root1 = ReactNoop.createRoot();
37
+ const root2 = ReactNoop.createRoot();
38
+ const root3 = ReactNoop.createRoot();
39
+
40
+ await act(async () => {
41
+ root1.render(<Text text="Hi" />);
42
+ root2.render(<Text text="Andrew" />);
43
+ root3.render(<Text text="!" />);
44
+ });
45
+ assertLog(['Hi', 'Andrew', '!']);
46
+
47
+ const aahh = new Error('AAHH!');
48
+ const nooo = new Error('Noooooooooo!');
49
+
50
+ let error;
51
+ try {
52
+ ReactNoop.flushSync(() => {
53
+ root1.render(<Throws error={aahh} />);
54
+ root2.render(<Throws error={nooo} />);
55
+ root3.render(<Text text="aww" />);
56
+ });
57
+ } catch (e) {
58
+ error = e;
59
+ }
60
+
61
+ // The update to root 3 should have finished synchronously, even though the
62
+ // earlier updates errored.
63
+ assertLog(['aww']);
64
+ // Roots 1 and 2 were unmounted.
65
+ expect(root1).toMatchRenderedOutput(null);
66
+ expect(root2).toMatchRenderedOutput(null);
67
+ expect(root3).toMatchRenderedOutput('aww');
68
+
69
+ // In modern environments, React would throw an AggregateError. Because
70
+ // AggregateError is not available, React throws the first error, then
71
+ // throws the remaining errors in separate tasks.
72
+ expect(error).toBe(aahh);
73
+ // TODO: Currently the remaining error is rethrown in an Immediate Scheduler
74
+ // task, but this may change to a timer or microtask in the future. The
75
+ // exact mechanism is an implementation detail; they just need to be logged
76
+ // in the order the occurred.
77
+ await waitForThrow(nooo);
78
+ });
79
+});
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+14
-11
@@ -1367,17 +1367,14 @@ describe('ReactIncrementalErrorHandling', () => {
1367
);
1368
await waitForAll([]);
1369
1370
- expect(() => {
1371
- expect(() => {
1372
- ReactNoop.flushSync(() => {
1373
- inst.setState({fail: true});
1374
- });
1375
- }).toThrow('Hello.');
1376
-
1377
- // The unmount is queued in a microtask. In order to capture the error
1378
- // that occurs during unmount, we can flush it early with `flushSync`.
1379
- ReactNoop.flushSync();
1380
- }).toThrow('One does not simply unmount me.');
1370
+ let aggregateError;
1371
+ try {
1372
+ ReactNoop.flushSync(() => {
1373
+ inst.setState({fail: true});
1374
+ });
1375
+ } catch (e) {
1376
+ aggregateError = e;
1377
+ }
1378
1379
assertLog([
1380
// Attempt to clean up.
@@ -1387,6 +1384,12 @@ describe('ReactIncrementalErrorHandling', () => {
1384
'BrokenRenderAndUnmount componentWillUnmount',
1385
]);
1386
expect(ReactNoop).toMatchRenderedOutput(null);
1387
+
1388
+ // React threw both errors as a single AggregateError
1389
+ const errors = aggregateError.errors;
1390
+ expect(errors.length).toBe(2);
1391
+ expect(errors[0].message).toBe('Hello.');
1392
+ expect(errors[1].message).toBe('One does not simply unmount me.');
1393
});
1394
1395
it('does not interrupt unmounting if detaching a ref throws', async () => {
packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js
+13
-8
@@ -31,18 +31,23 @@ describe('ReactTestRenderer', () => {
31
it('should warn if used to render a ReactDOM portal', () => {
32
const container = document.createElement('div');
33
expect(() => {
34
+ let error;
35
try {
36
ReactTestRenderer.create(ReactDOM.createPortal('foo', container));
37
} catch (e) {
37
- // TODO: After the update throws, a subsequent render is scheduled to
38
- // unmount the whole tree. This update also causes an error, and this
39
- // happens in a separate task. Flush this error now and capture it, to
40
- // prevent it from firing asynchronously and causing the Jest test
41
- // to fail.
42
- expect(() => Scheduler.unstable_flushAll()).toThrow(
43
- '.children.indexOf is not a function',
44
- );
38
+ error = e;
39
}
40
+ // After the update throws, a subsequent render is scheduled to
41
+ // unmount the whole tree. This update also causes an error, so React
42
+ // throws an AggregateError.
43
+ const errors = error.errors;
44
+ expect(errors.length).toBe(2);
45
+ expect(errors[0].message.includes('indexOf is not a function')).toBe(
46
+ true,
47
+ );
48
+ expect(errors[1].message.includes('indexOf is not a function')).toBe(
49
+ true,
50
+ );
51
}).toErrorDev('An invalid container has been provided.', {
52
withoutStack: true,
53
});
scripts/flow/environment.js
+1
@@ -22,6 +22,7 @@ declare var globalThis: Object;
22
23
declare var queueMicrotask: (fn: Function) => void;
24
declare var reportError: (error: mixed) => void;
25
+declare var AggregateError: Class<Error>;
26
27
declare module 'create-react-class' {
28
declare var exports: React$CreateClass;
scripts/rollup/validate/eslintrc.cjs.js
+1
@@ -33,6 +33,7 @@ module.exports = {
33
34
TaskController: 'readonly',
35
reportError: 'readonly',
36
+ AggregateError: 'readonly',
37
38
// Flight
39
Uint8Array: 'readonly',
scripts/rollup/validate/eslintrc.cjs2015.js
+1
@@ -33,6 +33,7 @@ module.exports = {
33
34
TaskController: 'readonly',
35
reportError: 'readonly',
36
+ AggregateError: 'readonly',
37
38
// Flight
39
Uint8Array: 'readonly',
scripts/rollup/validate/eslintrc.esm.js
+1
@@ -32,6 +32,7 @@ module.exports = {
32
33
TaskController: 'readonly',
34
reportError: 'readonly',
35
+ AggregateError: 'readonly',
36
37
// Flight
38
Uint8Array: 'readonly',
scripts/rollup/validate/eslintrc.fb.js
+1
@@ -33,6 +33,7 @@ module.exports = {
33
34
TaskController: 'readonly',
35
reportError: 'readonly',
36
+ AggregateError: 'readonly',
37
38
// Flight
39
Uint8Array: 'readonly',
scripts/rollup/validate/eslintrc.rn.js
+1
@@ -33,6 +33,7 @@ module.exports = {
33
34
TaskController: 'readonly',
35
reportError: 'readonly',
36
+ AggregateError: 'readonly',
37
38
// Temp
39
AsyncLocalStorage: 'readonly',
scripts/rollup/validate/eslintrc.umd.js
+1
@@ -37,6 +37,7 @@ module.exports = {
37
38
TaskController: 'readonly',
39
reportError: 'readonly',
40
+ AggregateError: 'readonly',
41
42
// Flight
43
Uint8Array: 'readonly',