Skip to content

Commit

Permalink
[Flight] model halting as never delivered chunks (#30740)
Browse files Browse the repository at this point in the history 
stacked on: #30731

We've refined the model of halting a prerender. Now when you abort
during a prerender we simply omit the rows that would complete the
flight render. This is analagous to prerendering in Fizz where you must
resume the prerender to actually result in errors propagating in the
postponed holes. We don't have a resume yet for flight and it's not
entirely clear how that will work however the key insight here is that
deciding whether the never resolving rows are an error or not should
really be done on the consuming side rather than in the producer.

This PR also reintroduces the logs for the abort error/postpone when
prerendering which will give you some indication that something wasn't
finished when the prerender was aborted.
  • Loading branch information
@gnoff
gnoff committed Aug 20, 2024
1 parent 0fa9476 commit a960b92
Show file tree
Hide file tree
Showing 13 changed files with 253 additions and 283 deletions.
22 changes: 0 additions & 22 deletions packages/react-client/src/ReactFlightClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
enableRefAsProp,
enableFlightReadableStream,
enableOwnerStacks,
enableHalt,
} from 'shared/ReactFeatureFlags';

import {
Expand Down Expand Up @@ -1997,20 +1996,6 @@ function resolvePostponeDev(
}
}

function resolveBlocked(response: Response, id: number): void {
const chunks = response._chunks;
const chunk = chunks.get(id);
if (!chunk) {
chunks.set(id, createBlockedChunk(response));
} else if (chunk.status === PENDING) {
// This chunk as contructed via other means but it is actually a blocked chunk
// so we update it here. We check the status because it might have been aborted
// before we attempted to resolve it.
const blockedChunk: BlockedChunk<mixed> = (chunk: any);
blockedChunk.status = BLOCKED;
}
}

function resolveHint<Code: HintCode>(
response: Response,
code: Code,
Expand Down Expand Up @@ -2637,13 +2622,6 @@ function processFullStringRow(
}
}
// Fallthrough
case 35 /* "#" */: {
if (enableHalt) {
resolveBlocked(response, id);
return;
}
}
// Fallthrough
default: /* """ "{" "[" "t" "f" "n" "0" - "9" */ {
// We assume anything else is JSON.
resolveModel(response, id, row);
Expand Down
22 changes: 6 additions & 16 deletions packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ import type {Thenable} from 'shared/ReactTypes';

import {Readable} from 'stream';

import {enableHalt} from 'shared/ReactFeatureFlags';

import {
createRequest,
createPrerenderRequest,
startWork,
startFlowing,
stopFlowing,
abort,
halt,
} from 'react-server/src/ReactFlightServer';

import {
Expand Down Expand Up @@ -175,35 +173,27 @@ function prerenderToNodeStream(
resolve({prelude: readable});
}

const request = createRequest(
const request = createPrerenderRequest(
model,
moduleBasePath,
onAllReady,
onFatalError,
options ? options.onError : undefined,
options ? options.identifierPrefix : undefined,
options ? options.onPostpone : undefined,
options ? options.temporaryReferences : undefined,
__DEV__ && options ? options.environmentName : undefined,
__DEV__ && options ? options.filterStackFrame : undefined,
onAllReady,
onFatalError,
);
if (options && options.signal) {
const signal = options.signal;
if (signal.aborted) {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
} else {
const listener = () => {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
Expand Down
22 changes: 6 additions & 16 deletions packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ import type {Thenable} from 'shared/ReactTypes';
import type {ClientManifest} from './ReactFlightServerConfigTurbopackBundler';
import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig';

import {enableHalt} from 'shared/ReactFeatureFlags';

import {
createRequest,
createPrerenderRequest,
startWork,
startFlowing,
stopFlowing,
abort,
halt,
} from 'react-server/src/ReactFlightServer';

import {
Expand Down Expand Up @@ -134,35 +132,27 @@ function prerender(
);
resolve({prelude: stream});
}
const request = createRequest(
const request = createPrerenderRequest(
model,
turbopackMap,
onAllReady,
onFatalError,
options ? options.onError : undefined,
options ? options.identifierPrefix : undefined,
options ? options.onPostpone : undefined,
options ? options.temporaryReferences : undefined,
__DEV__ && options ? options.environmentName : undefined,
__DEV__ && options ? options.filterStackFrame : undefined,
onAllReady,
onFatalError,
);
if (options && options.signal) {
const signal = options.signal;
if (signal.aborted) {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
} else {
const listener = () => {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
Expand Down
22 changes: 6 additions & 16 deletions packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ import type {Thenable} from 'shared/ReactTypes';
import type {ClientManifest} from './ReactFlightServerConfigTurbopackBundler';
import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig';

import {enableHalt} from 'shared/ReactFeatureFlags';

import {
createRequest,
createPrerenderRequest,
startWork,
startFlowing,
stopFlowing,
abort,
halt,
} from 'react-server/src/ReactFlightServer';

import {
Expand Down Expand Up @@ -134,35 +132,27 @@ function prerender(
);
resolve({prelude: stream});
}
const request = createRequest(
const request = createPrerenderRequest(
model,
turbopackMap,
onAllReady,
onFatalError,
options ? options.onError : undefined,
options ? options.identifierPrefix : undefined,
options ? options.onPostpone : undefined,
options ? options.temporaryReferences : undefined,
__DEV__ && options ? options.environmentName : undefined,
__DEV__ && options ? options.filterStackFrame : undefined,
onAllReady,
onFatalError,
);
if (options && options.signal) {
const signal = options.signal;
if (signal.aborted) {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
} else {
const listener = () => {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
Expand Down
22 changes: 6 additions & 16 deletions packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ import type {Thenable} from 'shared/ReactTypes';

import {Readable} from 'stream';

import {enableHalt} from 'shared/ReactFeatureFlags';

import {
createRequest,
createPrerenderRequest,
startWork,
startFlowing,
stopFlowing,
abort,
halt,
} from 'react-server/src/ReactFlightServer';

import {
Expand Down Expand Up @@ -177,35 +175,27 @@ function prerenderToNodeStream(
resolve({prelude: readable});
}

const request = createRequest(
const request = createPrerenderRequest(
model,
turbopackMap,
onAllReady,
onFatalError,
options ? options.onError : undefined,
options ? options.identifierPrefix : undefined,
options ? options.onPostpone : undefined,
options ? options.temporaryReferences : undefined,
__DEV__ && options ? options.environmentName : undefined,
__DEV__ && options ? options.filterStackFrame : undefined,
onAllReady,
onFatalError,
);
if (options && options.signal) {
const signal = options.signal;
if (signal.aborted) {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
} else {
const listener = () => {
const reason = (signal: any).reason;
if (enableHalt) {
halt(request, reason);
} else {
abort(request, reason);
}
abort(request, reason);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
Expand Down
24 changes: 14 additions & 10 deletions packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2724,7 +2724,7 @@ describe('ReactFlightDOM', () => {
});

// @gate enableHalt
it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => {
it('does not propagate abort reasons errors when aborting a prerender', async () => {
let resolveGreeting;
const greetingPromise = new Promise(resolve => {
resolveGreeting = resolve;
Expand All @@ -2746,6 +2746,7 @@ describe('ReactFlightDOM', () => {
}

const controller = new AbortController();
const errors = [];
const {pendingResult} = await serverAct(async () => {
// destructure trick to avoid the act scope from awaiting the returned value
return {
Expand All @@ -2754,15 +2755,20 @@ describe('ReactFlightDOM', () => {
webpackMap,
{
signal: controller.signal,
onError(err) {
errors.push(err);
},
},
),
};
});

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

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

const preludeWeb = Readable.toWeb(prelude);
const response = ReactServerDOMClient.createFromReadableStream(preludeWeb);

Expand All @@ -2772,7 +2778,7 @@ describe('ReactFlightDOM', () => {
return use(response);
}

const errors = [];
errors.length = 0;
let abortFizz;
await serverAct(async () => {
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
Expand All @@ -2788,10 +2794,10 @@ describe('ReactFlightDOM', () => {
});

await serverAct(() => {
abortFizz('boom');
abortFizz('bam');
});

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

const container = document.createElement('div');
await readInto(container, fizzReadable);
Expand Down Expand Up @@ -2861,7 +2867,7 @@ describe('ReactFlightDOM', () => {
it('will halt unfinished chunks inside Suspense when aborting a prerender', async () => {
const controller = new AbortController();
function ComponentThatAborts() {
controller.abort();
controller.abort('boom');
return null;
}

Expand Down Expand Up @@ -2912,11 +2918,8 @@ describe('ReactFlightDOM', () => {
};
});

controller.abort();

const {prelude} = await pendingResult;
expect(errors).toEqual([]);

expect(errors).toEqual(['boom']);
const response = ReactServerDOMClient.createFromReadableStream(
Readable.toWeb(prelude),
);
Expand All @@ -2926,6 +2929,7 @@ describe('ReactFlightDOM', () => {
function ClientApp() {
return use(response);
}
errors.length = 0;
let abortFizz;
await serverAct(async () => {
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
Expand Down
Loading

0 comments on commit a960b92

Please sign in to comment.