Skip to content

Commit

Permalink
[Fizz][Static] when aborting a prerender halt unfinished boundaries i…
Browse files Browse the repository at this point in the history
…nstead of erroring (#30732)

When we introduced prerendering for flight we modeled an abort of a
flight prerender as having unfinished rows. This is similar to how
postpone was already implemented when you postponed from "within" a
prerender using React.unstable_postpone. However when aborting with a
postponed instance every boundary would be eagerly marked for client
rendering which is more akin to prerendering and then resuming with an
aborted signal.

The insight with the flight work was that it's not so much the postpone
that describes the intended semantics but the abort combined with a
prerender. So like in flight when you abort a prerender and enableHalt
is enabled boundaries and the shell won't error for any reason. Fizz
will still call onPostpone and onError according to the abort reason but
the consuemr of the prerender should expect to resume it before trying
to use it.
  • Loading branch information
gnoff committed Aug 20, 2024
1 parent 4c2dfb3 commit 85180b8
Show file tree
Hide file tree
Showing 5 changed files with 424 additions and 21 deletions.
106 changes: 106 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7746,6 +7746,112 @@ describe('ReactDOMFizzServer', () => {
);
});

// @gate enableHalt
it('can resume a prerender that was aborted', async () => {
const promise = new Promise(r => {});

let prerendering = true;

function Wait() {
if (prerendering) {
return React.use(promise);
} else {
return 'Hello';
}
}

function App() {
return (
<div>
<Suspense fallback="Loading...">
<p>
<span>
<Suspense fallback="Loading again...">
<Wait />
</Suspense>
</span>
</p>
<p>
<span>
<Suspense fallback="Loading again too...">
<Wait />
</Suspense>
</span>
</p>
</Suspense>
</div>
);
}

const controller = new AbortController();
const signal = controller.signal;

const errors = [];
function onError(error) {
errors.push(error);
}
let pendingPrerender;
await act(() => {
pendingPrerender = ReactDOMFizzStatic.prerenderToNodeStream(<App />, {
signal,
onError,
});
});
controller.abort('boom');

const prerendered = await pendingPrerender;

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

const preludeWritable = new Stream.PassThrough();
preludeWritable.setEncoding('utf8');
preludeWritable.on('data', chunk => {
writable.write(chunk);
});

await act(() => {
prerendered.prelude.pipe(preludeWritable);
});

expect(getVisibleChildren(container)).toEqual(
<div>
<p>
<span>Loading again...</span>
</p>
<p>
<span>Loading again too...</span>
</p>
</div>,
);

prerendering = false;

errors.length = 0;
const resumed = await ReactDOMFizzServer.resumeToPipeableStream(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
{
onError,
},
);

await act(() => {
resumed.pipe(writable);
});

expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>
<span>Hello</span>
</p>
<p>
<span>Hello</span>
</p>
</div>,
);
});

// @gate enablePostpone
it('does not call onError when you abort with a postpone instance during resume', async () => {
let prerendering = true;
Expand Down
52 changes: 52 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,4 +454,56 @@ describe('ReactDOMFizzStatic', () => {
});
expect(getVisibleChildren(container)).toEqual(undefined);
});

// @gate enableHalt
it('will halt a prerender when aborting with an error during a render', async () => {
const controller = new AbortController();
function App() {
controller.abort('sync');
return <div>hello world</div>;
}

const errors = [];
const result = await ReactDOMFizzStatic.prerenderToNodeStream(<App />, {
signal: controller.signal,
onError(error) {
errors.push(error);
},
});
await act(async () => {
result.prelude.pipe(writable);
});
expect(errors).toEqual(['sync']);
expect(getVisibleChildren(container)).toEqual(undefined);
});

// @gate enableHalt
it('will halt a prerender when aborting with an error in a microtask', async () => {
const errors = [];

const controller = new AbortController();
function App() {
React.use(
new Promise(() => {
Promise.resolve().then(() => {
controller.abort('async');
});
}),
);
return <div>hello world</div>;
}

errors.length = 0;
const result = await ReactDOMFizzStatic.prerenderToNodeStream(<App />, {
signal: controller.signal,
onError(error) {
errors.push(error);
},
});
await act(async () => {
result.prelude.pipe(writable);
});
expect(errors).toEqual(['async']);
expect(getVisibleChildren(container)).toEqual(undefined);
});
});
101 changes: 92 additions & 9 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ describe('ReactDOMFizzStaticBrowser', () => {
});

// @gate experimental
it('should reject if aborting before the shell is complete', async () => {
// @gate !enableHalt
it('should reject if aborting before the shell is complete and enableHalt is disabled', async () => {
const errors = [];
const controller = new AbortController();
const promise = serverAct(() =>
Expand Down Expand Up @@ -339,6 +340,42 @@ describe('ReactDOMFizzStaticBrowser', () => {
expect(errors).toEqual(['aborted for reasons']);
});

// @gate enableHalt
it('should resolve an empty prelude if aborting before the shell is complete', async () => {
const errors = [];
const controller = new AbortController();
const promise = serverAct(() =>
ReactDOMFizzStatic.prerender(
<div>
<InfiniteSuspend />
</div>,
{
signal: controller.signal,
onError(x) {
errors.push(x.message);
},
},
),
);

await jest.runAllTimers();

const theReason = new Error('aborted for reasons');
controller.abort(theReason);

let rejected = false;
let prelude;
try {
({prelude} = await promise);
} catch (error) {
rejected = true;
}
expect(rejected).toBe(false);
expect(errors).toEqual(['aborted for reasons']);
const content = await readContent(prelude);
expect(content).toBe('');
});

// @gate experimental
it('should be able to abort before something suspends', async () => {
const errors = [];
Expand All @@ -365,18 +402,26 @@ describe('ReactDOMFizzStaticBrowser', () => {
),
);

let caughtError = null;
try {
await streamPromise;
} catch (error) {
caughtError = error;
if (gate(flags => flags.enableHalt)) {
const {prelude} = await streamPromise;
const content = await readContent(prelude);
expect(errors).toEqual(['The operation was aborted.']);
expect(content).toBe('');
} else {
let caughtError = null;
try {
await streamPromise;
} catch (error) {
caughtError = error;
}
expect(caughtError.message).toBe('The operation was aborted.');
expect(errors).toEqual(['The operation was aborted.']);
}
expect(caughtError.message).toBe('The operation was aborted.');
expect(errors).toEqual(['The operation was aborted.']);
});

// @gate experimental
it('should reject if passing an already aborted signal', async () => {
// @gate !enableHalt
it('should reject if passing an already aborted signal and enableHalt is disabled', async () => {
const errors = [];
const controller = new AbortController();
const theReason = new Error('aborted for reasons');
Expand Down Expand Up @@ -410,6 +455,44 @@ describe('ReactDOMFizzStaticBrowser', () => {
expect(errors).toEqual(['aborted for reasons']);
});

// @gate enableHalt
it('should resolve an empty prelude if passing an already aborted signal', async () => {
const errors = [];
const controller = new AbortController();
const theReason = new Error('aborted for reasons');
controller.abort(theReason);

const promise = serverAct(() =>
ReactDOMFizzStatic.prerender(
<div>
<Suspense fallback={<div>Loading</div>}>
<InfiniteSuspend />
</Suspense>
</div>,
{
signal: controller.signal,
onError(x) {
errors.push(x.message);
},
},
),
);

// Technically we could still continue rendering the shell but currently the
// semantics mean that we also abort any pending CPU work.
let didThrow = false;
let prelude;
try {
({prelude} = await promise);
} catch (error) {
didThrow = true;
}
expect(didThrow).toBe(false);
expect(errors).toEqual(['aborted for reasons']);
const content = await readContent(prelude);
expect(content).toBe('');
});

// @gate experimental
it('supports custom abort reasons with a string', async () => {
const promise = new Promise(r => {});
Expand Down
Loading

0 comments on commit 85180b8

Please sign in to comment.