Skip to content

Commit

Permalink
[Flight] don't emit chunks for rejected thenables after abort (#31169)
Browse files Browse the repository at this point in the history
When aborting we emit chunks for each pending task. However there was a
bug where a thenable could also reject before we could flush and we end
up with an extra chunk throwing off the pendingChunks bookeeping. When a
task is retried we skip it if is is not in PENDING status because we
understand it was completed some other way. We need to replciate this
for the reject pathway on serialized thenables since aborting if
effectively completing all pending tasks and not something we need to
continue to do once the thenable rejects later.
  • Loading branch information
gnoff authored Oct 10, 2024
1 parent 566b0b0 commit 38af456
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3084,4 +3084,54 @@ describe('ReactFlightDOM', () => {
</div>,
);
});

it('rejecting a thenable after an abort before flush should not lead to a frozen readable', async () => {
const ClientComponent = clientExports(function (props: {
promise: Promise<void>,
}) {
return 'hello world';
});

let reject;
const promise = new Promise((_, re) => {
reject = re;
});

function App() {
return (
<div>
<Suspense fallback="loading...">
<ClientComponent promise={promise} />
</Suspense>
</div>
);
}

const errors = [];
const {writable, readable} = getTestStream();
const {pipe, abort} = await serverAct(() =>
ReactServerDOMServer.renderToPipeableStream(<App />, webpackMap, {
onError(x) {
errors.push(x);
},
}),
);
await serverAct(() => {
abort('STOP');
reject('STOP');
});
pipe(writable);

const reader = readable.getReader();
while (true) {
const {done} = await reader.read();
if (done) {
break;
}
}

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

// We expect it to get to the end here rather than hang on the reader.
});
});
35 changes: 20 additions & 15 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,22 +696,27 @@ function serializeThenable(
pingTask(request, newTask);
},
reason => {
if (
enablePostpone &&
typeof reason === 'object' &&
reason !== null &&
(reason: any).$$typeof === REACT_POSTPONE_TYPE
) {
const postponeInstance: Postpone = (reason: any);
logPostpone(request, postponeInstance.message, newTask);
emitPostponeChunk(request, newTask.id, postponeInstance);
} else {
const digest = logRecoverableError(request, reason, newTask);
emitErrorChunk(request, newTask.id, digest, reason);
if (newTask.status === PENDING) {
// We expect that the only status it might be otherwise is ABORTED.
// When we abort we emit chunks in each pending task slot and don't need
// to do so again here.
if (
enablePostpone &&
typeof reason === 'object' &&
reason !== null &&
(reason: any).$$typeof === REACT_POSTPONE_TYPE
) {
const postponeInstance: Postpone = (reason: any);
logPostpone(request, postponeInstance.message, newTask);
emitPostponeChunk(request, newTask.id, postponeInstance);
} else {
const digest = logRecoverableError(request, reason, newTask);
emitErrorChunk(request, newTask.id, digest, reason);
}
newTask.status = ERRORED;
request.abortableTasks.delete(newTask);
enqueueFlush(request);
}
newTask.status = ERRORED;
request.abortableTasks.delete(newTask);
enqueueFlush(request);
},
);

Expand Down

0 comments on commit 38af456

Please sign in to comment.