Skip to content

Commit

Permalink
[Fizz] handle throwing after abort during render (facebook#30730)
Browse files Browse the repository at this point in the history
It is possible to throw after aborting during a render and we were not
properly tracking this. We use an AbortSigil to mark whether a rendering
task needs to abort but the throw interrupts that and we end up handling
an error on the error pathway instead.

This change reworks the abort-while-rendering support to be robust to
throws after calling abort
  • Loading branch information
gnoff authored Aug 17, 2024
1 parent 177b241 commit 7954db9
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 10 deletions.
42 changes: 42 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8377,6 +8377,48 @@ describe('ReactDOMFizzServer', () => {
);
});

it('can support throwing after aborting during a render', async () => {
function App() {
return (
<div>
<Suspense fallback={<p>loading...</p>}>
<ComponentThatAborts />
</Suspense>
</div>
);
}

function ComponentThatAborts() {
abortRef.current('boom');
throw new Error('bam');
}

const abortRef = {current: null};
let finished = false;
const errors = [];
await act(() => {
const {pipe, abort} = renderToPipeableStream(<App />, {
onError(err) {
errors.push(err);
},
});
abortRef.current = abort;
writable.on('finish', () => {
finished = true;
});
pipe(writable);
});

expect(errors).toEqual(['boom']);

expect(finished).toBe(true);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>loading...</p>
</div>,
);
});

it('should warn for using generators as children props', async () => {
function* getChildren() {
yield <h1 key="1">Hello</h1>;
Expand Down
22 changes: 12 additions & 10 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,6 @@ export function resumeRequest(
return request;
}

const AbortSigil = {};

let currentRequest: null | Request = null;

export function resolveRequest(): null | Request {
Expand Down Expand Up @@ -1173,7 +1171,7 @@ function renderSuspenseBoundary(
);
boundarySegment.status = COMPLETED;
} catch (thrownValue: mixed) {
if (thrownValue === AbortSigil) {
if (request.status === ABORTING) {
boundarySegment.status = ABORTED;
} else {
boundarySegment.status = ERRORED;
Expand Down Expand Up @@ -1246,7 +1244,7 @@ function renderSuspenseBoundary(
} catch (thrownValue: mixed) {
newBoundary.status = CLIENT_RENDERED;
let error: mixed;
if (thrownValue === AbortSigil) {
if (request.status === ABORTING) {
contentRootSegment.status = ABORTED;
error = request.fatalError;
} else {
Expand Down Expand Up @@ -1601,7 +1599,8 @@ function finishClassComponent(
nextChildren = instance.render();
}
if (request.status === ABORTING) {
throw AbortSigil;
// eslint-disable-next-line no-throw-literal
throw null;
}

if (__DEV__) {
Expand Down Expand Up @@ -1757,7 +1756,8 @@ function renderFunctionComponent(
legacyContext,
);
if (request.status === ABORTING) {
throw AbortSigil;
// eslint-disable-next-line no-throw-literal
throw null;
}

const hasId = checkDidRenderIdHook();
Expand Down Expand Up @@ -2076,7 +2076,8 @@ function renderLazyComponent(
Component = init(payload);
}
if (request.status === ABORTING) {
throw AbortSigil;
// eslint-disable-next-line no-throw-literal
throw null;
}
const resolvedProps = resolveDefaultPropsOnNonClassComponent(
Component,
Expand Down Expand Up @@ -2655,7 +2656,8 @@ function retryNode(request: Request, task: Task): void {
resolvedNode = init(payload);
}
if (request.status === ABORTING) {
throw AbortSigil;
// eslint-disable-next-line no-throw-literal
throw null;
}
// Now we render the resolved node
renderNodeDestructive(request, task, resolvedNode, childIndex);
Expand Down Expand Up @@ -4127,7 +4129,7 @@ function retryRenderTask(
// (unstable) API for suspending. This implementation detail can change
// later, once we deprecate the old API in favor of `use`.
getSuspendedThenable()
: thrownValue === AbortSigil
: request.status === ABORTING
? request.fatalError
: thrownValue;

Expand Down Expand Up @@ -4250,7 +4252,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void {
erroredReplay(
request,
task.blockedBoundary,
x === AbortSigil ? request.fatalError : x,
request.status === ABORTING ? request.fatalError : x,
errorInfo,
task.replay.nodes,
task.replay.slots,
Expand Down

0 comments on commit 7954db9

Please sign in to comment.