diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index efb7e887cf18..bc2b086b643e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3562,6 +3562,47 @@ describe('ReactDOMFizzServer', () => { ); }); + it('reports abort errors for every suspended task when aborting fatals the shell', async () => { + const promise = new Promise(() => {}); + const rendered = []; + function Suspend({label}) { + rendered.push(label); + use(promise); + return null; + } + + function App() { + return ( + <> + + + + + + + ); + } + + const errors = []; + let abort; + await act(() => { + abort = renderToPipeableStream(, { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }).abort; + }); + + expect(rendered).toEqual(['boundary', 'root one', 'root two']); + + await act(() => { + abort(new Error('abort reason')); + }); + + expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']); + }); + it('warns in dev if you access digest from errorInfo in onRecoverableError', async () => { await act(() => { const {pipe} = renderToPipeableStream( @@ -3803,8 +3844,8 @@ describe('ReactDOMFizzServer', () => { expect(headers).toEqual({ Link: ` -; rel=preload; as="image"; fetchpriority="high", -; rel=preload; as="image"; fetchpriority="high" +; rel=preload; as="image"; fetchpriority="high", + ; rel=preload; as="image"; fetchpriority="high" ` .replaceAll('\n', '') .trim(), @@ -4003,10 +4044,46 @@ describe('ReactDOMFizzServer', () => { await act(() => pipe(testWritable)); expect(didRender).toBe(false); expect(didFatal).toBe(didFatal); - expect(errors).toEqual([ - 'boom', - 'The destination stream errored while writing data.', - ]); + expect(errors).toEqual(['boom']); + }); + + it('does not report aborts after fatally erroring', async () => { + const promise = new Promise(() => {}); + function AsyncComp() { + React.use(promise); + return 'Async'; + } + + function ErrorComp() { + throw new Error('boom'); + } + + const errors = []; + let abort; + await act(() => { + abort = renderToPipeableStream( +
+ + + + +
, + { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }, + ).abort; + }); + + expect(errors).toEqual(['boom']); + + await act(() => { + abort(new Error('too late')); + }); + + expect(errors).toEqual(['boom']); }); describe('error escaping', () => { @@ -6986,6 +7063,83 @@ describe('ReactDOMFizzServer', () => { ); }); + it('currently does not report an in-flight root task after another root task fatals while aborting', async () => { + const promise = new Promise(() => {}); + function SuspendedRoot() { + use(promise); + return null; + } + + function Child() { + return 'child'; + } + + const abortRef = {current: null}; + function ComponentThatAborts() { + abortRef.current(new Error('abort reason')); + return ; + } + + const errors = []; + await act(() => { + const {abort} = renderToPipeableStream( + <> + + + , + { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }, + ); + abortRef.current = abort; + }); + + expect(errors).toEqual(['abort reason']); + }); + + it('currently does not report a root task that suspends after aborting during render', async () => { + const promise = new Promise(() => {}); + function SuspendedRoot() { + use(promise); + return null; + } + + function Child() { + use(promise); + return null; + } + + const abortRef = {current: null}; + function ComponentThatAborts() { + abortRef.current(new Error('abort reason')); + return ; + } + + const errors = []; + await act(() => { + const {abort} = renderToPipeableStream( + <> + + + , + { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }, + ); + abortRef.current = abort; + }); + + // TODO: Once abort completion is async, this still-suspended task should + // observe ABORTING and report the abort reason as well. + expect(errors).toEqual(['abort reason']); + }); + it('can abort during render in a lazy initializer for a component', async () => { function Sibling() { return

sibling

; diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 98030d43386c..d4152e8efc00 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -237,6 +237,37 @@ describe('ReactDOMFizzServerNode', () => { expect(reportedShellErrors).toEqual([theError]); }); + it('should not report aborts after the shell has fatally errored', async () => { + const reportedErrors = []; + const reportedShellErrors = []; + const {abort} = ReactDOMFizzServer.renderToPipeableStream( +
+ + + + +
, + { + onError(x) { + reportedErrors.push(x); + }, + onShellError(x) { + reportedShellErrors.push(x); + }, + }, + ); + + await jest.runAllTimers(); + + expect(reportedErrors).toEqual([theError]); + expect(reportedShellErrors).toEqual([theError]); + + abort(new Error('too late')); + + expect(reportedErrors).toEqual([theError]); + expect(reportedShellErrors).toEqual([theError]); + }); + it('should error the stream when an error is thrown inside a fallback', async () => { const reportedErrors = []; const reportedShellErrors = []; @@ -263,10 +294,7 @@ describe('ReactDOMFizzServerNode', () => { expect(output.error).toBe(theError); expect(output.result).toBe(''); - expect(reportedErrors).toEqual([ - theError.message, - 'The destination stream errored while writing data.', - ]); + expect(reportedErrors).toEqual([theError.message]); expect(reportedShellErrors).toEqual([theError]); }); @@ -406,6 +434,45 @@ describe('ReactDOMFizzServerNode', () => { expect(isCompleteCalls).toBe(0); }); + it('should report abort errors for every suspended task but fail the shell only once', async () => { + const promise = new Promise(() => {}); + const rendered = []; + function Suspend({label}) { + rendered.push(label); + React.use(promise); + return null; + } + + const errors = []; + const shellErrors = []; + const {abort} = ReactDOMFizzServer.renderToPipeableStream( + <> + + + + + + , + { + onError(error) { + errors.push(error.message); + }, + onShellError(error) { + shellErrors.push(error); + }, + }, + ); + + await jest.runAllTimers(); + expect(rendered).toEqual(['boundary', 'root one', 'root two']); + + const reason = new Error('abort reason'); + abort(reason); + + expect(shellErrors).toEqual([reason]); + expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']); + }); + it('should be able to complete by abort when the fallback is also suspended', async () => { let isCompleteCalls = 0; const errors = []; diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 73b3e435d189..9695b39050fb 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -4643,51 +4643,52 @@ function abortTask(task: Task, request: Request, error: mixed): void { } if (boundary === null) { - if (request.status !== CLOSING && request.status !== CLOSED) { - const replay: null | ReplaySet = task.replay; - if (replay === null) { - // We didn't complete the root so we have nothing to show. We can close - // the request; - if (request.trackedPostpones !== null && segment !== null) { - const trackedPostpones = request.trackedPostpones; - // 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, task.debugTask); - trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, null, task.row, segment); - } else { - logRecoverableError(request, error, errorInfo, task.debugTask); - fatalError(request, error, errorInfo, task.debugTask); - } - return; + const replay: null | ReplaySet = task.replay; + if (replay === null) { + // We didn't complete the root so we have nothing to show. We can close + // the request; + if (request.trackedPostpones !== null && segment !== null) { + const trackedPostpones = request.trackedPostpones; + // 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, task.debugTask); + trackPostpone(request, trackedPostpones, task, segment); + finishedTask(request, null, task.row, segment); } else { - // If the shell aborts during a replay, that's not a fatal error. Instead - // we should be able to recover by client rendering all the root boundaries in - // the ReplaySet. - replay.pendingTasks--; - if (replay.pendingTasks === 0 && replay.nodes.length > 0) { - const errorDigest = logRecoverableError( - request, - error, - errorInfo, - null, - ); - abortRemainingReplayNodes( - request, - null, - replay.nodes, - replay.slots, - error, - errorDigest, - errorInfo, - true, - ); - } - request.pendingRootTasks--; - if (request.pendingRootTasks === 0) { - completeShell(request); + logRecoverableError(request, error, errorInfo, task.debugTask); + if (request.status !== CLOSING && request.status !== CLOSED) { + fatalError(request, error, errorInfo, task.debugTask); } } + return; + } + if (request.status !== CLOSING && request.status !== CLOSED) { + // If the shell aborts during a replay, that's not a fatal error. Instead + // we should be able to recover by client rendering all the root boundaries in + // the ReplaySet. + replay.pendingTasks--; + if (replay.pendingTasks === 0 && replay.nodes.length > 0) { + const errorDigest = logRecoverableError( + request, + error, + errorInfo, + null, + ); + abortRemainingReplayNodes( + request, + null, + replay.nodes, + replay.slots, + error, + errorDigest, + errorInfo, + true, + ); + } + request.pendingRootTasks--; + if (request.pendingRootTasks === 0) { + completeShell(request); + } } } else { // We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which @@ -6143,9 +6144,12 @@ export function stopFlowing(request: Request): void { // This is called to early terminate a request. It puts all pending boundaries in client rendered state. export function abort(request: Request, reason: mixed): void { - if (request.status === OPEN || request.status === OPENING) { - request.status = ABORTING; + if (request.status !== OPEN && request.status !== OPENING) { + // Only requests that are not already complete or in the process of aborting + // can be aborted. in practice this makes abort callable at most once per render. + return; } + request.status = ABORTING; try { const abortableTasks = request.abortableTasks;