[Fizz] Continue reporting aborted task errors after a fatal abort

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

@@ -3562,6 +3562,47 @@ describe('ReactDOMFizzServer', () => {
     );
   });
 
+  it('reports abort errors for every suspended task when aborting fatals the shell', async () => {
+    const promise = new Promise(() => {});
+    const rendered = [];
+    function Suspend({label}) {
+      rendered.push(label);
+      use(promise);
+      return null;
+    }
+
+    function App() {
+      return (
+        <>
+          <Suspense fallback="Loading...">
+            <Suspend label="boundary" />
+          </Suspense>
+          <Suspend label="root one" />
+          <Suspend label="root two" />
+        </>
+      );
+    }
+
+    const errors = [];
+    let abort;
+    await act(() => {
+      abort = renderToPipeableStream(<App />, {
+        onError(error) {
+          errors.push(error.message);
+        },
+        onShellError() {},
+      }).abort;
+    });
+
+    expect(rendered).toEqual(['boundary', 'root one', 'root two']);
+
+    await act(() => {
+      abort(new Error('abort reason'));
+    });
+
+    expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']);
+  });
+
   it('warns in dev if you access digest from errorInfo in onRecoverableError', async () => {
     await act(() => {
       const {pipe} = renderToPipeableStream(
@@ -3803,8 +3844,8 @@ describe('ReactDOMFizzServer', () => {
 
     expect(headers).toEqual({
       Link: `
-<non-responsive-preload>; rel=preload; as="image"; fetchpriority="high", 
-<non-responsive-img>; rel=preload; as="image"; fetchpriority="high"
+<non-responsive-preload>; rel=preload; as="image"; fetchpriority="high",
+ <non-responsive-img>; rel=preload; as="image"; fetchpriority="high"
 `
         .replaceAll('\n', '')
         .trim(),
@@ -4003,10 +4044,46 @@ describe('ReactDOMFizzServer', () => {
     await act(() => pipe(testWritable));
     expect(didRender).toBe(false);
     expect(didFatal).toBe(didFatal);
-    expect(errors).toEqual([
-      'boom',
-      'The destination stream errored while writing data.',
-    ]);
+    expect(errors).toEqual(['boom']);
+  });
+
+  it('does not report aborts after fatally erroring', async () => {
+    const promise = new Promise(() => {});
+    function AsyncComp() {
+      React.use(promise);
+      return 'Async';
+    }
+
+    function ErrorComp() {
+      throw new Error('boom');
+    }
+
+    const errors = [];
+    let abort;
+    await act(() => {
+      abort = renderToPipeableStream(
+        <div>
+          <Suspense fallback="loading...">
+            <AsyncComp />
+          </Suspense>
+          <ErrorComp />
+        </div>,
+        {
+          onError(error) {
+            errors.push(error.message);
+          },
+          onShellError() {},
+        },
+      ).abort;
+    });
+
+    expect(errors).toEqual(['boom']);
+
+    await act(() => {
+      abort(new Error('too late'));
+    });
+
+    expect(errors).toEqual(['boom']);
   });
 
   describe('error escaping', () => {
@@ -6986,6 +7063,83 @@ describe('ReactDOMFizzServer', () => {
     );
   });
 
+  it('currently does not report an in-flight root task after another root task fatals while aborting', async () => {
+    const promise = new Promise(() => {});
+    function SuspendedRoot() {
+      use(promise);
+      return null;
+    }
+
+    function Child() {
+      return 'child';
+    }
+
+    const abortRef = {current: null};
+    function ComponentThatAborts() {
+      abortRef.current(new Error('abort reason'));
+      return <Child />;
+    }
+
+    const errors = [];
+    await act(() => {
+      const {abort} = renderToPipeableStream(
+        <>
+          <SuspendedRoot />
+          <ComponentThatAborts />
+        </>,
+        {
+          onError(error) {
+            errors.push(error.message);
+          },
+          onShellError() {},
+        },
+      );
+      abortRef.current = abort;
+    });
+
+    expect(errors).toEqual(['abort reason']);
+  });
+
+  it('currently does not report a root task that suspends after aborting during render', async () => {
+    const promise = new Promise(() => {});
+    function SuspendedRoot() {
+      use(promise);
+      return null;
+    }
+
+    function Child() {
+      use(promise);
+      return null;
+    }
+
+    const abortRef = {current: null};
+    function ComponentThatAborts() {
+      abortRef.current(new Error('abort reason'));
+      return <Child />;
+    }
+
+    const errors = [];
+    await act(() => {
+      const {abort} = renderToPipeableStream(
+        <>
+          <SuspendedRoot />
+          <ComponentThatAborts />
+        </>,
+        {
+          onError(error) {
+            errors.push(error.message);
+          },
+          onShellError() {},
+        },
+      );
+      abortRef.current = abort;
+    });
+
+    // TODO: Once abort completion is async, this still-suspended task should
+    // observe ABORTING and report the abort reason as well.
+    expect(errors).toEqual(['abort reason']);
+  });
+
   it('can abort during render in a lazy initializer for a component', async () => {
     function Sibling() {
       return <p>sibling</p>;

packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js

@@ -237,6 +237,37 @@ describe('ReactDOMFizzServerNode', () => {
     expect(reportedShellErrors).toEqual([theError]);
   });
 
+  it('should not report aborts after the shell has fatally errored', async () => {
+    const reportedErrors = [];
+    const reportedShellErrors = [];
+    const {abort} = ReactDOMFizzServer.renderToPipeableStream(
+      <div>
+        <Suspense fallback="Loading">
+          <InfiniteSuspend />
+        </Suspense>
+        <Throw />
+      </div>,
+      {
+        onError(x) {
+          reportedErrors.push(x);
+        },
+        onShellError(x) {
+          reportedShellErrors.push(x);
+        },
+      },
+    );
+
+    await jest.runAllTimers();
+
+    expect(reportedErrors).toEqual([theError]);
+    expect(reportedShellErrors).toEqual([theError]);
+
+    abort(new Error('too late'));
+
+    expect(reportedErrors).toEqual([theError]);
+    expect(reportedShellErrors).toEqual([theError]);
+  });
+
   it('should error the stream when an error is thrown inside a fallback', async () => {
     const reportedErrors = [];
     const reportedShellErrors = [];
@@ -263,10 +294,7 @@ describe('ReactDOMFizzServerNode', () => {
 
     expect(output.error).toBe(theError);
     expect(output.result).toBe('');
-    expect(reportedErrors).toEqual([
-      theError.message,
-      'The destination stream errored while writing data.',
-    ]);
+    expect(reportedErrors).toEqual([theError.message]);
     expect(reportedShellErrors).toEqual([theError]);
   });
 
@@ -406,6 +434,45 @@ describe('ReactDOMFizzServerNode', () => {
     expect(isCompleteCalls).toBe(0);
   });
 
+  it('should report abort errors for every suspended task but fail the shell only once', async () => {
+    const promise = new Promise(() => {});
+    const rendered = [];
+    function Suspend({label}) {
+      rendered.push(label);
+      React.use(promise);
+      return null;
+    }
+
+    const errors = [];
+    const shellErrors = [];
+    const {abort} = ReactDOMFizzServer.renderToPipeableStream(
+      <>
+        <Suspense fallback="Loading...">
+          <Suspend label="boundary" />
+        </Suspense>
+        <Suspend label="root one" />
+        <Suspend label="root two" />
+      </>,
+      {
+        onError(error) {
+          errors.push(error.message);
+        },
+        onShellError(error) {
+          shellErrors.push(error);
+        },
+      },
+    );
+
+    await jest.runAllTimers();
+    expect(rendered).toEqual(['boundary', 'root one', 'root two']);
+
+    const reason = new Error('abort reason');
+    abort(reason);
+
+    expect(shellErrors).toEqual([reason]);
+    expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']);
+  });
+
   it('should be able to complete by abort when the fallback is also suspended', async () => {
     let isCompleteCalls = 0;
     const errors = [];

packages/react-server/src/ReactFizzServer.js

@@ -4643,51 +4643,52 @@ function abortTask(task: Task, request: Request, error: mixed): void {
   }
 
   if (boundary === null) {
-    if (request.status !== CLOSING && request.status !== CLOSED) {
-      const replay: null | ReplaySet = task.replay;
-      if (replay === null) {
-        // We didn't complete the root so we have nothing to show. We can close
-        // the request;
-        if (request.trackedPostpones !== null && segment !== null) {
-          const trackedPostpones = request.trackedPostpones;
-          // We are aborting a prerender and must treat the shell as halted
-          // We log the error but we still resolve the prerender
-          logRecoverableError(request, error, errorInfo, task.debugTask);
-          trackPostpone(request, trackedPostpones, task, segment);
-          finishedTask(request, null, task.row, segment);
-        } else {
-          logRecoverableError(request, error, errorInfo, task.debugTask);
-          fatalError(request, error, errorInfo, task.debugTask);
-        }
-        return;
+    const replay: null | ReplaySet = task.replay;
+    if (replay === null) {
+      // We didn't complete the root so we have nothing to show. We can close
+      // the request;
+      if (request.trackedPostpones !== null && segment !== null) {
+        const trackedPostpones = request.trackedPostpones;
+        // We are aborting a prerender and must treat the shell as halted
+        // We log the error but we still resolve the prerender
+        logRecoverableError(request, error, errorInfo, task.debugTask);
+        trackPostpone(request, trackedPostpones, task, segment);
+        finishedTask(request, null, task.row, segment);
       } else {
-        // If the shell aborts during a replay, that's not a fatal error. Instead
-        // we should be able to recover by client rendering all the root boundaries in
-        // the ReplaySet.
-        replay.pendingTasks--;
-        if (replay.pendingTasks === 0 && replay.nodes.length > 0) {
-          const errorDigest = logRecoverableError(
-            request,
-            error,
-            errorInfo,
-            null,
-          );
-          abortRemainingReplayNodes(
-            request,
-            null,
-            replay.nodes,
-            replay.slots,
-            error,
-            errorDigest,
-            errorInfo,
-            true,
-          );
-        }
-        request.pendingRootTasks--;
-        if (request.pendingRootTasks === 0) {
-          completeShell(request);
+        logRecoverableError(request, error, errorInfo, task.debugTask);
+        if (request.status !== CLOSING && request.status !== CLOSED) {
+          fatalError(request, error, errorInfo, task.debugTask);
         }
       }
+      return;
+    }
+    if (request.status !== CLOSING && request.status !== CLOSED) {
+      // If the shell aborts during a replay, that's not a fatal error. Instead
+      // we should be able to recover by client rendering all the root boundaries in
+      // the ReplaySet.
+      replay.pendingTasks--;
+      if (replay.pendingTasks === 0 && replay.nodes.length > 0) {
+        const errorDigest = logRecoverableError(
+          request,
+          error,
+          errorInfo,
+          null,
+        );
+        abortRemainingReplayNodes(
+          request,
+          null,
+          replay.nodes,
+          replay.slots,
+          error,
+          errorDigest,
+          errorInfo,
+          true,
+        );
+      }
+      request.pendingRootTasks--;
+      if (request.pendingRootTasks === 0) {
+        completeShell(request);
+      }
     }
   } else {
     // We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which
@@ -6143,9 +6144,12 @@ export function stopFlowing(request: Request): void {
 
 // This is called to early terminate a request. It puts all pending boundaries in client rendered state.
 export function abort(request: Request, reason: mixed): void {
-  if (request.status === OPEN || request.status === OPENING) {
-    request.status = ABORTING;
+  if (request.status !== OPEN && request.status !== OPENING) {
+    // Only requests that are not already complete or in the process of aborting
+    // can be aborted. in practice this makes abort callable at most once per render.
+    return;
   }
+  request.status = ABORTING;
 
   try {
     const abortableTasks = request.abortableTasks;