Skip to content

Commit f39ed9f

Browse files
authored
[Fizz] Continue reporting aborted task errors after a fatal abort (react#36575)
Stacked on react#36574 Normally, a fatal error transitions the request to CLOSED or CLOSING, which prevents later aborted root tasks from reporting their errors. Errors inside Suspense boundaries can still be reported, but other pending root tasks are hidden once the first one fatally errors. That behavior is useful for a normal fatal render error, where subsequent work does not need to be processed. During an abort, however, the abort reason is already the source of failure for every unfinished task. Treating the first root task visited during abort cleanup as the only observable fatal error privileges arbitrary task ordering and hides useful information about the unfinished render. Continue logging errors for pending tasks aborted after the request has already fatally errored, while still only failing the shell once.
1 parent f1af67e commit f39ed9f

3 files changed

Lines changed: 202 additions & 44 deletions

File tree

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3562,6 +3562,47 @@ describe('ReactDOMFizzServer', () => {
35623562
);
35633563
});
35643564

3565+
it('reports abort errors for every suspended task when aborting fatals the shell', async () => {
3566+
const promise = new Promise(() => {});
3567+
const rendered = [];
3568+
function Suspend({label}) {
3569+
rendered.push(label);
3570+
use(promise);
3571+
return null;
3572+
}
3573+
3574+
function App() {
3575+
return (
3576+
<>
3577+
<Suspense fallback="Loading...">
3578+
<Suspend label="boundary" />
3579+
</Suspense>
3580+
<Suspend label="root one" />
3581+
<Suspend label="root two" />
3582+
</>
3583+
);
3584+
}
3585+
3586+
const errors = [];
3587+
let abort;
3588+
await act(() => {
3589+
abort = renderToPipeableStream(<App />, {
3590+
onError(error) {
3591+
errors.push(error.message);
3592+
},
3593+
onShellError() {},
3594+
}).abort;
3595+
});
3596+
3597+
expect(rendered).toEqual(['boundary', 'root one', 'root two']);
3598+
3599+
await act(() => {
3600+
abort(new Error('abort reason'));
3601+
});
3602+
3603+
expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']);
3604+
});
3605+
35653606
it('warns in dev if you access digest from errorInfo in onRecoverableError', async () => {
35663607
await act(() => {
35673608
const {pipe} = renderToPipeableStream(
@@ -3803,8 +3844,8 @@ describe('ReactDOMFizzServer', () => {
38033844

38043845
expect(headers).toEqual({
38053846
Link: `
3806-
<non-responsive-preload>; rel=preload; as="image"; fetchpriority="high",
3807-
<non-responsive-img>; rel=preload; as="image"; fetchpriority="high"
3847+
<non-responsive-preload>; rel=preload; as="image"; fetchpriority="high",
3848+
<non-responsive-img>; rel=preload; as="image"; fetchpriority="high"
38083849
`
38093850
.replaceAll('\n', '')
38103851
.trim(),
@@ -7022,6 +7063,83 @@ describe('ReactDOMFizzServer', () => {
70227063
);
70237064
});
70247065

7066+
it('currently does not report an in-flight root task after another root task fatals while aborting', async () => {
7067+
const promise = new Promise(() => {});
7068+
function SuspendedRoot() {
7069+
use(promise);
7070+
return null;
7071+
}
7072+
7073+
function Child() {
7074+
return 'child';
7075+
}
7076+
7077+
const abortRef = {current: null};
7078+
function ComponentThatAborts() {
7079+
abortRef.current(new Error('abort reason'));
7080+
return <Child />;
7081+
}
7082+
7083+
const errors = [];
7084+
await act(() => {
7085+
const {abort} = renderToPipeableStream(
7086+
<>
7087+
<SuspendedRoot />
7088+
<ComponentThatAborts />
7089+
</>,
7090+
{
7091+
onError(error) {
7092+
errors.push(error.message);
7093+
},
7094+
onShellError() {},
7095+
},
7096+
);
7097+
abortRef.current = abort;
7098+
});
7099+
7100+
expect(errors).toEqual(['abort reason']);
7101+
});
7102+
7103+
it('currently does not report a root task that suspends after aborting during render', async () => {
7104+
const promise = new Promise(() => {});
7105+
function SuspendedRoot() {
7106+
use(promise);
7107+
return null;
7108+
}
7109+
7110+
function Child() {
7111+
use(promise);
7112+
return null;
7113+
}
7114+
7115+
const abortRef = {current: null};
7116+
function ComponentThatAborts() {
7117+
abortRef.current(new Error('abort reason'));
7118+
return <Child />;
7119+
}
7120+
7121+
const errors = [];
7122+
await act(() => {
7123+
const {abort} = renderToPipeableStream(
7124+
<>
7125+
<SuspendedRoot />
7126+
<ComponentThatAborts />
7127+
</>,
7128+
{
7129+
onError(error) {
7130+
errors.push(error.message);
7131+
},
7132+
onShellError() {},
7133+
},
7134+
);
7135+
abortRef.current = abort;
7136+
});
7137+
7138+
// TODO: Once abort completion is async, this still-suspended task should
7139+
// observe ABORTING and report the abort reason as well.
7140+
expect(errors).toEqual(['abort reason']);
7141+
});
7142+
70257143
it('can abort during render in a lazy initializer for a component', async () => {
70267144
function Sibling() {
70277145
return <p>sibling</p>;

packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,45 @@ describe('ReactDOMFizzServerNode', () => {
434434
expect(isCompleteCalls).toBe(0);
435435
});
436436

437+
it('should report abort errors for every suspended task but fail the shell only once', async () => {
438+
const promise = new Promise(() => {});
439+
const rendered = [];
440+
function Suspend({label}) {
441+
rendered.push(label);
442+
React.use(promise);
443+
return null;
444+
}
445+
446+
const errors = [];
447+
const shellErrors = [];
448+
const {abort} = ReactDOMFizzServer.renderToPipeableStream(
449+
<>
450+
<Suspense fallback="Loading...">
451+
<Suspend label="boundary" />
452+
</Suspense>
453+
<Suspend label="root one" />
454+
<Suspend label="root two" />
455+
</>,
456+
{
457+
onError(error) {
458+
errors.push(error.message);
459+
},
460+
onShellError(error) {
461+
shellErrors.push(error);
462+
},
463+
},
464+
);
465+
466+
await jest.runAllTimers();
467+
expect(rendered).toEqual(['boundary', 'root one', 'root two']);
468+
469+
const reason = new Error('abort reason');
470+
abort(reason);
471+
472+
expect(shellErrors).toEqual([reason]);
473+
expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']);
474+
});
475+
437476
it('should be able to complete by abort when the fallback is also suspended', async () => {
438477
let isCompleteCalls = 0;
439478
const errors = [];

packages/react-server/src/ReactFizzServer.js

Lines changed: 43 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4643,51 +4643,52 @@ function abortTask(task: Task, request: Request, error: mixed): void {
46434643
}
46444644

46454645
if (boundary === null) {
4646-
if (request.status !== CLOSING && request.status !== CLOSED) {
4647-
const replay: null | ReplaySet = task.replay;
4648-
if (replay === null) {
4649-
// We didn't complete the root so we have nothing to show. We can close
4650-
// the request;
4651-
if (request.trackedPostpones !== null && segment !== null) {
4652-
const trackedPostpones = request.trackedPostpones;
4653-
// We are aborting a prerender and must treat the shell as halted
4654-
// We log the error but we still resolve the prerender
4655-
logRecoverableError(request, error, errorInfo, task.debugTask);
4656-
trackPostpone(request, trackedPostpones, task, segment);
4657-
finishedTask(request, null, task.row, segment);
4658-
} else {
4659-
logRecoverableError(request, error, errorInfo, task.debugTask);
4660-
fatalError(request, error, errorInfo, task.debugTask);
4661-
}
4662-
return;
4646+
const replay: null | ReplaySet = task.replay;
4647+
if (replay === null) {
4648+
// We didn't complete the root so we have nothing to show. We can close
4649+
// the request;
4650+
if (request.trackedPostpones !== null && segment !== null) {
4651+
const trackedPostpones = request.trackedPostpones;
4652+
// We are aborting a prerender and must treat the shell as halted
4653+
// We log the error but we still resolve the prerender
4654+
logRecoverableError(request, error, errorInfo, task.debugTask);
4655+
trackPostpone(request, trackedPostpones, task, segment);
4656+
finishedTask(request, null, task.row, segment);
46634657
} else {
4664-
// If the shell aborts during a replay, that's not a fatal error. Instead
4665-
// we should be able to recover by client rendering all the root boundaries in
4666-
// the ReplaySet.
4667-
replay.pendingTasks--;
4668-
if (replay.pendingTasks === 0 && replay.nodes.length > 0) {
4669-
const errorDigest = logRecoverableError(
4670-
request,
4671-
error,
4672-
errorInfo,
4673-
null,
4674-
);
4675-
abortRemainingReplayNodes(
4676-
request,
4677-
null,
4678-
replay.nodes,
4679-
replay.slots,
4680-
error,
4681-
errorDigest,
4682-
errorInfo,
4683-
true,
4684-
);
4685-
}
4686-
request.pendingRootTasks--;
4687-
if (request.pendingRootTasks === 0) {
4688-
completeShell(request);
4658+
logRecoverableError(request, error, errorInfo, task.debugTask);
4659+
if (request.status !== CLOSING && request.status !== CLOSED) {
4660+
fatalError(request, error, errorInfo, task.debugTask);
46894661
}
46904662
}
4663+
return;
4664+
}
4665+
if (request.status !== CLOSING && request.status !== CLOSED) {
4666+
// If the shell aborts during a replay, that's not a fatal error. Instead
4667+
// we should be able to recover by client rendering all the root boundaries in
4668+
// the ReplaySet.
4669+
replay.pendingTasks--;
4670+
if (replay.pendingTasks === 0 && replay.nodes.length > 0) {
4671+
const errorDigest = logRecoverableError(
4672+
request,
4673+
error,
4674+
errorInfo,
4675+
null,
4676+
);
4677+
abortRemainingReplayNodes(
4678+
request,
4679+
null,
4680+
replay.nodes,
4681+
replay.slots,
4682+
error,
4683+
errorDigest,
4684+
errorInfo,
4685+
true,
4686+
);
4687+
}
4688+
request.pendingRootTasks--;
4689+
if (request.pendingRootTasks === 0) {
4690+
completeShell(request);
4691+
}
46914692
}
46924693
} else {
46934694
// We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which

0 commit comments

Comments
 (0)