Skip to content

Commit

Permalink
[flight] When halting call onError/onPostpone
Browse files Browse the repository at this point in the history 
Halt was originally implemented as an alternative to error handling and thus halted reasons were not exposed through any observability event like onError or onPostpone. We could add something like onAbort or onHalt in it's place but it's not clear that this is particularly well motivated. Instead in this change we update halt semantics to still call onError and onPostpone with the abort reason. So a halt doesn't change what you can observe but it does change the serialization model. So while you will see errors through onError they won't propagate to the consumer as errors.
  • Loading branch information
@gnoff
gnoff committed Aug 19, 2024
1 parent 6a12b85 commit 8e9c567
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 248 deletions.
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('serializes a forever blocked reference 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
9 changes: 7 additions & 2 deletions packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2390,7 +2390,7 @@ describe('ReactFlightDOMBrowser', () => {
});

// @gate enableHalt
it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => {
it('serializes a forever blocked reference when aborting a prerender', async () => {
let resolveGreeting;
const greetingPromise = new Promise(resolve => {
resolveGreeting = resolve;
Expand All @@ -2412,6 +2412,7 @@ describe('ReactFlightDOMBrowser', () => {
}

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 @@ -2420,14 +2421,18 @@ describe('ReactFlightDOMBrowser', () => {
webpackMap,
{
signal: controller.signal,
onError(err) {
errors.push(err);
},
},
),
};
});

controller.abort();
controller.abort('boom');
resolveGreeting();
const {prelude} = await pendingResult;
expect(errors).toEqual(['boom']);

function ClientRoot({response}) {
return use(response);
Expand Down
Loading

0 comments on commit 8e9c567

Please sign in to comment.