Skip to content

Commit ddcb58f

Browse files
authored
[Fizz] Track abort state on Request (react#36583)
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.
1 parent 05ca66a commit ddcb58f

2 files changed

Lines changed: 36 additions & 37 deletions

File tree

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7063,7 +7063,7 @@ describe('ReactDOMFizzServer', () => {
70637063
);
70647064
});
70657065

7066-
it('currently does not report an in-flight root task after another root task fatals while aborting', async () => {
7066+
it('reports an in-flight root task after another root task fatals while aborting', async () => {
70677067
const promise = new Promise(() => {});
70687068
function SuspendedRoot() {
70697069
use(promise);
@@ -7097,10 +7097,10 @@ describe('ReactDOMFizzServer', () => {
70977097
abortRef.current = abort;
70987098
});
70997099

7100-
expect(errors).toEqual(['abort reason']);
7100+
expect(errors).toEqual(['abort reason', 'abort reason']);
71017101
});
71027102

7103-
it('currently does not report a root task that suspends after aborting during render', async () => {
7103+
it('reports a root task that suspends after aborting during render', async () => {
71047104
const promise = new Promise(() => {});
71057105
function SuspendedRoot() {
71067106
use(promise);
@@ -7135,9 +7135,7 @@ describe('ReactDOMFizzServer', () => {
71357135
abortRef.current = abort;
71367136
});
71377137

7138-
// TODO: Once abort completion is async, this still-suspended task should
7139-
// observe ABORTING and report the abort reason as well.
7140-
expect(errors).toEqual(['abort reason']);
7138+
expect(errors).toEqual(['abort reason', 'abort reason']);
71417139
});
71427140

71437141
it('can abort during render in a lazy initializer for a component', async () => {
@@ -8444,10 +8442,6 @@ describe('ReactDOMFizzServer', () => {
84448442
}
84458443

84468444
expect(thrownError).toBe('boom');
8447-
// TODO there should actually be three errors. One for the pending Suspense, one for the fallback task, and one for the task
8448-
// that does the abort itself. At the moment abort will flush queues and if there is no pending tasks will close the request before
8449-
// 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
8450-
// so I am asserting the current behavior
84518445
expect(errors).toEqual([
84528446
{
84538447
error: 'boom',
@@ -8467,9 +8461,10 @@ describe('ReactDOMFizzServer', () => {
84678461
'html',
84688462
'App',
84698463
]),
8470-
// }, {
8471-
// error: 'boom',
8472-
// componentStack: componentStack(['Abort', 'body', 'html', 'App'])
8464+
},
8465+
{
8466+
error: 'boom',
8467+
componentStack: componentStack(['Abort', 'body', 'html', 'App']),
84738468
},
84748469
]);
84758470

packages/react-server/src/ReactFizzServer.js

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -359,10 +359,9 @@ type Segment = {
359359

360360
const OPENING = 10;
361361
const OPEN = 11;
362-
const ABORTING = 12;
363-
const CLOSING = 13;
364-
const CLOSED = 14;
365-
const STALLED_DEV = 15;
362+
const CLOSING = 12;
363+
const CLOSED = 13;
364+
const STALLED_DEV = 14;
366365

367366
export opaque type Request = {
368367
destination: null | Destination,
@@ -371,8 +370,9 @@ export opaque type Request = {
371370
+renderState: RenderState,
372371
+rootFormatContext: FormatContext,
373372
+progressiveChunkSize: number,
374-
status: 10 | 11 | 12 | 13 | 14 | 15,
373+
status: 10 | 11 | 12 | 13 | 14,
375374
fatalError: mixed,
375+
aborted: boolean,
376376
nextSegmentId: number,
377377
allPendingTasks: number, // when it reaches zero, we can close the connection.
378378
pendingRootTasks: number, // when this reaches zero, we've finished at least the root boundary.
@@ -530,6 +530,7 @@ function RequestInstance(
530530
: progressiveChunkSize;
531531
this.status = isWorkLoopExternallyDriven ? OPEN : OPENING;
532532
this.fatalError = null;
533+
this.aborted = false;
533534
this.nextSegmentId = 0;
534535
this.allPendingTasks = 0;
535536
this.pendingRootTasks = 0;
@@ -1038,7 +1039,11 @@ function pushHaltedAwaitOnComponentStack(
10381039
// performWork + retryTask without mutation
10391040
function rerenderStalledTask(request: Request, task: Task): void {
10401041
const prevStatus = request.status;
1042+
const prevAborted = request.aborted;
10411043
request.status = STALLED_DEV;
1044+
// This diagnostic replay must reach the suspended call site instead of
1045+
// taking the abort path.
1046+
request.aborted = false;
10421047

10431048
const prevContext = getActiveContext();
10441049
const prevDispatcher = ReactSharedInternals.H;
@@ -1082,6 +1087,7 @@ function rerenderStalledTask(request: Request, task: Task): void {
10821087
}
10831088
currentRequest = prevRequest;
10841089
request.status = prevStatus;
1090+
request.aborted = prevAborted;
10851091
}
10861092
}
10871093

@@ -1479,7 +1485,7 @@ function renderSuspenseBoundary(
14791485
boundarySegment.status = COMPLETED;
14801486
finishedSegment(request, parentBoundary, boundarySegment);
14811487
} catch (thrownValue: mixed) {
1482-
if (request.status === ABORTING) {
1488+
if (request.aborted) {
14831489
boundarySegment.status = ABORTED;
14841490
} else {
14851491
boundarySegment.status = ERRORED;
@@ -1589,7 +1595,7 @@ function renderSuspenseBoundary(
15891595
} catch (thrownValue: mixed) {
15901596
newBoundary.status = CLIENT_RENDERED;
15911597
let error: mixed;
1592-
if (request.status === ABORTING) {
1598+
if (request.aborted) {
15931599
contentRootSegment.status = ABORTED;
15941600
error = request.fatalError;
15951601
} else {
@@ -2075,7 +2081,7 @@ function renderSuspenseListRows(
20752081
finishSuspenseListRow(request, previousSuspenseListRow);
20762082
}
20772083
} catch (thrownValue: mixed) {
2078-
if (request.status === ABORTING) {
2084+
if (request.aborted) {
20792085
newSegment.status = ABORTED;
20802086
} else {
20812087
newSegment.status = ERRORED;
@@ -2392,7 +2398,7 @@ function finishClassComponent(
23922398
} else {
23932399
nextChildren = instance.render();
23942400
}
2395-
if (request.status === ABORTING) {
2401+
if (request.aborted) {
23962402
// eslint-disable-next-line no-throw-literal
23972403
throw null;
23982404
}
@@ -2541,7 +2547,7 @@ function renderFunctionComponent(
25412547
props,
25422548
legacyContext,
25432549
);
2544-
if (request.status === ABORTING) {
2550+
if (request.aborted) {
25452551
// eslint-disable-next-line no-throw-literal
25462552
throw null;
25472553
}
@@ -2818,12 +2824,7 @@ function renderLazyComponent(
28182824
const init = lazyComponent._init;
28192825
Component = init(payload);
28202826
}
2821-
if (
2822-
request.status === ABORTING &&
2823-
// We're going to discard this render anyway.
2824-
// We just need to reach the point where we suspended in dev.
2825-
(!__DEV__ || request.status !== STALLED_DEV)
2826-
) {
2827+
if (request.aborted) {
28272828
// eslint-disable-next-line no-throw-literal
28282829
throw null;
28292830
}
@@ -3441,7 +3442,7 @@ function retryNode(request: Request, task: Task): void {
34413442
const init = lazyNode._init;
34423443
resolvedNode = init(payload);
34433444
}
3444-
if (request.status === ABORTING) {
3445+
if (request.aborted) {
34453446
// eslint-disable-next-line no-throw-literal
34463447
throw null;
34473448
}
@@ -4185,7 +4186,7 @@ function renderNode(
41854186
getSuspendedThenable()
41864187
: thrownValue;
41874188

4188-
if (request.status === ABORTING) {
4189+
if (request.aborted) {
41894190
// We are aborting so we can just bubble up to the task by falling through
41904191
} else if (typeof x === 'object' && x !== null) {
41914192
// $FlowFixMe[method-unbinding]
@@ -4286,7 +4287,7 @@ function renderNode(
42864287
getSuspendedThenable()
42874288
: thrownValue;
42884289

4289-
if (request.status === ABORTING) {
4290+
if (request.aborted) {
42904291
// We are aborting so we can just bubble up to the task by falling through
42914292
} else if (typeof x === 'object' && x !== null) {
42924293
// $FlowFixMe[method-unbinding]
@@ -5124,11 +5125,11 @@ function retryRenderTask(
51245125
// (unstable) API for suspending. This implementation detail can change
51255126
// later, once we deprecate the old API in favor of `use`.
51265127
getSuspendedThenable()
5127-
: request.status === ABORTING
5128+
: request.aborted
51285129
? request.fatalError
51295130
: thrownValue;
51305131

5131-
if (request.status === ABORTING && request.trackedPostpones !== null) {
5132+
if (request.aborted && request.trackedPostpones !== null) {
51325133
// We are aborting a prerender and need to halt this task.
51335134
const trackedPostpones = request.trackedPostpones;
51345135
const thrownInfo = getThrownInfo(task.componentStack);
@@ -5250,7 +5251,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void {
52505251
erroredReplay(
52515252
request,
52525253
task.blockedBoundary,
5253-
request.status === ABORTING ? request.fatalError : x,
5254+
request.aborted ? request.fatalError : x,
52545255
errorInfo,
52555256
task.replay.nodes,
52565257
task.replay.slots,
@@ -6155,12 +6156,15 @@ export function stopFlowing(request: Request): void {
61556156

61566157
// This is called to early terminate a request. It puts all pending boundaries in client rendered state.
61576158
export function abort(request: Request, reason: mixed): void {
6158-
if (request.status !== OPEN && request.status !== OPENING) {
6159+
if (
6160+
request.aborted ||
6161+
(request.status !== OPEN && request.status !== OPENING)
6162+
) {
61596163
// Only requests that are not already complete or in the process of aborting
61606164
// can be aborted. in practice this makes abort callable at most once per render.
61616165
return;
61626166
}
6163-
request.status = ABORTING;
6167+
request.aborted = true;
61646168

61656169
try {
61666170
const abortableTasks = request.abortableTasks;

0 commit comments

Comments
 (0)