[Flight Reply] Reject any new Chunks not yet discovered at the time of reportGlobalError (#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.
Sebastian Markbåge committed
Dec 18, 2024 at 15:56 UTC
ef979d4703ffcd1f379a4b76195816bea3da3c81
2 files changed
+45
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyEdge-test.js
+27
@@ -245,4 +245,31 @@ describe('ReactFlightDOMReplyEdge', () => {
245
),
246
);
247
});
248
+
249
+ it('should abort when parsing an incomplete payload', async () => {
250
+ const infinitePromise = new Promise(() => {});
251
+ const controller = new AbortController();
252
+ const promiseForResult = ReactServerDOMClient.encodeReply(
253
+ {promise: infinitePromise},
254
+ {
255
+ signal: controller.signal,
256
+ },
257
+ );
258
+ controller.abort();
259
+ const body = await promiseForResult;
260
+
261
+ const decoded = await ReactServerDOMServer.decodeReply(
262
+ body,
263
+ webpackServerMap,
264
+ );
265
+
266
+ let error = null;
267
+ try {
268
+ await decoded.promise;
269
+ } catch (x) {
270
+ error = x;
271
+ }
272
+ expect(error).not.toBe(null);
273
+ expect(error.message).toBe('Connection closed.');
274
+ });
275
});
packages/react-server/src/ReactFlightReplyServer.js
+18
@@ -169,6 +169,8 @@ export type Response = {
169
_prefix: string,
170
_formData: FormData,
171
_chunks: Map<number, SomeChunk<any>>,
172
+ _closed: boolean,
173
+ _closedReason: mixed,
174
_temporaryReferences: void | TemporaryReferenceSet,
175
};
176
@@ -255,6 +257,14 @@ function createResolvedModelChunk<T>(
257
return new Chunk(RESOLVED_MODEL, value, id, response);
258
}
259
260
+function createErroredChunk<T>(
261
+ response: Response,
262
+ reason: mixed,
263
+): ErroredChunk<T> {
264
+ // $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
265
+ return new Chunk(ERRORED, null, reason, response);
266
+}
267
+
268
function resolveModelChunk<T>(
269
chunk: SomeChunk<T>,
270
value: string,
@@ -493,6 +503,8 @@ function initializeModelChunk<T>(chunk: ResolvedModelChunk<T>): void {
503
// Report that any missing chunks in the model is now going to throw this
504
// error upon read. Also notify any pending promises.
505
export function reportGlobalError(response: Response, error: Error): void {
506
+ response._closed = true;
507
+ response._closedReason = error;
508
response._chunks.forEach(chunk => {
509
// If this chunk was already resolved or errored, it won't
510
// trigger an error but if it wasn't then we need to
@@ -514,6 +526,10 @@ function getChunk(response: Response, id: number): SomeChunk<any> {
526
if (backingEntry != null) {
527
// We assume that this is a string entry for now.
528
chunk = createResolvedModelChunk(response, (backingEntry: any), id);
529
+ } else if (response._closed) {
530
+ // We have already errored the response and we're not going to get
531
+ // anything more streaming in so this will immediately error.
532
+ chunk = createErroredChunk(response, response._closedReason);
533
} else {
534
// We're still waiting on this entry to stream in.
535
chunk = createPendingChunk(response);
@@ -1082,6 +1098,8 @@ export function createResponse(
1098
_prefix: formFieldPrefix,
1099
_formData: backingFormData,
1100
_chunks: chunks,
1101
+ _closed: false,
1102
+ _closedReason: null,
1103
_temporaryReferences: temporaryReferences,
1104
};
1105
return response;