From 6e6fdaf2335570d874ad018b0e55c4a530841b8c Mon Sep 17 00:00:00 2001 From: Josh Story Date: Sun, 31 May 2026 09:40:49 -0700 Subject: [PATCH 1/3] [Fizz] Track abort state on Request Previously Fizz represented an active abort using the `ABORTING` request status. This is ambiguous because aborting a task can synchronously fatal the request, transitioning it to `CLOSING` or `CLOSED` while another task is still unwinding from the same abort. Once that happened, the in-flight task no longer observed that the request was aborted and could fail to report its abort error. This change removes the `ABORTING` status and instead tracks whether the request was aborted independently on the Request. The existing `fatalError` field continues to store the abort reason. As a result, tasks that were rendering when an abort occurred continue to observe the abort even if another aborted task has already fataled the request, allowing all relevant unfinished task errors to be reported. DEV stalled replays temporarily mask the aborted state so they can continue to reconstruct suspended call sites as before. This also establishes explicit abort state on the Request for follow-up work that delays abort completion and allows rejected suspended work to provide more specific abort errors. --- .../src/__tests__/ReactDOMFizzServer-test.js | 21 +++----- packages/react-server/src/ReactFizzServer.js | 52 ++++++++++--------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index bc2b086b643..0b56fe34710 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -7063,7 +7063,7 @@ describe('ReactDOMFizzServer', () => { ); }); - it('currently does not report an in-flight root task after another root task fatals while aborting', async () => { + it('reports an in-flight root task after another root task fatals while aborting', async () => { const promise = new Promise(() => {}); function SuspendedRoot() { use(promise); @@ -7097,10 +7097,10 @@ describe('ReactDOMFizzServer', () => { abortRef.current = abort; }); - expect(errors).toEqual(['abort reason']); + expect(errors).toEqual(['abort reason', 'abort reason']); }); - it('currently does not report a root task that suspends after aborting during render', async () => { + it('reports a root task that suspends after aborting during render', async () => { const promise = new Promise(() => {}); function SuspendedRoot() { use(promise); @@ -7135,9 +7135,7 @@ describe('ReactDOMFizzServer', () => { 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']); + expect(errors).toEqual(['abort reason', 'abort reason']); }); it('can abort during render in a lazy initializer for a component', async () => { @@ -8444,10 +8442,6 @@ describe('ReactDOMFizzServer', () => { } expect(thrownError).toBe('boom'); - // TODO there should actually be three errors. One for the pending Suspense, one for the fallback task, and one for the task - // that does the abort itself. At the moment abort will flush queues and if there is no pending tasks will close the request before - // the task which initiated the abort can even be processed. This is a bug but not one that I am fixing with the current change - // so I am asserting the current behavior expect(errors).toEqual([ { error: 'boom', @@ -8467,9 +8461,10 @@ describe('ReactDOMFizzServer', () => { 'html', 'App', ]), - // }, { - // error: 'boom', - // componentStack: componentStack(['Abort', 'body', 'html', 'App']) + }, + { + error: 'boom', + componentStack: componentStack(['Abort', 'body', 'html', 'App']), }, ]); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 1270a5cf0b0..5d479f142ab 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -359,10 +359,9 @@ type Segment = { const OPENING = 10; const OPEN = 11; -const ABORTING = 12; -const CLOSING = 13; -const CLOSED = 14; -const STALLED_DEV = 15; +const CLOSING = 12; +const CLOSED = 13; +const STALLED_DEV = 14; export opaque type Request = { destination: null | Destination, @@ -371,8 +370,9 @@ export opaque type Request = { +renderState: RenderState, +rootFormatContext: FormatContext, +progressiveChunkSize: number, - status: 10 | 11 | 12 | 13 | 14 | 15, + status: 10 | 11 | 12 | 13 | 14, fatalError: mixed, + aborted: boolean, nextSegmentId: number, allPendingTasks: number, // when it reaches zero, we can close the connection. pendingRootTasks: number, // when this reaches zero, we've finished at least the root boundary. @@ -530,6 +530,7 @@ function RequestInstance( : progressiveChunkSize; this.status = isWorkLoopExternallyDriven ? OPEN : OPENING; this.fatalError = null; + this.aborted = false; this.nextSegmentId = 0; this.allPendingTasks = 0; this.pendingRootTasks = 0; @@ -1038,7 +1039,11 @@ function pushHaltedAwaitOnComponentStack( // performWork + retryTask without mutation function rerenderStalledTask(request: Request, task: Task): void { const prevStatus = request.status; + const prevAborted = request.aborted; request.status = STALLED_DEV; + // This diagnostic replay must reach the suspended call site instead of + // taking the abort path. + request.aborted = false; const prevContext = getActiveContext(); const prevDispatcher = ReactSharedInternals.H; @@ -1082,6 +1087,7 @@ function rerenderStalledTask(request: Request, task: Task): void { } currentRequest = prevRequest; request.status = prevStatus; + request.aborted = prevAborted; } } @@ -1479,7 +1485,7 @@ function renderSuspenseBoundary( boundarySegment.status = COMPLETED; finishedSegment(request, parentBoundary, boundarySegment); } catch (thrownValue: mixed) { - if (request.status === ABORTING) { + if (request.aborted) { boundarySegment.status = ABORTED; } else { boundarySegment.status = ERRORED; @@ -1589,7 +1595,7 @@ function renderSuspenseBoundary( } catch (thrownValue: mixed) { newBoundary.status = CLIENT_RENDERED; let error: mixed; - if (request.status === ABORTING) { + if (request.aborted) { contentRootSegment.status = ABORTED; error = request.fatalError; } else { @@ -2075,7 +2081,7 @@ function renderSuspenseListRows( finishSuspenseListRow(request, previousSuspenseListRow); } } catch (thrownValue: mixed) { - if (request.status === ABORTING) { + if (request.aborted) { newSegment.status = ABORTED; } else { newSegment.status = ERRORED; @@ -2392,7 +2398,7 @@ function finishClassComponent( } else { nextChildren = instance.render(); } - if (request.status === ABORTING) { + if (request.aborted) { // eslint-disable-next-line no-throw-literal throw null; } @@ -2541,7 +2547,7 @@ function renderFunctionComponent( props, legacyContext, ); - if (request.status === ABORTING) { + if (request.aborted) { // eslint-disable-next-line no-throw-literal throw null; } @@ -2818,12 +2824,7 @@ function renderLazyComponent( const init = lazyComponent._init; Component = init(payload); } - if ( - request.status === ABORTING && - // We're going to discard this render anyway. - // We just need to reach the point where we suspended in dev. - (!__DEV__ || request.status !== STALLED_DEV) - ) { + if (request.aborted) { // eslint-disable-next-line no-throw-literal throw null; } @@ -3441,7 +3442,7 @@ function retryNode(request: Request, task: Task): void { const init = lazyNode._init; resolvedNode = init(payload); } - if (request.status === ABORTING) { + if (request.aborted) { // eslint-disable-next-line no-throw-literal throw null; } @@ -4185,7 +4186,7 @@ function renderNode( getSuspendedThenable() : thrownValue; - if (request.status === ABORTING) { + if (request.aborted) { // We are aborting so we can just bubble up to the task by falling through } else if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] @@ -4286,7 +4287,7 @@ function renderNode( getSuspendedThenable() : thrownValue; - if (request.status === ABORTING) { + if (request.aborted) { // We are aborting so we can just bubble up to the task by falling through } else if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] @@ -5124,11 +5125,11 @@ function retryRenderTask( // (unstable) API for suspending. This implementation detail can change // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() - : request.status === ABORTING + : request.aborted ? request.fatalError : thrownValue; - if (request.status === ABORTING && request.trackedPostpones !== null) { + if (request.aborted && request.trackedPostpones !== null) { // We are aborting a prerender and need to halt this task. const trackedPostpones = request.trackedPostpones; const thrownInfo = getThrownInfo(task.componentStack); @@ -5250,7 +5251,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { erroredReplay( request, task.blockedBoundary, - request.status === ABORTING ? request.fatalError : x, + request.aborted ? request.fatalError : x, errorInfo, task.replay.nodes, task.replay.slots, @@ -6155,12 +6156,15 @@ 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) { + if ( + request.aborted || + (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; + request.aborted = true; try { const abortableTasks = request.abortableTasks; From 059f19ebf519023d07a7d55f19d710192e86e2a2 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Sun, 31 May 2026 09:40:07 -0700 Subject: [PATCH 2/3] [Fizz] Fix aborts during resumed rendering Fizz previously identified a task that was rendering during an abort by marking its blocked Segment as `RENDERING`. This does not work for resumed replay tasks because they do not have a Segment, so an abort during replay could eagerly abort the task before it unwound and then report the internal `null` throw instead of the abort reason. Track the task currently executing on the Request so aborting can leave the in-flight task to unwind through its normal error path for both render and replay tasks. Since this replaces the only purpose of the Segment `RENDERING` status, remove that status and its associated bookkeeping. When resumed work unwinds after aborting, use the request's abort reason in the replay catch paths so aborting while replaying a prerendered tree or while rendering a resumed segment reports the meaningful abort reason instead of the internal control-flow value. --- .../ReactDOMFizzStaticBrowser-test.js | 109 ++++++++++++++++++ packages/react-server/src/ReactFizzServer.js | 34 +++--- 2 files changed, 127 insertions(+), 16 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 30dce64f104..416b665bf54 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -605,6 +605,115 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(getVisibleChildren(container)).toEqual(
Hello
); }); + it('can abort while replaying a prerendered tree', async () => { + const promise = new Promise(() => {}); + let prerendering = true; + const resumeController = new AbortController(); + + function AbortDuringReplay({children}) { + if (!prerendering) { + resumeController.abort('resume abort'); + } + return children; + } + + function Wait() { + return prerendering ? React.use(promise) : 'Hello'; + } + + function App() { + return ( +
+ + + + + +
+ ); + } + + const controller = new AbortController(); + let pendingResult; + await serverAct(() => { + pendingResult = ReactDOMFizzStatic.prerender(, { + signal: controller.signal, + onError() {}, + }); + }); + controller.abort('prerender abort'); + const prerendered = await pendingResult; + + prerendering = false; + const errors = []; + await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + { + signal: resumeController.signal, + onError(error) { + errors.push(error); + }, + }, + ), + ); + + expect(errors).toEqual(['resume abort']); + }); + + it('can abort while rendering a resumed segment', async () => { + const promise = new Promise(() => {}); + let prerendering = true; + const resumeController = new AbortController(); + + function Wait() { + if (prerendering) { + return React.use(promise); + } + resumeController.abort('resume abort'); + return 'Hello'; + } + + function App() { + return ( +
+ + + +
+ ); + } + + const controller = new AbortController(); + let pendingResult; + await serverAct(() => { + pendingResult = ReactDOMFizzStatic.prerender(, { + signal: controller.signal, + onError() {}, + }); + }); + controller.abort('prerender abort'); + const prerendered = await pendingResult; + + prerendering = false; + const errors = []; + await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + { + signal: resumeController.signal, + onError(error) { + errors.push(error); + }, + }, + ), + ); + + expect(errors).toEqual(['resume abort']); + }); + it('can prerender a preamble', async () => { const errors = []; diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 5d479f142ab..59146a21d92 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -336,12 +336,11 @@ const FLUSHED = 2; const ABORTED = 3; const ERRORED = 4; const POSTPONED = 5; -const RENDERING = 6; type Root = null; type Segment = { - status: 0 | 1 | 2 | 3 | 4 | 5 | 6, + status: 0 | 1 | 2 | 3 | 4 | 5, parentFlushed: boolean, // typically a segment will be flushed by its parent, except if its parent was already flushed id: number, // starts as 0 and is lazily assigned if the parent flushes early +index: number, // the index within the parent's chunks or 0 at the root @@ -381,6 +380,7 @@ export opaque type Request = { byteSize: number, // counts the number of bytes accumulated in the shell abortableTasks: Set, pingedTasks: Array, // High priority tasks that should be worked on first. + currentTask: null | Task, // The task currently executing in this request. // Queues to flush in order of priority clientRenderedBoundaries: Array, // Errored or client rendered but not yet flushed. completedBoundaries: Array, // Completed but not yet fully flushed boundaries to show. @@ -539,6 +539,7 @@ function RequestInstance( this.byteSize = 0; this.abortableTasks = abortSet; this.pingedTasks = pingedTasks; + this.currentTask = null; this.clientRenderedBoundaries = ([]: Array); this.completedBoundaries = ([]: Array); this.partialBoundaries = ([]: Array); @@ -1473,7 +1474,6 @@ function renderSuspenseBoundary( replaceSuspenseComponentStackWithSuspenseFallbackStack( suspenseComponentStack, ); - boundarySegment.status = RENDERING; try { renderNode(request, task, fallback, -1); pushSegmentFinale( @@ -1548,8 +1548,6 @@ function renderSuspenseBoundary( prevContext, ); task.row = null; - contentRootSegment.status = RENDERING; - try { // We use the safe form because we don't handle suspending here. Only error handling. renderNode(request, task, content, -1); @@ -1744,8 +1742,9 @@ function replaySuspenseBoundary( // faster return; } - } catch (error: mixed) { + } catch (thrownValue: mixed) { resumedBoundary.status = CLIENT_RENDERED; + const error = request.aborted ? request.fatalError : thrownValue; const thrownInfo = getThrownInfo(task.componentStack); const errorDigest = logRecoverableError( request, @@ -2263,7 +2262,6 @@ function renderPreamble( blockedSegment.preambleChildren.push(preambleSegment); task.blockedSegment = preambleSegment; try { - preambleSegment.status = RENDERING; renderNode(request, task, node, -1); pushSegmentFinale( preambleSegment.chunks, @@ -3161,7 +3159,7 @@ function replayElement( erroredReplay( request, task.blockedBoundary, - x, + request.aborted ? request.fatalError : x, thrownInfo, childNodes, childSlots, @@ -3676,7 +3674,7 @@ function replayFragment( erroredReplay( request, task.blockedBoundary, - x, + request.aborted ? request.fatalError : x, thrownInfo, childNodes, childSlots, @@ -4596,14 +4594,13 @@ function abortRemainingReplayNodes( function abortTask(task: Task, request: Request, error: mixed): void { // This aborts the task and aborts the parent that it blocks, putting it into // client rendered mode. + if (task === request.currentTask) { + // This is a currently rendering Task. The render itself will abort the task. + return; + } const boundary = task.blockedBoundary; const segment = task.blockedSegment; if (segment !== null) { - if (segment.status === RENDERING) { - // This is the a currently rendering Segment. The render itself will - // abort the task. - return; - } segment.status = ABORTED; } @@ -5079,8 +5076,8 @@ function retryRenderTask( return; } - // We track when a Segment is rendering so we can handle aborts while rendering - segment.status = RENDERING; + const prevTask = request.currentTask; + request.currentTask = task; // We restore the context to what it was when we suspended. // We don't restore it after we leave because it's likely that we'll end up @@ -5177,6 +5174,7 @@ function retryRenderTask( ); return; } finally { + request.currentTask = prevTask; if (__DEV__) { setCurrentTaskInDEV(prevTaskInDEV); } @@ -5189,6 +5187,9 @@ function retryReplayTask(request: Request, task: ReplayTask): void { return; } + const prevTask = request.currentTask; + request.currentTask = task; + // We restore the context to what it was when we suspended. // We don't restore it after we leave because it's likely that we'll end up // needing a very similar context soon again. @@ -5267,6 +5268,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { } return; } finally { + request.currentTask = prevTask; if (__DEV__) { setCurrentTaskInDEV(prevTaskInDEV); } From c7e71318bf455dc9375e5502ccf86045de86b6b0 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 29 May 2026 17:01:02 -0700 Subject: [PATCH 3/3] [Fizz] Finish abort in a scheduled task `abort()` currently performs both the synchronous transition into an aborted request and the reporting/completion of every unfinished task in the same call. This change splits those phases. Aborting now synchronously marks the request as aborted, captures the abort reason, claims pending tasks so already scheduled work cannot continue rendering them, and captures any DEV async debug information needed at the point of abort. Reporting and completing the claimed tasks is then performed from a scheduled `finishAbort()` callback. This split does not yet allow a promise rejected by an abort listener to replace the abort reason: work remains blocked once the request has been aborted, and tests assert that abort-time rejections still report the original abort reason. It establishes the task boundary needed for a follow-up change to selectively process rejected suspended work before completing the remaining aborted tasks. This is observable for streaming renders because abort cleanup may now happen after already available output is read. A Suspense boundary that was previously converted to client rendering before it could be serialized may instead be emitted as pending first and receive its client-render instruction when the scheduled abort completion runs. The scheduled finish must also preserve abort-during-render behavior in renderers whose scheduler executes synchronously. The request tracks its currently executing task, and both abort phases leave that task alone so it can unwind through its normal abort path rather than being completed twice or reporting an internal control-flow value. --- .../src/__tests__/ReactDOMFizzServer-test.js | 38 ++++- .../ReactDOMFizzServerBrowser-test.js | 18 ++- .../__tests__/ReactDOMFizzServerNode-test.js | 4 + .../ReactDOMFizzStaticBrowser-test.js | 104 +++++++++++-- .../__tests__/ReactDOMFizzStaticNode-test.js | 53 +++++++ .../src/__tests__/ReactFlightDOMNode-test.js | 2 +- packages/react-server/src/ReactFizzServer.js | 143 +++++++++++++----- 7 files changed, 296 insertions(+), 66 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0b56fe34710..bd28fa5443b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -7100,7 +7100,7 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual(['abort reason', 'abort reason']); }); - it('reports a root task that suspends after aborting during render', async () => { + it('reports a root task before rendering a suspended child returned after aborting', async () => { const promise = new Promise(() => {}); function SuspendedRoot() { use(promise); @@ -7138,6 +7138,34 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual(['abort reason', 'abort reason']); }); + it('currently does not report a root task that suspends directly after aborting during render', async () => { + const promise = new Promise(() => {}); + const abortRef = {current: null}; + function ComponentThatAbortsAndSuspends() { + abortRef.current(new Error('abort reason')); + use(promise); + return null; + } + + const errors = []; + await act(() => { + const {abort} = renderToPipeableStream( + , + { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }, + ); + abortRef.current = abort; + }); + + // The task suspends before renderFunctionComponent gets to throw the + // abort reason, and retryRenderTask currently suspends it again. + expect(errors).toEqual([]); + }); + it('can abort during render in a lazy initializer for a component', async () => { function Sibling() { return

sibling

; @@ -8443,6 +8471,10 @@ describe('ReactDOMFizzServer', () => { expect(thrownError).toBe('boom'); expect(errors).toEqual([ + { + error: 'boom', + componentStack: componentStack(['Abort', 'body', 'html', 'App']), + }, { error: 'boom', componentStack: componentStack([ @@ -8462,10 +8494,6 @@ describe('ReactDOMFizzServer', () => { 'App', ]), }, - { - error: 'boom', - componentStack: componentStack(['Abort', 'body', 'html', 'App']), - }, ]); // We expect the render to throw before streaming anything so the default diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 27e74750c45..9bd3f641040 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -221,7 +221,9 @@ describe('ReactDOMFizzServerBrowser', () => { ), ); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const result = await readResult(stream); expect(result).toContain('Loading'); @@ -247,7 +249,9 @@ describe('ReactDOMFizzServerBrowser', () => { ); const theReason = new Error('aborted for reasons'); - controller.abort(theReason); + await serverAct(() => { + controller.abort(theReason); + }); let caughtError = null; try { @@ -364,7 +368,7 @@ describe('ReactDOMFizzServerBrowser', () => { const reader = stream.getReader(); await reader.read(); - await reader.cancel(); + await serverAct(() => reader.cancel()); expect(errors).toEqual([ 'The render was aborted by the server without a reason.', @@ -463,7 +467,9 @@ describe('ReactDOMFizzServerBrowser', () => { }), ); - controller.abort('foobar'); + await serverAct(() => { + controller.abort('foobar'); + }); expect(errors).toEqual(['foobar', 'foobar']); }); @@ -502,7 +508,9 @@ describe('ReactDOMFizzServerBrowser', () => { }), ); - controller.abort(new Error('uh oh')); + await serverAct(() => { + controller.abort(new Error('uh oh')); + }); expect(errors).toEqual(['uh oh', 'uh oh']); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index d4152e8efc0..ad9678a2b36 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -382,6 +382,7 @@ describe('ReactDOMFizzServerNode', () => { expect(isCompleteCalls).toBe(0); abort(new Error('uh oh')); + await jest.runAllTimers(); await completed; @@ -468,6 +469,7 @@ describe('ReactDOMFizzServerNode', () => { const reason = new Error('abort reason'); abort(reason); + await jest.runAllTimers(); expect(shellErrors).toEqual([reason]); expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']); @@ -504,6 +506,7 @@ describe('ReactDOMFizzServerNode', () => { expect(isCompleteCalls).toBe(0); abort(); + await jest.runAllTimers(); await completed; @@ -746,6 +749,7 @@ describe('ReactDOMFizzServerNode', () => { resolve(); await completed; + await jest.runAllTimers(); expect(errors).toEqual([ 'The destination stream errored while writing data.', diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 416b665bf54..b9c9b0fd25b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -299,7 +299,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const result = await resultPromise; @@ -329,7 +331,9 @@ describe('ReactDOMFizzStaticBrowser', () => { await jest.runAllTimers(); const theReason = new Error('aborted for reasons'); - controller.abort(theReason); + await serverAct(() => { + controller.abort(theReason); + }); let rejected = false; let prelude; @@ -447,7 +451,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort('foobar'); + await serverAct(() => { + controller.abort('foobar'); + }); await resultPromise; @@ -489,13 +495,63 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(new Error('uh oh')); + await serverAct(() => { + controller.abort(new Error('uh oh')); + }); await resultPromise; expect(errors).toEqual(['uh oh', 'uh oh']); }); + it('currently uses the abort reason when an abort listener synchronously rejects pending work', async () => { + let reject; + const rejectedPromise = new Promise((resolve, rejectPromise) => { + reject = rejectPromise; + }); + const haltedPromise = new Promise(() => {}); + function RejectedWait() { + React.use(rejectedPromise); + return null; + } + function HaltedWait() { + React.use(haltedPromise); + return null; + } + + const errors = []; + const controller = new AbortController(); + let resultPromise; + await serverAct(() => { + resultPromise = ReactDOMFizzStatic.prerender( + <> + + + + + + + , + { + signal: controller.signal, + onError(error) { + errors.push(error.message); + }, + }, + ); + }); + + controller.signal.addEventListener('abort', () => { + reject(new Error('rejected during abort')); + }); + await serverAct(() => { + controller.abort(new Error('abort reason')); + }); + await resultPromise; + + expect(errors).toEqual(['abort reason', 'abort reason']); + }); + it('logs an error if onHeaders throws but continues the prerender', async () => { const errors = []; function onError(error) { @@ -562,7 +618,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -587,7 +645,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); @@ -641,7 +701,9 @@ describe('ReactDOMFizzStaticBrowser', () => { onError() {}, }); }); - controller.abort('prerender abort'); + await serverAct(() => { + controller.abort('prerender abort'); + }); const prerendered = await pendingResult; prerendering = false; @@ -693,7 +755,9 @@ describe('ReactDOMFizzStaticBrowser', () => { onError() {}, }); }); - controller.abort('prerender abort'); + await serverAct(() => { + controller.abort('prerender abort'); + }); const prerendered = await pendingResult; prerendering = false; @@ -761,7 +825,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -792,7 +858,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); @@ -855,7 +923,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(new Error('boom')); + await serverAct(() => { + controller.abort(new Error('boom')); + }); const prerendered = await pendingResult; @@ -917,7 +987,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; @@ -995,7 +1067,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -1021,7 +1095,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js index 83bc6cb5d30..56ffa30a048 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js @@ -216,6 +216,7 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort(); + await jest.runAllTimers(); const result = await resultPromise; @@ -244,6 +245,7 @@ describe('ReactDOMFizzStaticNode', () => { const theReason = new Error('aborted for reasons'); controller.abort(theReason); + await jest.runAllTimers(); let didThrow = false; let prelude; @@ -281,6 +283,7 @@ describe('ReactDOMFizzStaticNode', () => { }, ); + await jest.runAllTimers(); const {prelude} = await streamPromise; const content = await readContent(prelude); expect(errors).toEqual(['This operation was aborted']); @@ -309,6 +312,7 @@ describe('ReactDOMFizzStaticNode', () => { // Technically we could still continue rendering the shell but currently the // semantics mean that we also abort any pending CPU work. + await jest.runAllTimers(); let didThrow = false; let prelude; @@ -358,6 +362,7 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort('foobar'); + await jest.runAllTimers(); await resultPromise; @@ -399,9 +404,57 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort(new Error('uh oh')); + await jest.runAllTimers(); await resultPromise; expect(errors).toEqual(['uh oh', 'uh oh']); }); + + it('currently uses the abort reason when an abort listener synchronously rejects pending work', async () => { + let reject; + const rejectedPromise = new Promise((resolve, rejectPromise) => { + reject = rejectPromise; + }); + const haltedPromise = new Promise(() => {}); + function RejectedWait() { + React.use(rejectedPromise); + return null; + } + function HaltedWait() { + React.use(haltedPromise); + return null; + } + + const errors = []; + const controller = new AbortController(); + const resultPromise = ReactDOMFizzStatic.prerenderToNodeStream( + <> + + + + + + + , + { + signal: controller.signal, + onError(error) { + errors.push(error.message); + }, + }, + ); + + await jest.runAllTimers(); + + controller.signal.addEventListener('abort', () => { + reject(new Error('rejected during abort')); + }); + controller.abort(new Error('abort reason')); + + await jest.runAllTimers(); + await resultPromise; + + expect(errors).toEqual(['abort reason', 'abort reason']); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index d7b0fc6c99d..27853fc2381 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -2051,7 +2051,7 @@ describe('ReactFlightDOMNode', () => { ); expect(result).toContain( - 'Switched to client rendering because the server rendering aborted due to:\n\n' + + 'Switched to client rendering because the server rendering aborted due to:\\n\\n' + 'ssr-abort', ); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 59146a21d92..0cfae20be25 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -356,6 +356,9 @@ type Segment = { textEmbedded: boolean, }; +// The ordering of these statuses matters. OPENING and OPEN are the only +// statuses in which newly scheduled work may be performed. Any status greater +// than OPEN represents a request that no longer admits work. const OPENING = 10; const OPEN = 11; const CLOSING = 12; @@ -4591,9 +4594,9 @@ function abortRemainingReplayNodes( } } -function abortTask(task: Task, request: Request, error: mixed): void { - // This aborts the task and aborts the parent that it blocks, putting it into - // client rendered mode. +function abortTask(task: Task, request: Request): void { + // Mark pending tasks as aborted synchronously so any work that was already + // scheduled cannot begin after abort is called. if (task === request.currentTask) { // This is a currently rendering Task. The render itself will abort the task. return; @@ -4604,17 +4607,13 @@ function abortTask(task: Task, request: Request, error: mixed): void { segment.status = ABORTED; } - const errorInfo = getThrownInfo(task.componentStack); if (__DEV__ && enableAsyncDebugInfo) { - // If the task is not rendering, then this is an async abort. Conceptually it's as if - // the abort happened inside the async gap. The abort reason's stack frame won't have that - // on the stack so instead we use the owner stack and debug task of any halted async debug info. + // Capture async debug information at the point abort begins. The task may + // receive more data before finishAbort runs and no longer suspend at the + // call site we need to report. let node: any = task.node; if (node !== null && typeof node === 'object') { - // Push a fake component stack frame that represents the await. let debugInfo = node._debugInfo; - // First resolve lazy nodes to find debug info that has been transferred - // to the inner value. while ( typeof node === 'object' && node !== null && @@ -4645,6 +4644,29 @@ function abortTask(task: Task, request: Request, error: mixed): void { } } + if (boundary !== null) { + boundary.fallbackAbortableTasks.forEach(fallbackTask => + abortTask(fallbackTask, request), + ); + } +} + +function finishAbortedTask(task: Task, request: Request, error: mixed): void { + // Report and complete a task that was synchronously claimed by abortTask. + // A currently rendering task remains responsible for unwinding itself. + if (task === request.currentTask) { + return; + } + const boundary = task.blockedBoundary; + const segment = task.blockedSegment; + if (segment !== null) { + if (segment.status !== ABORTED) { + return; + } + } + + const errorInfo = getThrownInfo(task.componentStack); + if (boundary === null) { const replay: null | ReplaySet = task.replay; if (replay === null) { @@ -4706,7 +4728,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // If this boundary was still pending then we haven't already cancelled its fallbacks. // We'll need to abort the fallbacks, which will also error that parent boundary. boundary.fallbackAbortableTasks.forEach(fallbackTask => - abortTask(fallbackTask, request, error), + finishAbortedTask(fallbackTask, request, error), ); boundary.fallbackAbortableTasks.clear(); return finishedTask(request, boundary, task.row, segment); @@ -4743,7 +4765,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // If this boundary was still pending then we haven't already cancelled its fallbacks. // We'll need to abort the fallbacks, which will also error that parent boundary. boundary.fallbackAbortableTasks.forEach(fallbackTask => - abortTask(fallbackTask, request, error), + finishAbortedTask(fallbackTask, request, error), ); boundary.fallbackAbortableTasks.clear(); } @@ -4761,14 +4783,39 @@ function abortTask(task: Task, request: Request, error: mixed): void { } } -function abortTaskDEV(task: Task, request: Request, error: mixed): void { +function finishAbortedTaskDEV( + task: Task, + request: Request, + error: mixed, +): void { if (__DEV__) { const prevTaskInDEV = currentTaskInDEV; const prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack; setCurrentTaskInDEV(task); ReactSharedInternals.getCurrentStack = getCurrentStackInDEV; try { - abortTask(task, request, error); + finishAbortedTask(task, request, error); + } finally { + setCurrentTaskInDEV(prevTaskInDEV); + ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl; + } + } else { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'finishAbortedTaskDEV should never be called in production mode. This is a bug in React.', + ); + } +} + +function abortTaskDEV(task: Task, request: Request): void { + if (__DEV__) { + const prevTaskInDEV = currentTaskInDEV; + const prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack; + setCurrentTaskInDEV(task); + ReactSharedInternals.getCurrentStack = getCurrentStackInDEV; + try { + abortTask(task, request); } finally { setCurrentTaskInDEV(prevTaskInDEV); ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl; @@ -5276,7 +5323,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { } export function performWork(request: Request): void { - if (request.status === CLOSED || request.status === CLOSING) { + if (request.aborted || request.status > OPEN) { return; } const prevContext = getActiveContext(); @@ -6156,36 +6203,16 @@ export function stopFlowing(request: Request): void { request.destination = null; } -// 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.aborted || - (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.aborted = true; - +function finishAbort(request: Request, abortableTasks: Set): void { try { - const abortableTasks = request.abortableTasks; if (abortableTasks.size > 0) { - const error = - reason === undefined - ? new Error('The render was aborted by the server without a reason.') - : typeof reason === 'object' && - reason !== null && - typeof reason.then === 'function' - ? new Error('The render was aborted by the server with a promise.') - : reason; - // This error isn't necessarily fatal in this case but we need to stash it - // so we can use it to abort any pending work - request.fatalError = error; + const error = request.fatalError; if (__DEV__) { - abortableTasks.forEach(task => abortTaskDEV(task, request, error)); + abortableTasks.forEach(task => + finishAbortedTaskDEV(task, request, error), + ); } else { - abortableTasks.forEach(task => abortTask(task, request, error)); + abortableTasks.forEach(task => finishAbortedTask(task, request, error)); } abortableTasks.clear(); } @@ -6199,6 +6226,40 @@ export function abort(request: Request, reason: mixed): 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.aborted || + (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.aborted = true; + const error = + reason === undefined + ? new Error('The render was aborted by the server without a reason.') + : typeof reason === 'object' && + reason !== null && + typeof reason.then === 'function' + ? new Error('The render was aborted by the server with a promise.') + : reason; + // This error isn't necessarily fatal in this case but we need to stash it + // so we can use it to abort any pending work. + request.fatalError = error; + const abortableTasks = request.abortableTasks; + if (__DEV__) { + abortableTasks.forEach(task => abortTaskDEV(task, request)); + } else { + abortableTasks.forEach(task => abortTask(task, request)); + } + // Even though this looks async some renderers schedule work sync + // So it is important that the finish step does not assume the stack + // has unwinded yet + scheduleWork(() => finishAbort(request, abortableTasks)); +} + export function flushResources(request: Request): void { enqueueFlush(request); }