From d875c0c282955023172989704dc94ca4fb739718 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 14 Feb 2024 18:22:39 -0500 Subject: [PATCH 1/2] Align recoverable error serialization in dev mode --- packages/react-server/src/ReactFizzServer.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a928b2651a934..d4bb51211d672 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -33,6 +33,7 @@ import type {ComponentStackNode} from './ReactFizzComponentStack'; import type {TreeContext} from './ReactFizzTreeContext'; import type {ThenableState} from './ReactFizzThenable'; import {enableRenderableContext} from 'shared/ReactFeatureFlags'; +import {describeObjectForErrorMessage} from 'shared/ReactSerializationErrors'; import { scheduleWork, @@ -816,18 +817,19 @@ function encodeErrorForBoundary( ) { boundary.errorDigest = digest; if (__DEV__) { + let message; // In dev we additionally encode the error message and component stack on the boundary - let errorMessage; - if (typeof error === 'string') { - errorMessage = error; - } else if (error && typeof error.message === 'string') { - errorMessage = error.message; + if (error instanceof Error) { + // eslint-disable-next-line react-internal/safe-string-coercion + message = String(error.message); + } else if (typeof error === 'object' && error !== null) { + message = 'Error: ' + describeObjectForErrorMessage(error); } else { // eslint-disable-next-line react-internal/safe-string-coercion - errorMessage = String(error); + message = 'Error: ' + String(error); } - boundary.errorMessage = errorMessage; + boundary.errorMessage = message; boundary.errorComponentStack = thrownInfo.componentStack; } } From 685f199bc748d99e03fbf60a8240000a5d6784f2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 14 Feb 2024 19:42:49 -0500 Subject: [PATCH 2/2] Remove Error: prefix These don't really make sense to add to the message string since these prefixes are typically printed when the errors are printed anyway and are part of .stack. So prefixing just leads to duplicate prefixes. --- .../src/__tests__/ReactFlight-test.js | 28 +++++++++---------- .../src/__tests__/ReactFlightDOM-test.js | 4 +-- .../__tests__/ReactFlightDOMBrowser-test.js | 2 +- packages/react-server/src/ReactFizzServer.js | 4 +-- .../react-server/src/ReactFlightServer.js | 4 +-- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index f4a5929a2dd72..06dee1d54ed22 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -85,11 +85,11 @@ describe('ReactFlight', () => { ); let expectedDigest = this.props.expectedMessage; if ( - expectedDigest.startsWith('Error: {') || - expectedDigest.startsWith('Error: <') + expectedDigest.startsWith('{') || + expectedDigest.startsWith('<') ) { expectedDigest = '{}'; - } else if (expectedDigest.startsWith('Error: [')) { + } else if (expectedDigest.startsWith('[')) { expectedDigest = '[]'; } expect(this.state.error.digest).toContain(expectedDigest); @@ -799,12 +799,12 @@ describe('ReactFlight', () => { - +
- +
{
+ expectedMessage={'{message: "Short", extra: ..., nested: ...}'}>
{ />
- +
- +
- +
- +
} />
- +
- +
@@ -874,7 +872,7 @@ describe('ReactFlight', () => { } else if (typeof x === 'object' && x !== null) { return `digest({})`; } - return `digest(Error: ${String(x)})`; + return `digest(${String(x)})`; }, }); 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 91ba06dd329b9..465123f825500 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -919,9 +919,7 @@ describe('ReactFlightDOM', () => { abort('for reasons'); }); if (__DEV__) { - expect(container.innerHTML).toBe( - '

Error: for reasons + a dev digest

', - ); + expect(container.innerHTML).toBe('

for reasons + a dev digest

'); } else { expect(container.innerHTML).toBe('

digest("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 2427317ce2c32..f8f3a1c3f2877 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -583,7 +583,7 @@ describe('ReactFlightDOMBrowser', () => { controller.abort('for reasons'); }); const expectedValue = __DEV__ - ? '

Error: for reasons + a dev digest

' + ? '

for reasons + a dev digest

' : '

digest("for reasons")

'; expect(container.innerHTML).toBe(expectedValue); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index d4bb51211d672..e8ea394386df6 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -823,10 +823,10 @@ function encodeErrorForBoundary( // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); } else if (typeof error === 'object' && error !== null) { - message = 'Error: ' + describeObjectForErrorMessage(error); + message = describeObjectForErrorMessage(error); } else { // eslint-disable-next-line react-internal/safe-string-coercion - message = 'Error: ' + String(error); + message = String(error); } boundary.errorMessage = message; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 4c81931223e46..072a71b03dcc5 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1678,10 +1678,10 @@ function emitErrorChunk( // eslint-disable-next-line react-internal/safe-string-coercion stack = String(error.stack); } else if (typeof error === 'object' && error !== null) { - message = 'Error: ' + describeObjectForErrorMessage(error); + message = describeObjectForErrorMessage(error); } else { // eslint-disable-next-line react-internal/safe-string-coercion - message = 'Error: ' + String(error); + message = String(error); } } catch (x) { message = 'An error occurred but serializing the error message failed.';