From 326832a56d41b4462919f9efe69916712ca87a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 30 Sep 2024 12:45:13 -0700 Subject: [PATCH] [Flight] Serialize Error Values (#31104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The idea is that the RSC protocol is a superset of Structured Clone. #25687 One exception that we left out was serializing Error objects as values. We serialize "throws" or "rejections" as Error (regardless of their type) but not Error values. This fixes that by serializing `Error` objects. We don't include digest in this case since we don't call `onError` and it's not really expected that you'd log it on the server with some way to look it up. In general this is not super useful outside throws. Especially since we hide their values in prod. However, there is one case where it is quite useful. When you replay console logs in DEV you might often log an Error object within the scope of a Server Component. E.g. the default RSC error handling just console.error and error object. Before this would just be an empty object due to our lax console log serialization: Screenshot 2024-09-30 at 2 24 03 PM After: Screenshot 2024-09-30 at 2 36 48 PM TODO for a follow up: Flight Reply direction. This direction doesn't actually serialize thrown errors because they always reject the serialization. --- .../react-client/src/ReactFlightClient.js | 74 +++++++++---------- .../src/__tests__/ReactFlight-test.js | 40 ++++++++++ .../react-server/src/ReactFlightServer.js | 36 +++++++++ 3 files changed, 112 insertions(+), 38 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 83c509dab46a8..98a52c4ec4a9a 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1287,6 +1287,21 @@ function parseModelString( createFormData, ); } + case 'Z': { + // Error + if (__DEV__) { + const ref = value.slice(2); + return getOutlinedModel( + response, + ref, + parentObject, + key, + resolveErrorDev, + ); + } else { + return resolveErrorProd(response); + } + } case 'i': { // Iterator const ref = value.slice(2); @@ -1881,11 +1896,7 @@ function formatV8Stack( } type ErrorWithDigest = Error & {digest?: string}; -function resolveErrorProd( - response: Response, - id: number, - digest: string, -): void { +function resolveErrorProd(response: Response): Error { if (__DEV__) { // 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 @@ -1899,25 +1910,17 @@ function resolveErrorProd( ' may provide additional details about the nature of the error.', ); error.stack = 'Error: ' + error.message; - (error: any).digest = digest; - const errorWithDigest: ErrorWithDigest = (error: any); - const chunks = response._chunks; - const chunk = chunks.get(id); - if (!chunk) { - chunks.set(id, createErrorChunk(response, errorWithDigest)); - } else { - triggerErrorOnChunk(chunk, errorWithDigest); - } + return error; } function resolveErrorDev( response: Response, - id: number, - digest: string, - message: string, - stack: ReactStackTrace, - env: string, -): void { + errorInfo: {message: string, stack: ReactStackTrace, env: string, ...}, +): Error { + const message: string = errorInfo.message; + const stack: ReactStackTrace = errorInfo.stack; + const env: string = errorInfo.env; + if (!__DEV__) { // 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 @@ -1957,16 +1960,8 @@ function resolveErrorDev( } } - (error: any).digest = digest; (error: any).environmentName = env; - const errorWithDigest: ErrorWithDigest = (error: any); - const chunks = response._chunks; - const chunk = chunks.get(id); - if (!chunk) { - chunks.set(id, createErrorChunk(response, errorWithDigest)); - } else { - triggerErrorOnChunk(chunk, errorWithDigest); - } + return error; } function resolvePostponeProd(response: Response, id: number): void { @@ -2622,17 +2617,20 @@ function processFullStringRow( } case 69 /* "E" */: { const errorInfo = JSON.parse(row); + let error; if (__DEV__) { - resolveErrorDev( - response, - id, - errorInfo.digest, - errorInfo.message, - errorInfo.stack, - errorInfo.env, - ); + error = resolveErrorDev(response, errorInfo); + } else { + error = resolveErrorProd(response); + } + (error: any).digest = errorInfo.digest; + const errorWithDigest: ErrorWithDigest = (error: any); + const chunks = response._chunks; + const chunk = chunks.get(id); + if (!chunk) { + chunks.set(id, createErrorChunk(response, errorWithDigest)); } else { - resolveErrorProd(response, id, errorInfo.digest); + triggerErrorOnChunk(chunk, errorWithDigest); } return; } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 34986dc623de8..27db069ef324f 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -653,6 +653,46 @@ describe('ReactFlight', () => { `); }); + it('can transport Error objects as values', async () => { + function ComponentClient({prop}) { + return ` + is error: ${prop instanceof Error} + message: ${prop.message} + stack: ${normalizeCodeLocInfo(prop.stack).split('\n').slice(0, 2).join('\n')} + environmentName: ${prop.environmentName} + `; + } + const Component = clientReference(ComponentClient); + + function ServerComponent() { + const error = new Error('hello'); + return ; + } + + const transport = ReactNoopFlightServer.render(); + + await act(async () => { + ReactNoop.render(await ReactNoopFlightClient.read(transport)); + }); + + if (__DEV__) { + expect(ReactNoop).toMatchRenderedOutput(` + is error: true + message: hello + stack: Error: hello + in ServerComponent (at **) + environmentName: Server + `); + } else { + expect(ReactNoop).toMatchRenderedOutput(` + is error: true + message: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error. + stack: Error: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error. + environmentName: undefined + `); + } + }); + it('can transport cyclic objects', async () => { function ComponentClient({prop}) { expect(prop.obj.obj.obj).toBe(prop.obj.obj); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 5d79482de0186..19c40c214b918 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -2688,6 +2688,9 @@ function renderModelDestructive( if (typeof FormData === 'function' && value instanceof FormData) { return serializeFormData(request, value); } + if (value instanceof Error) { + return serializeErrorValue(request, value); + } if (enableBinaryFlight) { if (value instanceof ArrayBuffer) { @@ -3114,6 +3117,36 @@ function emitPostponeChunk( request.completedErrorChunks.push(processedChunk); } +function serializeErrorValue(request: Request, error: Error): string { + if (__DEV__) { + let message; + let stack: ReactStackTrace; + let env = (0, request.environmentName)(); + try { + // eslint-disable-next-line react-internal/safe-string-coercion + message = String(error.message); + stack = filterStackTrace(request, error, 0); + const errorEnv = (error: any).environmentName; + if (typeof errorEnv === 'string') { + // This probably came from another FlightClient as a pass through. + // Keep the environment name. + env = errorEnv; + } + } catch (x) { + message = 'An error occurred but serializing the error message failed.'; + stack = []; + } + const errorInfo = {message, stack, env}; + const id = outlineModel(request, errorInfo); + return '$Z' + id.toString(16); + } else { + // In prod we don't emit any information about this Error object to avoid + // unintentional leaks. Since this doesn't actually throw on the server + // we don't go through onError and so don't register any digest neither. + return '$Z'; + } +} + function emitErrorChunk( request: Request, id: number, @@ -3403,6 +3436,9 @@ function renderConsoleValue( if (typeof FormData === 'function' && value instanceof FormData) { return serializeFormData(request, value); } + if (value instanceof Error) { + return serializeErrorValue(request, value); + } if (enableBinaryFlight) { if (value instanceof ArrayBuffer) {