flushSync: Exhaust queue even if something throws

packages/react-reconciler/src/ReactFiberSyncTaskQueue.js

@@ -52,37 +52,67 @@ export function flushSyncCallbacks(): null {
   if (!isFlushingSyncQueue && syncQueue !== null) {
     // Prevent re-entrance.
     isFlushingSyncQueue = true;
-    let i = 0;
+
+    // Set the event priority to discrete
+    // TODO: Is this necessary anymore? The only user code that runs in this
+    // queue is in the render or commit phases, which already set the
+    // event priority. Should be able to remove.
     const previousUpdatePriority = getCurrentUpdatePriority();
-    try {
-      const isSync = true;
-      const queue = syncQueue;
-      // TODO: Is this necessary anymore? The only user code that runs in this
-      // queue is in the render or commit phases.
-      setCurrentUpdatePriority(DiscreteEventPriority);
+    setCurrentUpdatePriority(DiscreteEventPriority);
+
+    let errors: Array<mixed> | null = null;
+
+    const queue = syncQueue;
+    // $FlowFixMe[incompatible-use] found when upgrading Flow
+    for (let i = 0; i < queue.length; i++) {
       // $FlowFixMe[incompatible-use] found when upgrading Flow
-      for (; i < queue.length; i++) {
-        // $FlowFixMe[incompatible-use] found when upgrading Flow
-        let callback: SchedulerCallback = queue[i];
+      let callback: SchedulerCallback = queue[i];
+      try {
         do {
+          const isSync = true;
           // $FlowFixMe[incompatible-type] we bail out when we get a null
           callback = callback(isSync);
         } while (callback !== null);
+      } catch (error) {
+        // Collect errors so we can rethrow them at the end
+        if (errors === null) {
+          errors = [error];
+        } else {
+          errors.push(error);
+        }
       }
-      syncQueue = null;
-      includesLegacySyncCallbacks = false;
-    } catch (error) {
-      // If something throws, leave the remaining callbacks on the queue.
-      if (syncQueue !== null) {
-        syncQueue = syncQueue.slice(i + 1);
+    }
+
+    syncQueue = null;
+    includesLegacySyncCallbacks = false;
+    setCurrentUpdatePriority(previousUpdatePriority);
+    isFlushingSyncQueue = false;
+
+    if (errors !== null) {
+      if (errors.length > 1) {
+        if (typeof AggregateError === 'function') {
+          // eslint-disable-next-line no-undef
+          throw new AggregateError(errors);
+        } else {
+          for (let i = 1; i < errors.length; i++) {
+            scheduleCallback(
+              ImmediatePriority,
+              throwError.bind(null, errors[i]),
+            );
+          }
+          const firstError = errors[0];
+          throw firstError;
+        }
+      } else {
+        const error = errors[0];
+        throw error;
       }
-      // Resume flushing in the next tick
-      scheduleCallback(ImmediatePriority, flushSyncCallbacks);
-      throw error;
-    } finally {
-      setCurrentUpdatePriority(previousUpdatePriority);
-      isFlushingSyncQueue = false;
     }
   }
+
   return null;
 }
+
+function throwError(error: mixed) {
+  throw error;
+}

packages/react-reconciler/src/__tests__/ReactFlushSync-test.js

@@ -248,4 +248,50 @@ describe('ReactFlushSync', () => {
     // Now the effects have fired.
     assertLog(['Effect']);
   });
+
+  test('completely exhausts synchronous work queue even if something throws', async () => {
+    function Throws({error}) {
+      throw error;
+    }
+
+    const root1 = ReactNoop.createRoot();
+    const root2 = ReactNoop.createRoot();
+    const root3 = ReactNoop.createRoot();
+
+    await act(async () => {
+      root1.render(<Text text="Hi" />);
+      root2.render(<Text text="Andrew" />);
+      root3.render(<Text text="!" />);
+    });
+    assertLog(['Hi', 'Andrew', '!']);
+
+    const aahh = new Error('AAHH!');
+    const nooo = new Error('Noooooooooo!');
+
+    let error;
+    try {
+      ReactNoop.flushSync(() => {
+        root1.render(<Throws error={aahh} />);
+        root2.render(<Throws error={nooo} />);
+        root3.render(<Text text="aww" />);
+      });
+    } catch (e) {
+      error = e;
+    }
+
+    // The update to root 3 should have finished synchronously, even though the
+    // earlier updates errored.
+    assertLog(['aww']);
+    // Roots 1 and 2 were unmounted.
+    expect(root1).toMatchRenderedOutput(null);
+    expect(root2).toMatchRenderedOutput(null);
+    expect(root3).toMatchRenderedOutput('aww');
+
+    // Because there were multiple errors, React threw an AggregateError.
+    // eslint-disable-next-line no-undef
+    expect(error).toBeInstanceOf(AggregateError);
+    expect(error.errors.length).toBe(2);
+    expect(error.errors[0]).toBe(aahh);
+    expect(error.errors[1]).toBe(nooo);
+  });
 });

packages/react-reconciler/src/__tests__/ReactFlushSyncNoAggregateError-test.js

@@ -0,0 +1,79 @@
+let React;
+let ReactNoop;
+let Scheduler;
+let act;
+let assertLog;
+let waitForThrow;
+
+// TODO: Migrate tests to React DOM instead of React Noop
+
+describe('ReactFlushSync (AggregateError not available)', () => {
+  beforeEach(() => {
+    jest.resetModules();
+
+    global.AggregateError = undefined;
+
+    React = require('react');
+    ReactNoop = require('react-noop-renderer');
+    Scheduler = require('scheduler');
+    act = require('internal-test-utils').act;
+
+    const InternalTestUtils = require('internal-test-utils');
+    assertLog = InternalTestUtils.assertLog;
+    waitForThrow = InternalTestUtils.waitForThrow;
+  });
+
+  function Text({text}) {
+    Scheduler.log(text);
+    return text;
+  }
+
+  test('completely exhausts synchronous work queue even if something throws', async () => {
+    function Throws({error}) {
+      throw error;
+    }
+
+    const root1 = ReactNoop.createRoot();
+    const root2 = ReactNoop.createRoot();
+    const root3 = ReactNoop.createRoot();
+
+    await act(async () => {
+      root1.render(<Text text="Hi" />);
+      root2.render(<Text text="Andrew" />);
+      root3.render(<Text text="!" />);
+    });
+    assertLog(['Hi', 'Andrew', '!']);
+
+    const aahh = new Error('AAHH!');
+    const nooo = new Error('Noooooooooo!');
+
+    let error;
+    try {
+      ReactNoop.flushSync(() => {
+        root1.render(<Throws error={aahh} />);
+        root2.render(<Throws error={nooo} />);
+        root3.render(<Text text="aww" />);
+      });
+    } catch (e) {
+      error = e;
+    }
+
+    // The update to root 3 should have finished synchronously, even though the
+    // earlier updates errored.
+    assertLog(['aww']);
+    // Roots 1 and 2 were unmounted.
+    expect(root1).toMatchRenderedOutput(null);
+    expect(root2).toMatchRenderedOutput(null);
+    expect(root3).toMatchRenderedOutput('aww');
+
+    // In modern environments, React would throw an AggregateError. Because
+    // AggregateError is not available, React throws the first error, then
+    // throws the remaining errors in separate tasks.
+    expect(error).toBe(aahh);
+    // TODO: Currently the remaining error is rethrown in an Immediate Scheduler
+    // task, but this may change to a timer or microtask in the future. The
+    // exact mechanism is an implementation detail; they just need to be logged
+    // in the order the occurred.
+    await waitForThrow(nooo);
+  });
+});

packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js

@@ -1367,17 +1367,14 @@ describe('ReactIncrementalErrorHandling', () => {
     );
     await waitForAll([]);
 
-    expect(() => {
-      expect(() => {
-        ReactNoop.flushSync(() => {
-          inst.setState({fail: true});
-        });
-      }).toThrow('Hello.');
-
-      // The unmount is queued in a microtask. In order to capture the error
-      // that occurs during unmount, we can flush it early with `flushSync`.
-      ReactNoop.flushSync();
-    }).toThrow('One does not simply unmount me.');
+    let aggregateError;
+    try {
+      ReactNoop.flushSync(() => {
+        inst.setState({fail: true});
+      });
+    } catch (e) {
+      aggregateError = e;
+    }
 
     assertLog([
       // Attempt to clean up.
@@ -1387,6 +1384,12 @@ describe('ReactIncrementalErrorHandling', () => {
       'BrokenRenderAndUnmount componentWillUnmount',
     ]);
     expect(ReactNoop).toMatchRenderedOutput(null);
+
+    // React threw both errors as a single AggregateError
+    const errors = aggregateError.errors;
+    expect(errors.length).toBe(2);
+    expect(errors[0].message).toBe('Hello.');
+    expect(errors[1].message).toBe('One does not simply unmount me.');
   });
 
   it('does not interrupt unmounting if detaching a ref throws', async () => {

packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js

@@ -31,18 +31,23 @@ describe('ReactTestRenderer', () => {
   it('should warn if used to render a ReactDOM portal', () => {
     const container = document.createElement('div');
     expect(() => {
+      let error;
       try {
         ReactTestRenderer.create(ReactDOM.createPortal('foo', container));
       } catch (e) {
-        // TODO: After the update throws, a subsequent render is scheduled to
-        // unmount the whole tree. This update also causes an error, and this
-        // happens in a separate task. Flush this error now and capture it, to
-        // prevent it from firing asynchronously and causing the Jest test
-        // to fail.
-        expect(() => Scheduler.unstable_flushAll()).toThrow(
-          '.children.indexOf is not a function',
-        );
+        error = e;
       }
+      // After the update throws, a subsequent render is scheduled to
+      // unmount the whole tree. This update also causes an error, so React
+      // throws an AggregateError.
+      const errors = error.errors;
+      expect(errors.length).toBe(2);
+      expect(errors[0].message.includes('indexOf is not a function')).toBe(
+        true,
+      );
+      expect(errors[1].message.includes('indexOf is not a function')).toBe(
+        true,
+      );
     }).toErrorDev('An invalid container has been provided.', {
       withoutStack: true,
     });

scripts/flow/environment.js

@@ -22,6 +22,7 @@ declare var globalThis: Object;
 
 declare var queueMicrotask: (fn: Function) => void;
 declare var reportError: (error: mixed) => void;
+declare var AggregateError: Class<Error>;
 
 declare module 'create-react-class' {
   declare var exports: React$CreateClass;

scripts/rollup/validate/eslintrc.cjs.js

@@ -33,6 +33,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Flight
     Uint8Array: 'readonly',

scripts/rollup/validate/eslintrc.cjs2015.js

@@ -33,6 +33,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Flight
     Uint8Array: 'readonly',

scripts/rollup/validate/eslintrc.esm.js

@@ -32,6 +32,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Flight
     Uint8Array: 'readonly',

scripts/rollup/validate/eslintrc.fb.js

@@ -33,6 +33,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Flight
     Uint8Array: 'readonly',

scripts/rollup/validate/eslintrc.rn.js

@@ -33,6 +33,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Temp
     AsyncLocalStorage: 'readonly',

scripts/rollup/validate/eslintrc.umd.js

@@ -37,6 +37,7 @@ module.exports = {
 
     TaskController: 'readonly',
     reportError: 'readonly',
+    AggregateError: 'readonly',
 
     // Flight
     Uint8Array: 'readonly',