Skip to content

Commit 5ad0221

Browse files
sebmarkbagegnoff
authored andcommitted
[Flight Reply] Reject any new Chunks not yet discovered at the time of reportGlobalError (facebook#31840)
We might have already resolved models that are not pending and so are not rejected by aborting the stream. When those later get parsed they might discover new chunks which end up as pending. These should be errored since they will never be able to resolve later. This avoids infinitely hanging the stream. This same fix needs to be ported to ReactFlightClient that has the same issue.
1 parent 1db6222 commit 5ad0221

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyEdge-test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,31 @@ describe('ReactFlightDOMReplyEdge', () => {
4545

4646
expect(decoded).toEqual({some: 'object'});
4747
});
48+
49+
it('should abort when parsing an incomplete payload', async () => {
50+
const infinitePromise = new Promise(() => {});
51+
const controller = new AbortController();
52+
const promiseForResult = ReactServerDOMClient.encodeReply(
53+
{promise: infinitePromise},
54+
{
55+
signal: controller.signal,
56+
},
57+
);
58+
controller.abort();
59+
const body = await promiseForResult;
60+
61+
const decoded = await ReactServerDOMServer.decodeReply(
62+
body,
63+
webpackServerMap,
64+
);
65+
66+
let error = null;
67+
try {
68+
await decoded.promise;
69+
} catch (x) {
70+
error = x;
71+
}
72+
expect(error).not.toBe(null);
73+
expect(error.message).toBe('Connection closed.');
74+
});
4875
});

packages/react-server/src/ReactFlightReplyServer.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ export type Response = {
135135
_formData: FormData,
136136
_chunks: Map<number, SomeChunk<any>>,
137137
_fromJSON: (key: string, value: JSONValue) => any,
138+
_closed: boolean,
139+
_closedReason: mixed,
138140
};
139141

140142
export function getRoot<T>(response: Response): Thenable<T> {
@@ -198,6 +200,14 @@ function createResolvedModelChunk<T>(
198200
return new Chunk(RESOLVED_MODEL, value, null, response);
199201
}
200202

203+
function createErroredChunk<T>(
204+
response: Response,
205+
reason: mixed,
206+
): ErroredChunk<T> {
207+
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
208+
return new Chunk(ERRORED, null, reason, response);
209+
}
210+
201211
function resolveModelChunk<T>(chunk: SomeChunk<T>, value: string): void {
202212
if (chunk.status !== PENDING) {
203213
// We already resolved. We didn't expect to see this.
@@ -297,6 +307,8 @@ function initializeModelChunk<T>(chunk: ResolvedModelChunk<T>): void {
297307
// Report that any missing chunks in the model is now going to throw this
298308
// error upon read. Also notify any pending promises.
299309
export function reportGlobalError(response: Response, error: Error): void {
310+
response._closed = true;
311+
response._closedReason = error;
300312
response._chunks.forEach(chunk => {
301313
// If this chunk was already resolved or errored, it won't
302314
// trigger an error but if it wasn't then we need to
@@ -318,6 +330,10 @@ function getChunk(response: Response, id: number): SomeChunk<any> {
318330
if (backingEntry != null) {
319331
// We assume that this is a string entry for now.
320332
chunk = createResolvedModelChunk(response, (backingEntry: any));
333+
} else if (response._closed) {
334+
// We have already errored the response and we're not going to get
335+
// anything more streaming in so this will immediately error.
336+
chunk = createErroredChunk(response, response._closedReason);
321337
} else {
322338
// We're still waiting on this entry to stream in.
323339
chunk = createPendingChunk(response);
@@ -519,6 +535,8 @@ export function createResponse(
519535
}
520536
return value;
521537
},
538+
_closed: false,
539+
_closedReason: null,
522540
};
523541
return response;
524542
}

0 commit comments

Comments
 (0)