Abort cacheSignal() when the render fatals or completes successfully

sebmarkbage · sebmarkbage · commit f7d041adcd72 · 2025-06-17T15:46:57.000-04:00
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -227,6 +227,42 @@ describe('ReactCache', () => {
     expect(cacheSignal()).toBe(null);
   });
 
+  it('cacheSignal() aborts when the render finishes normally', async () => {
+    let renderedCacheSignal = null;
+
+    let resolve;
+    const promise = new Promise(r => (resolve = r));
+
+    async function Test() {
+      renderedCacheSignal = cacheSignal();
+      await promise;
+      return 'Hi';
+    }
+
+    const controller = new AbortController();
+    const errors = [];
+    const result = ReactNoopFlightServer.render(<Test />, {
+      signal: controller.signal,
+      onError(x) {
+        errors.push(x);
+      },
+    });
+    expect(errors).toEqual([]);
+    expect(renderedCacheSignal).not.toBe(controller.signal); // In the future we might make these the same
+    expect(renderedCacheSignal.aborted).toBe(false);
+    await resolve();
+    await 0;
+    await 0;
+
+    expect(await ReactNoopFlightClient.read(result)).toBe('Hi');
+
+    expect(errors).toEqual([]);
+    expect(renderedCacheSignal.aborted).toBe(true);
+    expect(renderedCacheSignal.reason.message).toContain(
+      'This render completed successfully.',
+    );
+  });
+
   it('cacheSignal() aborts when the render is aborted', async () => {
     let renderedCacheSignal = null;
 
diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
@@ -3371,6 +3371,13 @@ function fatalError(request: Request, error: mixed): void {
     request.status = CLOSING;
     request.fatalError = error;
   }
+  const abortReason = new Error(
+    'The render was aborted due to a fatal error.',
+    {
+      cause: error,
+    },
+  );
+  request.cacheController.abort(abortReason);
 }
 
 function emitPostponeChunk(
@@ -4842,6 +4849,12 @@ function flushCompletedChunks(
     if (enableTaint) {
       cleanupTaintQueue(request);
     }
+    if (request.status < ABORTING) {
+      const abortReason = new Error(
+        'This render completed successfully. All cacheSignals are now aborted to allow clean up of any unused resources.',
+      );
+      request.cacheController.abort(abortReason);
+    }
     request.status = CLOSED;
     close(destination);
     request.destination = null;
@@ -4923,8 +4936,8 @@ export function abort(request: Request, reason: mixed): void {
     // We define any status below OPEN as OPEN equivalent
     if (request.status <= OPEN) {
       request.status = ABORTING;
+      request.cacheController.abort(reason);
     }
-    request.cacheController.abort(reason);
     const abortableTasks = request.abortableTasks;
     if (abortableTasks.size > 0) {
       if (enableHalt && request.type === PRERENDER) {
diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
@@ -546,5 +546,7 @@
   "558": "Client rendering an Activity suspended it again. This is a bug in React.",
   "559": "Expected to find a host node. This is a bug in React.",
   "560": "Cannot use a startGestureTransition() with a comment node root.",
-  "561": "This rendered a large document (>%s kB) without any Suspense boundaries around most of it. That can delay initial paint longer than necessary. To improve load performance, add a <Suspense> or <SuspenseList> around the content you expect to be below the header or below the fold. In the meantime, the content will deopt to paint arbitrary incomplete pieces of HTML."
+  "561": "This rendered a large document (>%s kB) without any Suspense boundaries around most of it. That can delay initial paint longer than necessary. To improve load performance, add a <Suspense> or <SuspenseList> around the content you expect to be below the header or below the fold. In the meantime, the content will deopt to paint arbitrary incomplete pieces of HTML.",
+  "562": "The render was aborted due to a fatal error.",
+  "563": "This render completed successfully. All cacheSignals are now aborted to allow clean up of any unused resources."
 }

Original file line number	Diff line number	Diff line change
`@@ -546,5 +546,7 @@`
`546`	`546`	`"558": "Client rendering an Activity suspended it again. This is a bug in React.",`
`547`	`547`	`"559": "Expected to find a host node. This is a bug in React.",`
`548`	`548`	`"560": "Cannot use a startGestureTransition() with a comment node root.",`
`549`		`- "561": "This rendered a large document (>%s kB) without any Suspense boundaries around most of it. That can delay initial paint longer than necessary. To improve load performance, add a <Suspense> or <SuspenseList> around the content you expect to be below the header or below the fold. In the meantime, the content will deopt to paint arbitrary incomplete pieces of HTML."`
	`549`	`+ "561": "This rendered a large document (>%s kB) without any Suspense boundaries around most of it. That can delay initial paint longer than necessary. To improve load performance, add a <Suspense> or <SuspenseList> around the content you expect to be below the header or below the fold. In the meantime, the content will deopt to paint arbitrary incomplete pieces of HTML.",`
	`550`	`+ "562": "The render was aborted due to a fatal error.",`
	`551`	`+ "563": "This render completed successfully. All cacheSignals are now aborted to allow clean up of any unused resources."`
`550`	`552`	`}`