[Flight] do not emit error after abort (#30683)
When synchronously aborting in a non-async Function Component if you throw after aborting the task would error rather than abort because React never observed the AbortSignal. Using a sigil to throw after aborting during render isn't effective b/c the user code itself could throw so insteead we just read the request status. This is ok b/c we don't expect any tasks to still be pending after the currently running task finishes. However I found one instance where that wasn't true related to serializing thenables which I've fixed so we may find other cases. If we do, though it's almost certainly a bug in our task bookkeeping so we should just fix it if it comes up. I also updated `abort` to not set the status to ABORTING unless the status was OPEN. we don't want to ever leave CLOSED or CLOSING status
Josh Story committed
Aug 13, 2024 at 20:59 UTC
2a540194adde100c1af2b57b346e62eee760524c
2 files changed
+110
-9
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
+96
@@ -2554,4 +2554,100 @@ describe('ReactFlightDOM', () => {
2554
</div>,
2555
);
2556
});
2557
+
2558
+ it('can error synchronously after aborting in a synchronous Component', async () => {
2559
+ const rejectError = new Error('bam!');
2560
+ const rejectedPromise = Promise.reject(rejectError);
2561
+ rejectedPromise.catch(() => {});
2562
+ rejectedPromise.status = 'rejected';
2563
+ rejectedPromise.reason = rejectError;
2564
+
2565
+ const resolvedValue = <p>hello world</p>;
2566
+ const resolvedPromise = Promise.resolve(resolvedValue);
2567
+ resolvedPromise.status = 'fulfilled';
2568
+ resolvedPromise.value = resolvedValue;
2569
+
2570
+ function App() {
2571
+ return (
2572
+ <div>
2573
+ <Suspense fallback={<p>loading...</p>}>
2574
+ <ComponentThatAborts />
2575
+ </Suspense>
2576
+ <Suspense fallback={<p>loading too...</p>}>
2577
+ {rejectedPromise}
2578
+ </Suspense>
2579
+ <Suspense fallback={<p>loading three...</p>}>
2580
+ {resolvedPromise}
2581
+ </Suspense>
2582
+ </div>
2583
+ );
2584
+ }
2585
+
2586
+ const abortRef = {current: null};
2587
+
2588
+ // This test is specifically asserting that this works with Sync Server Component
2589
+ function ComponentThatAborts() {
2590
+ abortRef.current();
2591
+ throw new Error('boom');
2592
+ }
2593
+
2594
+ const {writable: flightWritable, readable: flightReadable} =
2595
+ getTestStream();
2596
+
2597
+ await serverAct(() => {
2598
+ const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream(
2599
+ <App />,
2600
+ webpackMap,
2601
+ {
2602
+ onError(e) {
2603
+ console.error(e);
2604
+ },
2605
+ },
2606
+ );
2607
+ abortRef.current = abort;
2608
+ pipe(flightWritable);
2609
+ });
2610
+
2611
+ assertConsoleErrorDev([
2612
+ 'The render was aborted by the server without a reason.',
2613
+ 'bam!',
2614
+ ]);
2615
+
2616
+ const response =
2617
+ ReactServerDOMClient.createFromReadableStream(flightReadable);
2618
+
2619
+ const {writable: fizzWritable, readable: fizzReadable} = getTestStream();
2620
+
2621
+ function ClientApp() {
2622
+ return use(response);
2623
+ }
2624
+
2625
+ const shellErrors = [];
2626
+ await serverAct(async () => {
2627
+ ReactDOMFizzServer.renderToPipeableStream(
2628
+ React.createElement(ClientApp),
2629
+ {
2630
+ onShellError(error) {
2631
+ shellErrors.push(error.message);
2632
+ },
2633
+ },
2634
+ ).pipe(fizzWritable);
2635
+ });
2636
+ assertConsoleErrorDev([
2637
+ 'The render was aborted by the server without a reason.',
2638
+ 'bam!',
2639
+ ]);
2640
+
2641
+ expect(shellErrors).toEqual([]);
2642
+
2643
+ const container = document.createElement('div');
2644
+ await readInto(container, fizzReadable);
2645
+ expect(getMeaningfulChildren(container)).toEqual(
2646
+ <div>
2647
+ <p>loading...</p>
2648
+ <p>loading too...</p>
2649
+ <p>hello world</p>
2650
+ </div>,
2651
+ );
2652
+ });
2653
});
packages/react-server/src/ReactFlightServer.js
+14
-9
@@ -382,8 +382,6 @@ export type Request = {
382
didWarnForKey: null | WeakSet<ReactComponentInfo>,
383
};
384
385
-const AbortSigil = {};
386
-
385
const {
386
TaintRegistryObjects,
387
TaintRegistryValues,
@@ -594,6 +592,8 @@ function serializeThenable(
592
const digest = logRecoverableError(request, x, null);
593
emitErrorChunk(request, newTask.id, digest, x);
594
}
595
+ newTask.status = ERRORED;
596
+ request.abortableTasks.delete(newTask);
597
return newTask.id;
598
}
599
default: {
@@ -650,10 +650,10 @@ function serializeThenable(
650
logPostpone(request, postponeInstance.message, newTask);
651
emitPostponeChunk(request, newTask.id, postponeInstance);
652
} else {
653
- newTask.status = ERRORED;
653
const digest = logRecoverableError(request, reason, newTask);
654
emitErrorChunk(request, newTask.id, digest, reason);
655
}
656
+ newTask.status = ERRORED;
657
request.abortableTasks.delete(newTask);
658
enqueueFlush(request);
659
},
@@ -1114,7 +1114,8 @@ function renderFunctionComponent<Props>(
1114
// If we aborted during rendering we should interrupt the render but
1115
// we don't need to provide an error because the renderer will encode
1116
// the abort error as the reason.
1117
- throw AbortSigil;
1117
+ // eslint-disable-next-line no-throw-literal
1118
+ throw null;
1119
}
1120
1121
if (
@@ -1616,7 +1617,8 @@ function renderElement(
1617
// lazy initializers are user code and could abort during render
1618
// we don't wan to return any value resolved from the lazy initializer
1619
// if it aborts so we interrupt rendering here
1619
- throw AbortSigil;
1620
+ // eslint-disable-next-line no-throw-literal
1621
+ throw null;
1622
}
1623
return renderElement(
1624
request,
@@ -2183,7 +2185,7 @@ function renderModel(
2185
}
2186
}
2187
2186
- if (thrownValue === AbortSigil) {
2188
+ if (request.status === ABORTING) {
2189
task.status = ABORTED;
2190
const errorId: number = (request.fatalError: any);
2191
if (wasReactNode) {
@@ -2357,7 +2359,8 @@ function renderModelDestructive(
2359
// lazy initializers are user code and could abort during render
2360
// we don't wan to return any value resolved from the lazy initializer
2361
// if it aborts so we interrupt rendering here
2360
- throw AbortSigil;
2362
+ // eslint-disable-next-line no-throw-literal
2363
+ throw null;
2364
}
2365
if (__DEV__) {
2366
const debugInfo: ?ReactDebugInfo = lazy._debugInfo;
@@ -3690,7 +3693,7 @@ function retryTask(request: Request, task: Task): void {
3693
}
3694
}
3695
3693
- if (x === AbortSigil) {
3696
+ if (request.status === ABORTING) {
3697
request.abortableTasks.delete(task);
3698
task.status = ABORTED;
3699
const errorId: number = (request.fatalError: any);
@@ -3909,7 +3912,9 @@ export function stopFlowing(request: Request): void {
3912
// This is called to early terminate a request. It creates an error at all pending tasks.
3913
export function abort(request: Request, reason: mixed): void {
3914
try {
3912
- request.status = ABORTING;
3915
+ if (request.status === OPEN) {
3916
+ request.status = ABORTING;
3917
+ }
3918
const abortableTasks = request.abortableTasks;
3919
// We have tasks to abort. We'll emit one error row and then emit a reference
3920
// to that row from every row that's still remaining.