From 239858a8590c09d82d4662aafcd1de515c2886de Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 29 May 2026 15:57:54 -0700 Subject: [PATCH] [Fizz] Do not allow abort reentrancy Aborting is a gate you can only pass through once. A request that is already aborting, already completed, or already fatalled cannot be aborted a second time. Previously this was generally functionally true but you could contrive sequences where an onError would fire after a render fataled. This change makes it more explicit that this is not semantically correct by bailing out of abort if the request is in a status that cannot be aborted. --- .../src/__tests__/ReactDOMFizzServer-test.js | 44 +++++++++++++++++-- .../__tests__/ReactDOMFizzServerNode-test.js | 36 +++++++++++++-- packages/react-server/src/ReactFizzServer.js | 7 ++- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index efb7e887cf18..3ef1db6e7453 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4003,10 +4003,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( +
+ + + + +
, + { + 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', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 98030d43386c..c3554c757df2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/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( +
+ + + + +
, + { + 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]); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 73b3e435d189..3320d545b153 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -6143,9 +6143,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;