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

When we introduces 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 31631d6 commit 007cea5
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 5 deletions.
101 changes: 101 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7667,6 +7667,7 @@ describe('ReactDOMFizzServer', () => {
});

// @gate enablePostpone
// @gate !enableHalt
it('does not call onError when you abort with a postpone instance during prerender', async () => {
const promise = new Promise(r => {});

Expand Down Expand Up @@ -7746,6 +7747,106 @@ 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']);

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

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);
});

// @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);
});

// @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);
});
});
35 changes: 33 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,39 @@ describe('ReactDOMFizzStaticNode', () => {
expect(errors).toEqual(['This operation was aborted']);
});

// @gate experimental
it('should reject if aborting before the shell is complete', async () => {
// @gate !enableHalt
fit('should reject if aborting before the shell is complete', async () => {

Check failure on line 215 in packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Unexpected focused test

Check failure on line 215 in packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Use "it.only" instead
const errors = [];
const controller = new AbortController();
const promise = ReactDOMFizzStatic.prerenderToNodeStream(
<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 caughtError = null;
try {
await promise;
} catch (error) {
caughtError = error;
}
expect(caughtError).toBe(theReason);
expect(errors).toEqual(['aborted for reasons']);
});

// @gate enableHalt
fit('should resolve an empty shell if aborting before the shell is complete', async () => {

Check failure on line 246 in packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Unexpected focused test

Check failure on line 246 in packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Use "it.only" instead
const errors = [];
const controller = new AbortController();
const promise = ReactDOMFizzStatic.prerenderToNodeStream(
Expand Down
67 changes: 64 additions & 3 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
enableSuspenseAvoidThisFallbackFizz,
enableCache,
enablePostpone,
enableHalt,
enableRenderableContext,
enableRefAsProp,
disableDefaultPropsExceptForClasses,
Expand Down Expand Up @@ -3625,6 +3626,9 @@ function erroredTask(
) {
// Report the error to a global handler.
let errorDigest;
// We don't handle halts here because we only halt when prerendering and
// when prerendering we should be finishing tasks not erroring them when
// they halt or postpone
if (
enablePostpone &&
typeof error === 'object' &&
Expand Down Expand Up @@ -3812,6 +3816,12 @@ function abortTask(task: Task, request: Request, error: mixed): void {
logRecoverableError(request, fatal, errorInfo, null);
fatalError(request, fatal, errorInfo, null);
}
} else if (enableHalt && request.trackedPostpones !== null) {
// We are aborting a prerender and must treat the shell as halted
// We log the error but we still resolve the prerender
logRecoverableError(request, error, errorInfo, null);
trackPostpone(request, request.trackedPostpones, task, segment);
finishedTask(request, null, segment);
} else {
logRecoverableError(request, error, errorInfo, null);
fatalError(request, error, errorInfo, null);
Expand Down Expand Up @@ -3856,10 +3866,40 @@ function abortTask(task: Task, request: Request, error: mixed): void {
}
} else {
boundary.pendingTasks--;
// We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which
// boundary the message is referring to
const errorInfo = getThrownInfo(task.componentStack);
const trackedPostpones = request.trackedPostpones;
if (boundary.status !== CLIENT_RENDERED) {
// We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which
// boundary the message is referring to
const errorInfo = getThrownInfo(task.componentStack);
if (enableHalt) {
if (trackedPostpones !== null) {
// We are aborting a prerender
if (
enablePostpone &&
typeof error === 'object' &&
error !== null &&
error.$$typeof === REACT_POSTPONE_TYPE
) {
const postponeInstance: Postpone = (error: any);
logPostpone(request, postponeInstance.message, errorInfo, null);
} else {
// We are aborting a prerender and must halt this boundary.
// We treat this like other postpones during prerendering
logRecoverableError(request, error, errorInfo, null);
}
trackPostpone(request, trackedPostpones, task, segment);
// If this boundary was still pending then we haven't already cancelled its fallbacks.
// We'll need to abort the fallbacks, which will also error that parent boundary.
boundary.fallbackAbortableTasks.forEach(fallbackTask =>
abortTask(fallbackTask, request, error),
);
boundary.fallbackAbortableTasks.clear();
return finishedTask(request, boundary, segment);
}
}
boundary.status = CLIENT_RENDERED;
// We are aborting a render or resume which should put boundaries
// into an explicitly client rendered state
let errorDigest;
if (
enablePostpone &&
Expand Down Expand Up @@ -4178,6 +4218,27 @@ function retryRenderTask(
finishedTask(request, task.blockedBoundary, segment);
return;
}
} else if (
enableHalt &&
request.status === ABORTING &&
request.trackedPostpones !== null
) {
// We are aborting a prerender and need to halt this task.
// We log the error but encode a POSTPONE. Since we are prerendering
// and we have postpone semantics we also need to finish the task
// rather than erroring it.
const trackedPostpones = request.trackedPostpones;
task.abortSet.delete(task);

logRecoverableError(
request,
x,
errorInfo,
__DEV__ && enableOwnerStacks ? task.debugTask : null,
);
trackPostpone(request, trackedPostpones, task, segment);
finishedTask(request, task.blockedBoundary, segment);
return;
}

const errorInfo = getThrownInfo(task.componentStack);
Expand Down

0 comments on commit 007cea5

Please sign in to comment.