Skip to content

Commit

Permalink
[Flight] Reject any new Chunks not yet discovered at the time of repo…
Browse files Browse the repository at this point in the history
…rtGlobalError (facebook#31851)

Same as facebook#31840 but for the Flight Client.
  • Loading branch information
sebmarkbage authored Dec 19, 2024
1 parent 17520b6 commit a9bbe34
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 57 deletions.
16 changes: 14 additions & 2 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ export type Response = {
_rowTag: number, // 0 indicates that we're currently parsing the row ID
_rowLength: number, // remaining bytes in the row. 0 indicates that we're looking for a newline.
_buffer: Array<Uint8Array>, // chunks received so far as part of this row
_closed: boolean,
_closedReason: mixed,
_tempRefs: void | TemporaryReferenceSet, // the set temporary references can be resolved from
_timeOrigin: number, // Profiling-only
_debugRootOwner?: null | ReactComponentInfo, // DEV-only
Expand Down Expand Up @@ -358,7 +360,7 @@ function createBlockedChunk<T>(response: Response): BlockedChunk<T> {

function createErrorChunk<T>(
response: Response,
error: Error | Postpone,
error: mixed,
): ErroredChunk<T> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(ERRORED, null, error, response);
Expand Down Expand Up @@ -639,6 +641,8 @@ function initializeModuleChunk<T>(chunk: ResolvedModuleChunk<T>): void {
// Report that any missing chunks in the model is now going to throw this
// error upon read. Also notify any pending promises.
export function reportGlobalError(response: Response, error: Error): void {
response._closed = true;
response._closedReason = error;
response._chunks.forEach(chunk => {
// If this chunk was already resolved or errored, it won't
// trigger an error but if it wasn't then we need to
Expand Down Expand Up @@ -913,7 +917,13 @@ function getChunk(response: Response, id: number): SomeChunk<any> {
const chunks = response._chunks;
let chunk = chunks.get(id);
if (!chunk) {
chunk = createPendingChunk(response);
if (response._closed) {
// We have already errored the response and we're not going to get
// anything more streaming in so this will immediately error.
chunk = createErrorChunk(response, response._closedReason);
} else {
chunk = createPendingChunk(response);
}
chunks.set(id, chunk);
}
return chunk;
Expand Down Expand Up @@ -1640,6 +1650,8 @@ function ResponseInstance(
this._rowTag = 0;
this._rowLength = 0;
this._buffer = [];
this._closed = false;
this._closedReason = null;
this._tempRefs = temporaryReferences;
if (enableProfilerTimer && enableComponentPerformanceTrack) {
this._timeOrigin = 0;
Expand Down
3 changes: 1 addition & 2 deletions packages/react-noop-renderer/src/ReactNoopFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Source = Array<Uint8Array>;

const decoderOptions = {stream: true};

const {createResponse, processBinaryChunk, getRoot, close} = ReactFlightClient({
const {createResponse, processBinaryChunk, getRoot} = ReactFlightClient({
createStringDecoder() {
return new TextDecoder();
},
Expand Down Expand Up @@ -74,7 +74,6 @@ function read<T>(source: Source, options: ReadOptions): Thenable<T> {
for (let i = 0; i < source.length; i++) {
processBinaryChunk(response, source[i], 0);
}
close(response);
return getRoot(response);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2902,16 +2902,7 @@ describe('ReactFlightDOM', () => {
abortFizz('bam');
});

if (__DEV__) {
expect(errors).toEqual([new Error('Connection closed.')]);
} else {
// This is likely a bug. In Dev we get a connection closed error
// because the debug info creates a chunk that has a pending status
// and when the stream finishes we error if any chunks are still pending.
// In production there is no debug info so the missing chunk is never instantiated
// because nothing triggers model evaluation before the stream completes
expect(errors).toEqual(['bam']);
}
expect(errors).toEqual([new Error('Connection closed.')]);

const container = document.createElement('div');
await readInto(container, fizzReadable);
Expand Down Expand Up @@ -3066,17 +3057,8 @@ describe('ReactFlightDOM', () => {
});

// one error per boundary
if (__DEV__) {
const err = new Error('Connection closed.');
expect(errors).toEqual([err, err, err]);
} else {
// This is likely a bug. In Dev we get a connection closed error
// because the debug info creates a chunk that has a pending status
// and when the stream finishes we error if any chunks are still pending.
// In production there is no debug info so the missing chunk is never instantiated
// because nothing triggers model evaluation before the stream completes
expect(errors).toEqual(['boom', 'boom', 'boom']);
}
const err = new Error('Connection closed.');
expect(errors).toEqual([err, err, err]);

const container = document.createElement('div');
await readInto(container, fizzReadable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2574,17 +2574,7 @@ describe('ReactFlightDOMBrowser', () => {
root.render(<ClientRoot response={response} />);
});

if (__DEV__) {
expect(errors).toEqual([new Error('Connection closed.')]);
expect(container.innerHTML).toBe('');
} else {
// This is likely a bug. In Dev we get a connection closed error
// because the debug info creates a chunk that has a pending status
// and when the stream finishes we error if any chunks are still pending.
// In production there is no debug info so the missing chunk is never instantiated
// because nothing triggers model evaluation before the stream completes
expect(errors).toEqual([]);
expect(container.innerHTML).toBe('<div>loading...</div>');
}
expect(errors).toEqual([new Error('Connection closed.')]);
expect(container.innerHTML).toBe('');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1245,20 +1245,59 @@ describe('ReactFlightDOMEdge', () => {
),
);
fizzController.abort('bam');
if (__DEV__) {
expect(errors).toEqual([new Error('Connection closed.')]);
} else {
// This is likely a bug. In Dev we get a connection closed error
// because the debug info creates a chunk that has a pending status
// and when the stream finishes we error if any chunks are still pending.
// In production there is no debug info so the missing chunk is never instantiated
// because nothing triggers model evaluation before the stream completes
expect(errors).toEqual(['bam']);
}
expect(errors).toEqual([new Error('Connection closed.')]);
// Should still match the result when parsed
const result = await readResult(ssrStream);
const div = document.createElement('div');
div.innerHTML = result;
expect(div.textContent).toBe('loading...');
});

// @gate enableHalt
it('should abort parsing an incomplete prerender payload', async () => {
const infinitePromise = new Promise(() => {});
const controller = new AbortController();
const errors = [];
const {pendingResult} = await serverAct(async () => {
// destructure trick to avoid the act scope from awaiting the returned value
return {
pendingResult: ReactServerDOMStaticServer.unstable_prerender(
{promise: infinitePromise},
webpackMap,
{
signal: controller.signal,
onError(err) {
errors.push(err);
},
},
),
};
});

controller.abort();
const {prelude} = await pendingResult;

expect(errors).toEqual([]);

const response = ReactServerDOMClient.createFromReadableStream(prelude, {
serverConsumerManifest: {
moduleMap: {},
moduleLoading: {},
},
});

// Wait for the stream to finish and therefore abort before we try to .then the response.
await 0;

const result = await response;

let error = null;
try {
await result.promise;
} catch (x) {
error = x;
}
expect(error).not.toBe(null);
expect(error.message).toBe('Connection closed.');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -508,16 +508,7 @@ describe('ReactFlightDOMNode', () => {
),
);
ssrStream.abort('bam');
if (__DEV__) {
expect(errors).toEqual([new Error('Connection closed.')]);
} else {
// This is likely a bug. In Dev we get a connection closed error
// because the debug info creates a chunk that has a pending status
// and when the stream finishes we error if any chunks are still pending.
// In production there is no debug info so the missing chunk is never instantiated
// because nothing triggers model evaluation before the stream completes
expect(errors).toEqual(['bam']);
}
expect(errors).toEqual([new Error('Connection closed.')]);
// Should still match the result when parsed
const result = await readResult(ssrStream);
const div = document.createElement('div');
Expand Down

0 comments on commit a9bbe34

Please sign in to comment.