diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0b56fe34710..bd28fa5443b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -7100,7 +7100,7 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual(['abort reason', 'abort reason']); }); - it('reports a root task that suspends after aborting during render', async () => { + it('reports a root task before rendering a suspended child returned after aborting', async () => { const promise = new Promise(() => {}); function SuspendedRoot() { use(promise); @@ -7138,6 +7138,34 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual(['abort reason', 'abort reason']); }); + it('currently does not report a root task that suspends directly after aborting during render', async () => { + const promise = new Promise(() => {}); + const abortRef = {current: null}; + function ComponentThatAbortsAndSuspends() { + abortRef.current(new Error('abort reason')); + use(promise); + return null; + } + + const errors = []; + await act(() => { + const {abort} = renderToPipeableStream( + , + { + onError(error) { + errors.push(error.message); + }, + onShellError() {}, + }, + ); + abortRef.current = abort; + }); + + // The task suspends before renderFunctionComponent gets to throw the + // abort reason, and retryRenderTask currently suspends it again. + expect(errors).toEqual([]); + }); + it('can abort during render in a lazy initializer for a component', async () => { function Sibling() { return

sibling

; @@ -8443,6 +8471,10 @@ describe('ReactDOMFizzServer', () => { expect(thrownError).toBe('boom'); expect(errors).toEqual([ + { + error: 'boom', + componentStack: componentStack(['Abort', 'body', 'html', 'App']), + }, { error: 'boom', componentStack: componentStack([ @@ -8462,10 +8494,6 @@ describe('ReactDOMFizzServer', () => { 'App', ]), }, - { - error: 'boom', - componentStack: componentStack(['Abort', 'body', 'html', 'App']), - }, ]); // We expect the render to throw before streaming anything so the default diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 27e74750c45..9bd3f641040 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -221,7 +221,9 @@ describe('ReactDOMFizzServerBrowser', () => { ), ); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const result = await readResult(stream); expect(result).toContain('Loading'); @@ -247,7 +249,9 @@ describe('ReactDOMFizzServerBrowser', () => { ); const theReason = new Error('aborted for reasons'); - controller.abort(theReason); + await serverAct(() => { + controller.abort(theReason); + }); let caughtError = null; try { @@ -364,7 +368,7 @@ describe('ReactDOMFizzServerBrowser', () => { const reader = stream.getReader(); await reader.read(); - await reader.cancel(); + await serverAct(() => reader.cancel()); expect(errors).toEqual([ 'The render was aborted by the server without a reason.', @@ -463,7 +467,9 @@ describe('ReactDOMFizzServerBrowser', () => { }), ); - controller.abort('foobar'); + await serverAct(() => { + controller.abort('foobar'); + }); expect(errors).toEqual(['foobar', 'foobar']); }); @@ -502,7 +508,9 @@ describe('ReactDOMFizzServerBrowser', () => { }), ); - controller.abort(new Error('uh oh')); + await serverAct(() => { + controller.abort(new Error('uh oh')); + }); expect(errors).toEqual(['uh oh', 'uh oh']); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index d4152e8efc0..ad9678a2b36 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -382,6 +382,7 @@ describe('ReactDOMFizzServerNode', () => { expect(isCompleteCalls).toBe(0); abort(new Error('uh oh')); + await jest.runAllTimers(); await completed; @@ -468,6 +469,7 @@ describe('ReactDOMFizzServerNode', () => { const reason = new Error('abort reason'); abort(reason); + await jest.runAllTimers(); expect(shellErrors).toEqual([reason]); expect(errors).toEqual(['abort reason', 'abort reason', 'abort reason']); @@ -504,6 +506,7 @@ describe('ReactDOMFizzServerNode', () => { expect(isCompleteCalls).toBe(0); abort(); + await jest.runAllTimers(); await completed; @@ -746,6 +749,7 @@ describe('ReactDOMFizzServerNode', () => { resolve(); await completed; + await jest.runAllTimers(); expect(errors).toEqual([ 'The destination stream errored while writing data.', diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 416b665bf54..b9c9b0fd25b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -299,7 +299,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const result = await resultPromise; @@ -329,7 +331,9 @@ describe('ReactDOMFizzStaticBrowser', () => { await jest.runAllTimers(); const theReason = new Error('aborted for reasons'); - controller.abort(theReason); + await serverAct(() => { + controller.abort(theReason); + }); let rejected = false; let prelude; @@ -447,7 +451,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort('foobar'); + await serverAct(() => { + controller.abort('foobar'); + }); await resultPromise; @@ -489,13 +495,63 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(new Error('uh oh')); + await serverAct(() => { + controller.abort(new Error('uh oh')); + }); await resultPromise; expect(errors).toEqual(['uh oh', 'uh oh']); }); + it('currently uses the abort reason when an abort listener synchronously rejects pending work', async () => { + let reject; + const rejectedPromise = new Promise((resolve, rejectPromise) => { + reject = rejectPromise; + }); + const haltedPromise = new Promise(() => {}); + function RejectedWait() { + React.use(rejectedPromise); + return null; + } + function HaltedWait() { + React.use(haltedPromise); + return null; + } + + const errors = []; + const controller = new AbortController(); + let resultPromise; + await serverAct(() => { + resultPromise = ReactDOMFizzStatic.prerender( + <> + + + + + + + , + { + signal: controller.signal, + onError(error) { + errors.push(error.message); + }, + }, + ); + }); + + controller.signal.addEventListener('abort', () => { + reject(new Error('rejected during abort')); + }); + await serverAct(() => { + controller.abort(new Error('abort reason')); + }); + await resultPromise; + + expect(errors).toEqual(['abort reason', 'abort reason']); + }); + it('logs an error if onHeaders throws but continues the prerender', async () => { const errors = []; function onError(error) { @@ -562,7 +618,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -587,7 +645,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); @@ -641,7 +701,9 @@ describe('ReactDOMFizzStaticBrowser', () => { onError() {}, }); }); - controller.abort('prerender abort'); + await serverAct(() => { + controller.abort('prerender abort'); + }); const prerendered = await pendingResult; prerendering = false; @@ -693,7 +755,9 @@ describe('ReactDOMFizzStaticBrowser', () => { onError() {}, }); }); - controller.abort('prerender abort'); + await serverAct(() => { + controller.abort('prerender abort'); + }); const prerendered = await pendingResult; prerendering = false; @@ -761,7 +825,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -792,7 +858,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); @@ -855,7 +923,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(new Error('boom')); + await serverAct(() => { + controller.abort(new Error('boom')); + }); const prerendered = await pendingResult; @@ -917,7 +987,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; @@ -995,7 +1067,9 @@ describe('ReactDOMFizzStaticBrowser', () => { }); }); - controller.abort(); + await serverAct(() => { + controller.abort(); + }); const prerendered = await pendingResult; const postponedState = JSON.stringify(prerendered.postponed); @@ -1021,7 +1095,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); }); - controller2.abort(); + await serverAct(() => { + controller2.abort(); + }); const prerendered2 = await pendingResult; const postponedState2 = JSON.stringify(prerendered2.postponed); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js index 83bc6cb5d30..56ffa30a048 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js @@ -216,6 +216,7 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort(); + await jest.runAllTimers(); const result = await resultPromise; @@ -244,6 +245,7 @@ describe('ReactDOMFizzStaticNode', () => { const theReason = new Error('aborted for reasons'); controller.abort(theReason); + await jest.runAllTimers(); let didThrow = false; let prelude; @@ -281,6 +283,7 @@ describe('ReactDOMFizzStaticNode', () => { }, ); + await jest.runAllTimers(); const {prelude} = await streamPromise; const content = await readContent(prelude); expect(errors).toEqual(['This operation was aborted']); @@ -309,6 +312,7 @@ describe('ReactDOMFizzStaticNode', () => { // Technically we could still continue rendering the shell but currently the // semantics mean that we also abort any pending CPU work. + await jest.runAllTimers(); let didThrow = false; let prelude; @@ -358,6 +362,7 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort('foobar'); + await jest.runAllTimers(); await resultPromise; @@ -399,9 +404,57 @@ describe('ReactDOMFizzStaticNode', () => { await jest.runAllTimers(); controller.abort(new Error('uh oh')); + await jest.runAllTimers(); await resultPromise; expect(errors).toEqual(['uh oh', 'uh oh']); }); + + it('currently uses the abort reason when an abort listener synchronously rejects pending work', async () => { + let reject; + const rejectedPromise = new Promise((resolve, rejectPromise) => { + reject = rejectPromise; + }); + const haltedPromise = new Promise(() => {}); + function RejectedWait() { + React.use(rejectedPromise); + return null; + } + function HaltedWait() { + React.use(haltedPromise); + return null; + } + + const errors = []; + const controller = new AbortController(); + const resultPromise = ReactDOMFizzStatic.prerenderToNodeStream( + <> + + + + + + + , + { + signal: controller.signal, + onError(error) { + errors.push(error.message); + }, + }, + ); + + await jest.runAllTimers(); + + controller.signal.addEventListener('abort', () => { + reject(new Error('rejected during abort')); + }); + controller.abort(new Error('abort reason')); + + await jest.runAllTimers(); + await resultPromise; + + expect(errors).toEqual(['abort reason', 'abort reason']); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index d7b0fc6c99d..27853fc2381 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -2051,7 +2051,7 @@ describe('ReactFlightDOMNode', () => { ); expect(result).toContain( - 'Switched to client rendering because the server rendering aborted due to:\n\n' + + 'Switched to client rendering because the server rendering aborted due to:\\n\\n' + 'ssr-abort', ); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 59146a21d92..0cfae20be25 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -356,6 +356,9 @@ type Segment = { textEmbedded: boolean, }; +// The ordering of these statuses matters. OPENING and OPEN are the only +// statuses in which newly scheduled work may be performed. Any status greater +// than OPEN represents a request that no longer admits work. const OPENING = 10; const OPEN = 11; const CLOSING = 12; @@ -4591,9 +4594,9 @@ function abortRemainingReplayNodes( } } -function abortTask(task: Task, request: Request, error: mixed): void { - // This aborts the task and aborts the parent that it blocks, putting it into - // client rendered mode. +function abortTask(task: Task, request: Request): void { + // Mark pending tasks as aborted synchronously so any work that was already + // scheduled cannot begin after abort is called. if (task === request.currentTask) { // This is a currently rendering Task. The render itself will abort the task. return; @@ -4604,17 +4607,13 @@ function abortTask(task: Task, request: Request, error: mixed): void { segment.status = ABORTED; } - const errorInfo = getThrownInfo(task.componentStack); if (__DEV__ && enableAsyncDebugInfo) { - // If the task is not rendering, then this is an async abort. Conceptually it's as if - // the abort happened inside the async gap. The abort reason's stack frame won't have that - // on the stack so instead we use the owner stack and debug task of any halted async debug info. + // Capture async debug information at the point abort begins. The task may + // receive more data before finishAbort runs and no longer suspend at the + // call site we need to report. let node: any = task.node; if (node !== null && typeof node === 'object') { - // Push a fake component stack frame that represents the await. let debugInfo = node._debugInfo; - // First resolve lazy nodes to find debug info that has been transferred - // to the inner value. while ( typeof node === 'object' && node !== null && @@ -4645,6 +4644,29 @@ function abortTask(task: Task, request: Request, error: mixed): void { } } + if (boundary !== null) { + boundary.fallbackAbortableTasks.forEach(fallbackTask => + abortTask(fallbackTask, request), + ); + } +} + +function finishAbortedTask(task: Task, request: Request, error: mixed): void { + // Report and complete a task that was synchronously claimed by abortTask. + // A currently rendering task remains responsible for unwinding itself. + if (task === request.currentTask) { + return; + } + const boundary = task.blockedBoundary; + const segment = task.blockedSegment; + if (segment !== null) { + if (segment.status !== ABORTED) { + return; + } + } + + const errorInfo = getThrownInfo(task.componentStack); + if (boundary === null) { const replay: null | ReplaySet = task.replay; if (replay === null) { @@ -4706,7 +4728,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // 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), + finishAbortedTask(fallbackTask, request, error), ); boundary.fallbackAbortableTasks.clear(); return finishedTask(request, boundary, task.row, segment); @@ -4743,7 +4765,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // 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), + finishAbortedTask(fallbackTask, request, error), ); boundary.fallbackAbortableTasks.clear(); } @@ -4761,14 +4783,39 @@ function abortTask(task: Task, request: Request, error: mixed): void { } } -function abortTaskDEV(task: Task, request: Request, error: mixed): void { +function finishAbortedTaskDEV( + task: Task, + request: Request, + error: mixed, +): void { if (__DEV__) { const prevTaskInDEV = currentTaskInDEV; const prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack; setCurrentTaskInDEV(task); ReactSharedInternals.getCurrentStack = getCurrentStackInDEV; try { - abortTask(task, request, error); + finishAbortedTask(task, request, error); + } finally { + setCurrentTaskInDEV(prevTaskInDEV); + ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl; + } + } else { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'finishAbortedTaskDEV should never be called in production mode. This is a bug in React.', + ); + } +} + +function abortTaskDEV(task: Task, request: Request): void { + if (__DEV__) { + const prevTaskInDEV = currentTaskInDEV; + const prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack; + setCurrentTaskInDEV(task); + ReactSharedInternals.getCurrentStack = getCurrentStackInDEV; + try { + abortTask(task, request); } finally { setCurrentTaskInDEV(prevTaskInDEV); ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl; @@ -5276,7 +5323,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { } export function performWork(request: Request): void { - if (request.status === CLOSED || request.status === CLOSING) { + if (request.aborted || request.status > OPEN) { return; } const prevContext = getActiveContext(); @@ -6156,36 +6203,16 @@ export function stopFlowing(request: Request): void { request.destination = null; } -// 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.aborted || - (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.aborted = true; - +function finishAbort(request: Request, abortableTasks: Set): void { try { - const abortableTasks = request.abortableTasks; if (abortableTasks.size > 0) { - const error = - reason === undefined - ? new Error('The render was aborted by the server without a reason.') - : typeof reason === 'object' && - reason !== null && - typeof reason.then === 'function' - ? new Error('The render was aborted by the server with a promise.') - : reason; - // This error isn't necessarily fatal in this case but we need to stash it - // so we can use it to abort any pending work - request.fatalError = error; + const error = request.fatalError; if (__DEV__) { - abortableTasks.forEach(task => abortTaskDEV(task, request, error)); + abortableTasks.forEach(task => + finishAbortedTaskDEV(task, request, error), + ); } else { - abortableTasks.forEach(task => abortTask(task, request, error)); + abortableTasks.forEach(task => finishAbortedTask(task, request, error)); } abortableTasks.clear(); } @@ -6199,6 +6226,40 @@ export function abort(request: Request, reason: mixed): 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.aborted || + (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.aborted = true; + const error = + reason === undefined + ? new Error('The render was aborted by the server without a reason.') + : typeof reason === 'object' && + reason !== null && + typeof reason.then === 'function' + ? new Error('The render was aborted by the server with a promise.') + : reason; + // This error isn't necessarily fatal in this case but we need to stash it + // so we can use it to abort any pending work. + request.fatalError = error; + const abortableTasks = request.abortableTasks; + if (__DEV__) { + abortableTasks.forEach(task => abortTaskDEV(task, request)); + } else { + abortableTasks.forEach(task => abortTask(task, request)); + } + // Even though this looks async some renderers schedule work sync + // So it is important that the finish step does not assume the stack + // has unwinded yet + scheduleWork(() => finishAbort(request, abortableTasks)); +} + export function flushResources(request: Request): void { enqueueFlush(request); }