Skip to content

Commit de8e005

Browse files
authored
[Fizz] Fix aborts during resumed rendering (react#36584)
Stacked on react#36583 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.
1 parent ddcb58f commit de8e005

2 files changed

Lines changed: 127 additions & 16 deletions

File tree

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,115 @@ describe('ReactDOMFizzStaticBrowser', () => {
605605
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
606606
});
607607

608+
it('can abort while replaying a prerendered tree', async () => {
609+
const promise = new Promise(() => {});
610+
let prerendering = true;
611+
const resumeController = new AbortController();
612+
613+
function AbortDuringReplay({children}) {
614+
if (!prerendering) {
615+
resumeController.abort('resume abort');
616+
}
617+
return children;
618+
}
619+
620+
function Wait() {
621+
return prerendering ? React.use(promise) : 'Hello';
622+
}
623+
624+
function App() {
625+
return (
626+
<div>
627+
<AbortDuringReplay>
628+
<Suspense fallback="Loading 1...">
629+
<Wait />
630+
</Suspense>
631+
</AbortDuringReplay>
632+
</div>
633+
);
634+
}
635+
636+
const controller = new AbortController();
637+
let pendingResult;
638+
await serverAct(() => {
639+
pendingResult = ReactDOMFizzStatic.prerender(<App />, {
640+
signal: controller.signal,
641+
onError() {},
642+
});
643+
});
644+
controller.abort('prerender abort');
645+
const prerendered = await pendingResult;
646+
647+
prerendering = false;
648+
const errors = [];
649+
await serverAct(() =>
650+
ReactDOMFizzServer.resume(
651+
<App />,
652+
JSON.parse(JSON.stringify(prerendered.postponed)),
653+
{
654+
signal: resumeController.signal,
655+
onError(error) {
656+
errors.push(error);
657+
},
658+
},
659+
),
660+
);
661+
662+
expect(errors).toEqual(['resume abort']);
663+
});
664+
665+
it('can abort while rendering a resumed segment', async () => {
666+
const promise = new Promise(() => {});
667+
let prerendering = true;
668+
const resumeController = new AbortController();
669+
670+
function Wait() {
671+
if (prerendering) {
672+
return React.use(promise);
673+
}
674+
resumeController.abort('resume abort');
675+
return 'Hello';
676+
}
677+
678+
function App() {
679+
return (
680+
<div>
681+
<Suspense fallback="Loading...">
682+
<Wait />
683+
</Suspense>
684+
</div>
685+
);
686+
}
687+
688+
const controller = new AbortController();
689+
let pendingResult;
690+
await serverAct(() => {
691+
pendingResult = ReactDOMFizzStatic.prerender(<App />, {
692+
signal: controller.signal,
693+
onError() {},
694+
});
695+
});
696+
controller.abort('prerender abort');
697+
const prerendered = await pendingResult;
698+
699+
prerendering = false;
700+
const errors = [];
701+
await serverAct(() =>
702+
ReactDOMFizzServer.resume(
703+
<App />,
704+
JSON.parse(JSON.stringify(prerendered.postponed)),
705+
{
706+
signal: resumeController.signal,
707+
onError(error) {
708+
errors.push(error);
709+
},
710+
},
711+
),
712+
);
713+
714+
expect(errors).toEqual(['resume abort']);
715+
});
716+
608717
it('can prerender a preamble', async () => {
609718
const errors = [];
610719

packages/react-server/src/ReactFizzServer.js

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -336,12 +336,11 @@ const FLUSHED = 2;
336336
const ABORTED = 3;
337337
const ERRORED = 4;
338338
const POSTPONED = 5;
339-
const RENDERING = 6;
340339

341340
type Root = null;
342341

343342
type Segment = {
344-
status: 0 | 1 | 2 | 3 | 4 | 5 | 6,
343+
status: 0 | 1 | 2 | 3 | 4 | 5,
345344
parentFlushed: boolean, // typically a segment will be flushed by its parent, except if its parent was already flushed
346345
id: number, // starts as 0 and is lazily assigned if the parent flushes early
347346
+index: number, // the index within the parent's chunks or 0 at the root
@@ -381,6 +380,7 @@ export opaque type Request = {
381380
byteSize: number, // counts the number of bytes accumulated in the shell
382381
abortableTasks: Set<Task>,
383382
pingedTasks: Array<Task>, // High priority tasks that should be worked on first.
383+
currentTask: null | Task, // The task currently executing in this request.
384384
// Queues to flush in order of priority
385385
clientRenderedBoundaries: Array<SuspenseBoundary>, // Errored or client rendered but not yet flushed.
386386
completedBoundaries: Array<SuspenseBoundary>, // Completed but not yet fully flushed boundaries to show.
@@ -539,6 +539,7 @@ function RequestInstance(
539539
this.byteSize = 0;
540540
this.abortableTasks = abortSet;
541541
this.pingedTasks = pingedTasks;
542+
this.currentTask = null;
542543
this.clientRenderedBoundaries = ([]: Array<SuspenseBoundary>);
543544
this.completedBoundaries = ([]: Array<SuspenseBoundary>);
544545
this.partialBoundaries = ([]: Array<SuspenseBoundary>);
@@ -1473,7 +1474,6 @@ function renderSuspenseBoundary(
14731474
replaceSuspenseComponentStackWithSuspenseFallbackStack(
14741475
suspenseComponentStack,
14751476
);
1476-
boundarySegment.status = RENDERING;
14771477
try {
14781478
renderNode(request, task, fallback, -1);
14791479
pushSegmentFinale(
@@ -1548,8 +1548,6 @@ function renderSuspenseBoundary(
15481548
prevContext,
15491549
);
15501550
task.row = null;
1551-
contentRootSegment.status = RENDERING;
1552-
15531551
try {
15541552
// We use the safe form because we don't handle suspending here. Only error handling.
15551553
renderNode(request, task, content, -1);
@@ -1744,8 +1742,9 @@ function replaySuspenseBoundary(
17441742
// faster
17451743
return;
17461744
}
1747-
} catch (error: mixed) {
1745+
} catch (thrownValue: mixed) {
17481746
resumedBoundary.status = CLIENT_RENDERED;
1747+
const error = request.aborted ? request.fatalError : thrownValue;
17491748
const thrownInfo = getThrownInfo(task.componentStack);
17501749
const errorDigest = logRecoverableError(
17511750
request,
@@ -2263,7 +2262,6 @@ function renderPreamble(
22632262
blockedSegment.preambleChildren.push(preambleSegment);
22642263
task.blockedSegment = preambleSegment;
22652264
try {
2266-
preambleSegment.status = RENDERING;
22672265
renderNode(request, task, node, -1);
22682266
pushSegmentFinale(
22692267
preambleSegment.chunks,
@@ -3161,7 +3159,7 @@ function replayElement(
31613159
erroredReplay(
31623160
request,
31633161
task.blockedBoundary,
3164-
x,
3162+
request.aborted ? request.fatalError : x,
31653163
thrownInfo,
31663164
childNodes,
31673165
childSlots,
@@ -3676,7 +3674,7 @@ function replayFragment(
36763674
erroredReplay(
36773675
request,
36783676
task.blockedBoundary,
3679-
x,
3677+
request.aborted ? request.fatalError : x,
36803678
thrownInfo,
36813679
childNodes,
36823680
childSlots,
@@ -4596,14 +4594,13 @@ function abortRemainingReplayNodes(
45964594
function abortTask(task: Task, request: Request, error: mixed): void {
45974595
// This aborts the task and aborts the parent that it blocks, putting it into
45984596
// client rendered mode.
4597+
if (task === request.currentTask) {
4598+
// This is a currently rendering Task. The render itself will abort the task.
4599+
return;
4600+
}
45994601
const boundary = task.blockedBoundary;
46004602
const segment = task.blockedSegment;
46014603
if (segment !== null) {
4602-
if (segment.status === RENDERING) {
4603-
// This is the a currently rendering Segment. The render itself will
4604-
// abort the task.
4605-
return;
4606-
}
46074604
segment.status = ABORTED;
46084605
}
46094606

@@ -5079,8 +5076,8 @@ function retryRenderTask(
50795076
return;
50805077
}
50815078

5082-
// We track when a Segment is rendering so we can handle aborts while rendering
5083-
segment.status = RENDERING;
5079+
const prevTask = request.currentTask;
5080+
request.currentTask = task;
50845081

50855082
// We restore the context to what it was when we suspended.
50865083
// We don't restore it after we leave because it's likely that we'll end up
@@ -5177,6 +5174,7 @@ function retryRenderTask(
51775174
);
51785175
return;
51795176
} finally {
5177+
request.currentTask = prevTask;
51805178
if (__DEV__) {
51815179
setCurrentTaskInDEV(prevTaskInDEV);
51825180
}
@@ -5189,6 +5187,9 @@ function retryReplayTask(request: Request, task: ReplayTask): void {
51895187
return;
51905188
}
51915189

5190+
const prevTask = request.currentTask;
5191+
request.currentTask = task;
5192+
51925193
// We restore the context to what it was when we suspended.
51935194
// We don't restore it after we leave because it's likely that we'll end up
51945195
// needing a very similar context soon again.
@@ -5267,6 +5268,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void {
52675268
}
52685269
return;
52695270
} finally {
5271+
request.currentTask = prevTask;
52705272
if (__DEV__) {
52715273
setCurrentTaskInDEV(prevTaskInDEV);
52725274
}

0 commit comments

Comments
 (0)