From c4b433f8cb31d6f73d4a800fcf11ed55c8689daf Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 6 Jun 2024 14:41:27 -0700 Subject: [PATCH] [Flight] Allow aborting during render (#29764) Stacked on #29491 Previously if you aborted during a render the currently rendering task would itself be aborted which will cause the entire model to be replaced by the aborted error rather than just the slot currently being rendered. This change updates the abort logic to mark currently rendering tasks as aborted but allowing the current render to emit a partially serialized model with an error reference in place of the current model. The intent is to support aborting from rendering synchronously, in microtasks (after an await or in a .then) and in lazy initializers. We don't specifically support aborting from things like proxies that might be triggered during serialization of props --- .../src/__tests__/ReactFlightDOM-test.js | 587 +++++++++++++++++- .../react-server/src/ReactFlightServer.js | 114 +++- scripts/error-codes/codes.json | 3 +- 3 files changed, 675 insertions(+), 29 deletions(-) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 1ead6efe4b..3bf8e02e0f 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -36,6 +36,7 @@ let ErrorBoundary; let JSDOM; let ReactServerScheduler; let reactServerAct; +let assertConsoleErrorDev; describe('ReactFlightDOM', () => { beforeEach(() => { @@ -70,6 +71,8 @@ describe('ReactFlightDOM', () => { __unmockReact(); jest.resetModules(); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; Stream = require('stream'); React = require('react'); use = React.use; @@ -107,6 +110,38 @@ describe('ReactFlightDOM', () => { return maybePromise; } + async function readInto( + container: Document | HTMLElement, + stream: ReadableStream, + ) { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let content = ''; + while (true) { + const {done, value} = await reader.read(); + if (done) { + content += decoder.decode(); + break; + } + content += decoder.decode(value, {stream: true}); + } + if (container.nodeType === 9 /* DOCUMENT */) { + const doc = new JSDOM(content).window.document; + container.documentElement.innerHTML = doc.documentElement.innerHTML; + while (container.documentElement.attributes.length > 0) { + container.documentElement.removeAttribute( + container.documentElement.attributes[0].name, + ); + } + const attrs = doc.documentElement.attributes; + for (let i = 0; i < attrs.length; i++) { + container.documentElement.setAttribute(attrs[i].name, attrs[i].value); + } + } else { + container.innerHTML = content; + } + } + function getTestStream() { const writable = new Stream.PassThrough(); const readable = new ReadableStream({ @@ -1633,20 +1668,8 @@ describe('ReactFlightDOM', () => { ReactDOMFizzServer.renderToPipeableStream().pipe(fizzWritable); }); - const decoder = new TextDecoder(); - const reader = fizzReadable.getReader(); - let content = ''; - while (true) { - const {done, value} = await reader.read(); - if (done) { - content += decoder.decode(); - break; - } - content += decoder.decode(value, {stream: true}); - } - - const doc = new JSDOM(content).window.document; - expect(getMeaningfulChildren(doc)).toEqual( + await readInto(document, fizzReadable); + expect(getMeaningfulChildren(document)).toEqual( @@ -1912,4 +1935,540 @@ describe('ReactFlightDOM', () => { }); expect(container.innerHTML).toBe('Hello World'); }); + + it('can abort synchronously during render', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + + + + loading 2...

}> + + +
+ loading 3...

}> +
+ +
+ +
+
+ ); + } + + const abortRef = {current: null}; + function ComponentThatAborts() { + abortRef.current(); + return

hello world

; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in an async tick', async () => { + async function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + + + + loading 2...

}> + + +
+ loading 3...

}> +
+ +
+ +
+
+ ); + } + + const abortRef = {current: null}; + async function ComponentThatAborts() { + await 1; + abortRef.current(); + return

hello world

; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in a lazy initializer for a component', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + + + loading 2...

}> + + +
+ loading 3...

}> +
+ +
+ +
+
+ ); + } + + const abortRef = {current: null}; + const LazyAbort = React.lazy(() => { + abortRef.current(); + return { + then(cb) { + cb({default: 'div'}); + }, + }; + }); + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in a lazy initializer for an element', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}>{lazyAbort} + loading 2...

}> + + +
+ loading 3...

}> +
+ +
+ +
+
+ ); + } + + const abortRef = {current: null}; + const lazyAbort = React.lazy(() => { + abortRef.current(); + return { + then(cb) { + cb({default: 'hello world'}); + }, + }; + }); + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during a synchronous thenable resolution', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}>{thenable} + loading 2...

}> + + +
+ loading 3...

}> +
+ +
+ +
+
+ ); + } + + const abortRef = {current: null}; + const thenable = { + then(cb) { + abortRef.current(); + cb(thenable.value); + }, + }; + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('wont serialize thenables that were not already settled by the time an abort happens', async () => { + function App() { + return ( +
+ loading 1...

}> + + + loading 2...

}>{thenable1} +
+ loading 3...

}>{thenable2} +
+
+ ); + } + + const abortRef = {current: null}; + const thenable1 = { + then(cb) { + cb('hello world'); + }, + }; + + const thenable2 = { + then(cb) { + cb('hello world'); + }, + status: 'fulfilled', + value: 'hello world', + }; + + function ComponentThatAborts() { + abortRef.current(); + return thenable1; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
hello world
+
, + ); + }); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 2622b4e15c..11b558c592 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -381,10 +381,11 @@ const PENDING = 0; const COMPLETED = 1; const ABORTED = 3; const ERRORED = 4; +const RENDERING = 5; type Task = { id: number, - status: 0 | 1 | 3 | 4, + status: 0 | 1 | 3 | 4 | 5, model: ReactClientValue, ping: () => void, toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, @@ -396,7 +397,7 @@ type Task = { interface Reference {} export type Request = { - status: 0 | 1 | 2, + status: 0 | 1 | 2 | 3, flushScheduled: boolean, fatalError: mixed, destination: null | Destination, @@ -427,6 +428,8 @@ export type Request = { didWarnForKey: null | WeakSet, }; +const AbortSigil = {}; + const { TaintRegistryObjects, TaintRegistryValues, @@ -466,8 +469,9 @@ function defaultPostponeHandler(reason: string) { } const OPEN = 0; -const CLOSING = 1; -const CLOSED = 2; +const ABORTING = 1; +const CLOSING = 2; +const CLOSED = 3; export function createRequest( model: ReactClientValue, @@ -556,7 +560,6 @@ function serializeThenable( task.implicitSlot, request.abortableTasks, ); - if (__DEV__) { // If this came from Flight, forward any debug info into this new row. const debugInfo: ?ReactDebugInfo = (thenable: any)._debugInfo; @@ -590,6 +593,15 @@ function serializeThenable( return newTask.id; } default: { + if (request.status === ABORTING) { + // We can no longer accept any resolved values + newTask.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, newTask.id, model); + request.abortableTasks.delete(newTask); + return newTask.id; + } if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by @@ -1046,6 +1058,14 @@ function renderFunctionComponent( const secondArg = undefined; result = Component(props, secondArg); } + + if (request.status === ABORTING) { + // If we aborted during rendering we should interrupt the render but + // we don't need to provide an error because the renderer will encode + // the abort error as the reason. + throw AbortSigil; + } + if ( typeof result === 'object' && result !== null && @@ -1523,6 +1543,12 @@ function renderElement( const init = type._init; wrappedType = init(payload); } + if (request.status === ABORTING) { + // lazy initializers are user code and could abort during render + // we don't wan to return any value resolved from the lazy initializer + // if it aborts so we interrupt rendering here + throw AbortSigil; + } return renderElement( request, task, @@ -1942,6 +1968,15 @@ function renderModel( try { return renderModelDestructive(request, task, parent, key, value); } catch (thrownValue) { + // If the suspended/errored value was an element or lazy it can be reduced + // to a lazy reference, so that it doesn't error the parent. + const model = task.model; + const wasReactNode = + typeof model === 'object' && + model !== null && + ((model: any).$$typeof === REACT_ELEMENT_TYPE || + (model: any).$$typeof === REACT_LAZY_TYPE); + const x = thrownValue === SuspenseException ? // This is a special type of exception used for Suspense. For historical @@ -1951,17 +1986,18 @@ function renderModel( // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() : thrownValue; - // If the suspended/errored value was an element or lazy it can be reduced - // to a lazy reference, so that it doesn't error the parent. - const model = task.model; - const wasReactNode = - typeof model === 'object' && - model !== null && - ((model: any).$$typeof === REACT_ELEMENT_TYPE || - (model: any).$$typeof === REACT_LAZY_TYPE); + if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { + if (request.status === ABORTING) { + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + if (wasReactNode) { + return serializeLazyID(errorId); + } + return serializeByValueID(errorId); + } // Something suspended, we'll need to create a new task and resolve it later. const newTask = createTask( request, @@ -2004,6 +2040,15 @@ function renderModel( } } + if (thrownValue === AbortSigil) { + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + if (wasReactNode) { + return serializeLazyID(errorId); + } + return serializeByValueID(errorId); + } + // Restore the context. We assume that this will be restored by the inner // functions in case nothing throws so we don't use "finally" here. task.keyPath = prevKeyPath; @@ -2147,6 +2192,12 @@ function renderModelDestructive( const init = lazy._init; resolvedModel = init(payload); } + if (request.status === ABORTING) { + // lazy initializers are user code and could abort during render + // we don't wan to return any value resolved from the lazy initializer + // if it aborts so we interrupt rendering here + throw AbortSigil; + } if (__DEV__) { const debugInfo: ?ReactDebugInfo = lazy._debugInfo; if (debugInfo) { @@ -3262,6 +3313,7 @@ function retryTask(request: Request, task: Task): void { } const prevDebugID = debugID; + task.status = RENDERING; try { // Track the root so we know that we have to emit this object even though it @@ -3328,10 +3380,19 @@ function retryTask(request: Request, task: Task): void { if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { + if (request.status === ABORTING) { + request.abortableTasks.delete(task); + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + return; + } // Something suspended again, let's pick it back up later. + task.status = PENDING; + task.thenableState = getThenableStateAfterSuspending(); const ping = task.ping; x.then(ping, ping); - task.thenableState = getThenableStateAfterSuspending(); return; } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { request.abortableTasks.delete(task); @@ -3342,6 +3403,16 @@ function retryTask(request: Request, task: Task): void { return; } } + + if (x === AbortSigil) { + request.abortableTasks.delete(task); + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + return; + } + request.abortableTasks.delete(task); task.status = ERRORED; const digest = logRecoverableError(request, x); @@ -3399,6 +3470,10 @@ function performWork(request: Request): void { } function abortTask(task: Task, request: Request, errorId: number): void { + if (task.status === RENDERING) { + // This task will be aborted by the render + return; + } task.status = ABORTED; // Instead of emitting an error per task.id, we emit a model that only // has a single value referencing the error. @@ -3484,6 +3559,7 @@ function flushCompletedChunks( if (enableTaint) { cleanupTaintQueue(request); } + request.status = CLOSED; close(destination); request.destination = null; } @@ -3547,12 +3623,14 @@ export function stopFlowing(request: Request): void { // This is called to early terminate a request. It creates an error at all pending tasks. export function abort(request: Request, reason: mixed): void { try { + request.status = ABORTING; const abortableTasks = request.abortableTasks; // We have tasks to abort. We'll emit one error row and then emit a reference // to that row from every row that's still remaining. if (abortableTasks.size > 0) { request.pendingChunks++; const errorId = request.nextChunkId++; + request.fatalError = errorId; if ( enablePostpone && typeof reason === 'object' && @@ -3568,6 +3646,10 @@ export function abort(request: Request, reason: mixed): void { ? 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; const digest = logRecoverableError(request, error); emitErrorChunk(request, errorId, digest, error); @@ -3594,6 +3676,10 @@ export function abort(request: Request, reason: mixed): void { ? 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; } abortListeners.forEach(callback => callback(error)); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index b157b6eaef..ef4ae75a6d 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -514,5 +514,6 @@ "526": "Could not reference an opaque temporary reference. This is likely due to misconfiguring the temporaryReferences options on the server.", "527": "Incompatible React versions: The \"react\" and \"react-dom\" packages must have the exact same version. Instead got:\n - react: %s\n - react-dom: %s\nLearn more: https://react.dev/warnings/version-mismatch", "528": "Expected not to update to be updated to a stylesheet with precedence. Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s", - "529": "Expected stylesheet with precedence to not be updated to a different kind of . Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s" + "529": "Expected stylesheet with precedence to not be updated to a different kind of . Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s", + "530": "The render was aborted by the server with a promise." }