From 56389e81ffe17c97f3137f18cc6446a4b2c956de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 19 Jun 2022 11:05:41 -0400 Subject: [PATCH] Abort Flight (#24754) Add aborting to the Flight Server. This encodes the reason as an "error" row that gets thrown client side. These are still exposed in prod which is a follow up we'll still have to do to encode them as digests instead. The error is encoded once and then referenced by each row that needs to be updated. --- .../ReactFlightDOMRelayServerHostConfig.js | 8 ++ .../src/ReactFlightDOMServerBrowser.js | 18 ++++- .../src/ReactFlightDOMServerNode.js | 5 ++ .../src/__tests__/ReactFlightDOM-test.js | 76 ++++++++++++++++--- .../__tests__/ReactFlightDOMBrowser-test.js | 74 +++++++++++++++++- .../ReactFlightNativeRelayServerHostConfig.js | 8 ++ .../react-server/src/ReactFlightServer.js | 67 +++++++++++++++- .../src/ReactFlightServerConfigStream.js | 10 +++ 8 files changed, 250 insertions(+), 16 deletions(-) diff --git a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js index 8fcd823204cb..0ec5b2cf0dc4 100644 --- a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js +++ b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js @@ -117,6 +117,14 @@ export function processModelChunk( return ['J', id, json]; } +export function processReferenceChunk( + request: Request, + id: number, + reference: string, +): Chunk { + return ['J', id, reference]; +} + export function processModuleChunk( request: Request, id: number, diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js index bfd47cc40a69..93d047e4a043 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js @@ -15,12 +15,14 @@ import { createRequest, startWork, startFlowing, + abort, } from 'react-server/src/ReactFlightServer'; type Options = { - onError?: (error: mixed) => void, - context?: Array<[string, ServerContextJSONValue]>, identifierPrefix?: string, + signal?: AbortSignal, + context?: Array<[string, ServerContextJSONValue]>, + onError?: (error: mixed) => void, }; function renderToReadableStream( @@ -35,6 +37,18 @@ function renderToReadableStream( options ? options.context : undefined, options ? options.identifierPrefix : undefined, ); + if (options && options.signal) { + const signal = options.signal; + if (signal.aborted) { + abort(request, (signal: any).reason); + } else { + const listener = () => { + abort(request, (signal: any).reason); + signal.removeEventListener('abort', listener); + }; + signal.addEventListener('abort', listener); + } + } const stream = new ReadableStream( { type: 'bytes', diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js index 6bb32203baff..69fa772a304a 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js @@ -16,6 +16,7 @@ import { createRequest, startWork, startFlowing, + abort, } from 'react-server/src/ReactFlightServer'; function createDrainHandler(destination, request) { @@ -29,6 +30,7 @@ type Options = { }; type PipeableStream = {| + abort(reason: mixed): void, pipe(destination: T): T, |}; @@ -58,6 +60,9 @@ function renderToPipeableStream( destination.on('drain', createDrainHandler(destination, request)); return destination; }, + abort(reason: mixed) { + abort(request, reason); + }, }; } 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 4ca3f903e779..b68a9b28284b 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -30,6 +30,7 @@ let React; let ReactDOMClient; let ReactServerDOMWriter; let ReactServerDOMReader; +let Suspense; describe('ReactFlightDOM', () => { beforeEach(() => { @@ -42,6 +43,7 @@ describe('ReactFlightDOM', () => { ReactDOMClient = require('react-dom/client'); ReactServerDOMWriter = require('react-server-dom-webpack/writer.node.server'); ReactServerDOMReader = require('react-server-dom-webpack'); + Suspense = React.Suspense; }); function getTestStream() { @@ -92,6 +94,11 @@ describe('ReactFlightDOM', () => { } } + const theInfinitePromise = new Promise(() => {}); + function InfiniteSuspend() { + throw theInfinitePromise; + } + it('should resolve HTML using Node streams', async () => { function Text({children}) { return {children}; @@ -133,8 +140,6 @@ describe('ReactFlightDOM', () => { }); it('should resolve the root', async () => { - const {Suspense} = React; - // Model function Text({children}) { return {children}; @@ -184,8 +189,6 @@ describe('ReactFlightDOM', () => { }); it('should not get confused by $', async () => { - const {Suspense} = React; - // Model function RootModel() { return {text: '$1'}; @@ -220,8 +223,6 @@ describe('ReactFlightDOM', () => { }); it('should not get confused by @', async () => { - const {Suspense} = React; - // Model function RootModel() { return {text: '@div'}; @@ -257,7 +258,6 @@ describe('ReactFlightDOM', () => { it('should progressively reveal server components', async () => { let reportedErrors = []; - const {Suspense} = React; // Client Components @@ -460,8 +460,6 @@ describe('ReactFlightDOM', () => { }); it('should preserve state of client components on refetch', async () => { - const {Suspense} = React; - // Client function Page({response}) { @@ -545,4 +543,64 @@ describe('ReactFlightDOM', () => { expect(inputB.tagName).toBe('INPUT'); expect(inputB.value).toBe('goodbye'); }); + + it('should be able to complete after aborting and throw the reason client-side', async () => { + const reportedErrors = []; + + class ErrorBoundary extends React.Component { + state = {hasError: false, error: null}; + static getDerivedStateFromError(error) { + return { + hasError: true, + error, + }; + } + render() { + if (this.state.hasError) { + return this.props.fallback(this.state.error); + } + return this.props.children; + } + } + + const {writable, readable} = getTestStream(); + const {pipe, abort} = ReactServerDOMWriter.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x); + }, + }, + ); + pipe(writable); + const response = ReactServerDOMReader.createFromReadableStream(readable); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + function App({res}) { + return res.readRoot(); + } + + await act(async () => { + root.render( +

{e.message}

}> + (loading)

}> + + + , + ); + }); + expect(container.innerHTML).toBe('

(loading)

'); + + await act(async () => { + abort('for reasons'); + }); + expect(container.innerHTML).toBe('

Error: for reasons

'); + + expect(reportedErrors).toEqual(['for reasons']); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 3337b19d11fc..e2cc4989c8ee 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -27,6 +27,7 @@ let ReactDOMClient; let ReactDOMServer; let ReactServerDOMWriter; let ReactServerDOMReader; +let Suspense; describe('ReactFlightDOMBrowser', () => { beforeEach(() => { @@ -39,6 +40,7 @@ describe('ReactFlightDOMBrowser', () => { ReactDOMServer = require('react-dom/server.browser'); ReactServerDOMWriter = require('react-server-dom-webpack/writer.browser.server'); ReactServerDOMReader = require('react-server-dom-webpack'); + Suspense = React.Suspense; }); function moduleReference(moduleExport) { @@ -108,6 +110,11 @@ describe('ReactFlightDOMBrowser', () => { return [DelayedText, _resolve, _reject]; } + const theInfinitePromise = new Promise(() => {}); + function InfiniteSuspend() { + throw theInfinitePromise; + } + it('should resolve HTML using W3C streams', async () => { function Text({children}) { return {children}; @@ -180,7 +187,6 @@ describe('ReactFlightDOMBrowser', () => { it('should progressively reveal server components', async () => { let reportedErrors = []; - const {Suspense} = React; // Client Components @@ -356,8 +362,6 @@ describe('ReactFlightDOMBrowser', () => { }); it('should close the stream upon completion when rendering to W3C streams', async () => { - const {Suspense} = React; - // Model function Text({children}) { return children; @@ -512,4 +516,68 @@ describe('ReactFlightDOMBrowser', () => { const result = await readResult(ssrStream); expect(result).toEqual('Client Component'); }); + + it('should be able to complete after aborting and throw the reason client-side', async () => { + const reportedErrors = []; + + class ErrorBoundary extends React.Component { + state = {hasError: false, error: null}; + static getDerivedStateFromError(error) { + return { + hasError: true, + error, + }; + } + render() { + if (this.state.hasError) { + return this.props.fallback(this.state.error); + } + return this.props.children; + } + } + + const controller = new AbortController(); + const stream = ReactServerDOMWriter.renderToReadableStream( +
+ +
, + webpackMap, + { + signal: controller.signal, + onError(x) { + reportedErrors.push(x); + }, + }, + ); + const response = ReactServerDOMReader.createFromReadableStream(stream); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + function App({res}) { + return res.readRoot(); + } + + await act(async () => { + root.render( +

{e.message}

}> + (loading)

}> + + + , + ); + }); + expect(container.innerHTML).toBe('

(loading)

'); + + await act(async () => { + // @TODO this is a hack to work around lack of support for abortSignal.reason in node + // The abort call itself should set this property but since we are testing in node we + // set it here manually + controller.signal.reason = 'for reasons'; + controller.abort('for reasons'); + }); + expect(container.innerHTML).toBe('

Error: for reasons

'); + + expect(reportedErrors).toEqual(['for reasons']); + }); }); diff --git a/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js b/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js index c139769fe453..5c9360701ab8 100644 --- a/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js +++ b/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js @@ -114,6 +114,14 @@ export function processModelChunk( return ['J', id, json]; } +export function processReferenceChunk( + request: Request, + id: number, + reference: string, +): Chunk { + return ['J', id, reference]; +} + export function processModuleChunk( request: Request, id: number, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 4fc18de3a1dd..814f8f1a4f9d 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -34,6 +34,7 @@ import { processProviderChunk, processSymbolChunk, processErrorChunk, + processReferenceChunk, resolveModuleMetaData, getModuleKey, isModuleReference, @@ -86,8 +87,14 @@ export type ReactModel = type ReactModelObject = {+[key: string]: ReactModel}; +const PENDING = 0; +const COMPLETED = 1; +const ABORTED = 3; +const ERRORED = 4; + type Task = { id: number, + status: 0 | 1 | 3 | 4, model: ReactModel, ping: () => void, context: ContextSnapshot, @@ -101,6 +108,7 @@ export type Request = { cache: Map, nextChunkId: number, pendingChunks: number, + abortableTasks: Set, pingedTasks: Array, completedModuleChunks: Array, completedJSONChunks: Array, @@ -132,6 +140,7 @@ export function createRequest( context?: Array<[string, ServerContextJSONValue]>, identifierPrefix?: string, ): Request { + const abortSet: Set = new Set(); const pingedTasks = []; const request = { status: OPEN, @@ -141,6 +150,7 @@ export function createRequest( cache: new Map(), nextChunkId: 0, pendingChunks: 0, + abortableTasks: abortSet, pingedTasks: pingedTasks, completedModuleChunks: [], completedJSONChunks: [], @@ -157,7 +167,7 @@ export function createRequest( }; request.pendingChunks++; const rootContext = createRootContext(context); - const rootTask = createTask(request, model, rootContext); + const rootTask = createTask(request, model, rootContext, abortSet); pingedTasks.push(rootTask); return request; } @@ -263,14 +273,17 @@ function createTask( request: Request, model: ReactModel, context: ContextSnapshot, + abortSet: Set, ): Task { const id = request.nextChunkId++; const task = { id, + status: PENDING, model, context, ping: () => pingTask(request, task), }; + abortSet.add(task); return task; } @@ -520,7 +533,12 @@ export function resolveModelToJSON( if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended, we'll need to create a new task and resolve it later. request.pendingChunks++; - const newTask = createTask(request, value, getActiveContext()); + const newTask = createTask( + request, + value, + getActiveContext(), + request.abortableTasks, + ); const ping = newTask.ping; x.then(ping, ping); return serializeByRefID(newTask.id); @@ -791,6 +809,10 @@ function emitProviderChunk( } function retryTask(request: Request, task: Task): void { + if (task.status !== PENDING) { + // We completed this by other means before we had a chance to retry it. + return; + } switchContext(task.context); try { let value = task.model; @@ -814,6 +836,8 @@ function retryTask(request: Request, task: Task): void { } const processedChunk = processModelChunk(request, task.id, value); request.completedJSONChunks.push(processedChunk); + request.abortableTasks.delete(task); + task.status = COMPLETED; } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended again, let's pick it back up later. @@ -821,6 +845,8 @@ function retryTask(request: Request, task: Task): void { x.then(ping, ping); return; } else { + request.abortableTasks.delete(task); + task.status = ERRORED; logRecoverableError(request, x); // This errored, we need to serialize this error to the emitErrorChunk(request, task.id, x); @@ -855,6 +881,15 @@ function performWork(request: Request): void { } } +function abortTask(task: Task, request: Request, errorId: number): void { + task.status = ABORTED; + // Instead of emitting an error per task.id, we emit a model that only + // has a single value referencing the error. + const ref = serializeByValueID(errorId); + const processedChunk = processReferenceChunk(request, task.id, ref); + request.completedJSONChunks.push(processedChunk); +} + function flushCompletedChunks( request: Request, destination: Destination, @@ -942,6 +977,34 @@ export function startFlowing(request: Request, destination: Destination): 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 { + const abortableTasks = request.abortableTasks; + if (abortableTasks.size > 0) { + // 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. + const error = + reason === undefined + ? new Error('The render was aborted by the server without a reason.') + : reason; + + logRecoverableError(request, error); + request.pendingChunks++; + const errorId = request.nextChunkId++; + emitErrorChunk(request, errorId, error); + abortableTasks.forEach(task => abortTask(task, request, errorId)); + abortableTasks.clear(); + } + if (request.destination !== null) { + flushCompletedChunks(request, request.destination); + } + } catch (error) { + logRecoverableError(request, error); + fatalError(request, error); + } +} + function importServerContexts( contexts?: Array<[string, ServerContextJSONValue]>, ) { diff --git a/packages/react-server/src/ReactFlightServerConfigStream.js b/packages/react-server/src/ReactFlightServerConfigStream.js index 08e9cbff2f50..b9772ef6f2ea 100644 --- a/packages/react-server/src/ReactFlightServerConfigStream.js +++ b/packages/react-server/src/ReactFlightServerConfigStream.js @@ -99,6 +99,16 @@ export function processModelChunk( return stringToChunk(row); } +export function processReferenceChunk( + request: Request, + id: number, + reference: string, +): Chunk { + const json = stringify(reference); + const row = serializeRowHeader('J', id) + json + '\n'; + return stringToChunk(row); +} + export function processModuleChunk( request: Request, id: number,