Skip to content

Commit a7144f2

Browse files
authored
[Flight] Improve error message when it's not a real Error object (#28327)
Also deals with symbols. Alternative to #28312. We currently always normalize rejections or thrown values into `Error` objects. Partly because in prod it'll be an error object and you shouldn't fork behavior on knowing the value outside a digest. We might want to even make the message always opaque to avoid being tempted and then discover in prod that it doesn't work. However, we do include the message in DEV. If this is a non-Error object we don't know what the properties mean. Ofc, we don't want to include too much information in the rendered string, so we use the general `describeObjectForErrorMessage` helper. Unfortunately it's pretty conservative about emitting values so it's likely to exclude any embedded string atm. Could potentially expand it a bit. We could in theory try to serialize as much as possible and re-throw the actual object to allow for inspection to be expanded inside devtools which is what I plan on for consoles, but since we're normalizing to an Error this is in conflict with that approach.
1 parent adadb81 commit a7144f2

File tree

2 files changed

+114
-2
lines changed

2 files changed

+114
-2
lines changed

packages/react-client/src/__tests__/ReactFlight-test.js

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,16 @@ describe('ReactFlight', () => {
8383
' builds to avoid leaking sensitive details. A digest property is included on this error instance which' +
8484
' may provide additional details about the nature of the error.',
8585
);
86-
expect(this.state.error.digest).toContain(this.props.expectedMessage);
86+
let expectedDigest = this.props.expectedMessage;
87+
if (
88+
expectedDigest.startsWith('Error: {') ||
89+
expectedDigest.startsWith('Error: <')
90+
) {
91+
expectedDigest = '{}';
92+
} else if (expectedDigest.startsWith('Error: [')) {
93+
expectedDigest = '[]';
94+
}
95+
expect(this.state.error.digest).toContain(expectedDigest);
8796
expect(this.state.error.stack).toBe(
8897
'Error: ' + this.state.error.message,
8998
);
@@ -776,6 +785,106 @@ describe('ReactFlight', () => {
776785
});
777786
});
778787

788+
it('should emit descriptions of errors in dev', async () => {
789+
const ClientErrorBoundary = clientReference(ErrorBoundary);
790+
791+
function Throw({value}) {
792+
throw value;
793+
}
794+
795+
const testCases = (
796+
<>
797+
<ClientErrorBoundary expectedMessage="This is a real Error.">
798+
<div>
799+
<Throw value={new TypeError('This is a real Error.')} />
800+
</div>
801+
</ClientErrorBoundary>
802+
<ClientErrorBoundary expectedMessage="Error: This is a string error.">
803+
<div>
804+
<Throw value="This is a string error." />
805+
</div>
806+
</ClientErrorBoundary>
807+
<ClientErrorBoundary expectedMessage="Error: {message: ..., extra: ..., nested: ...}">
808+
<div>
809+
<Throw
810+
value={{
811+
message: 'This is a long message',
812+
extra: 'properties',
813+
nested: {more: 'prop'},
814+
}}
815+
/>
816+
</div>
817+
</ClientErrorBoundary>
818+
<ClientErrorBoundary
819+
expectedMessage={
820+
'Error: {message: "Short", extra: ..., nested: ...}'
821+
}>
822+
<div>
823+
<Throw
824+
value={{
825+
message: 'Short',
826+
extra: 'properties',
827+
nested: {more: 'prop'},
828+
}}
829+
/>
830+
</div>
831+
</ClientErrorBoundary>
832+
<ClientErrorBoundary expectedMessage="Error: Symbol(hello)">
833+
<div>
834+
<Throw value={Symbol('hello')} />
835+
</div>
836+
</ClientErrorBoundary>
837+
<ClientErrorBoundary expectedMessage="Error: 123">
838+
<div>
839+
<Throw value={123} />
840+
</div>
841+
</ClientErrorBoundary>
842+
<ClientErrorBoundary expectedMessage="Error: undefined">
843+
<div>
844+
<Throw value={undefined} />
845+
</div>
846+
</ClientErrorBoundary>
847+
<ClientErrorBoundary expectedMessage="Error: <div/>">
848+
<div>
849+
<Throw value={<div />} />
850+
</div>
851+
</ClientErrorBoundary>
852+
<ClientErrorBoundary expectedMessage="Error: function Foo() {}">
853+
<div>
854+
<Throw value={function Foo() {}} />
855+
</div>
856+
</ClientErrorBoundary>
857+
<ClientErrorBoundary expectedMessage={'Error: ["array"]'}>
858+
<div>
859+
<Throw value={['array']} />
860+
</div>
861+
</ClientErrorBoundary>
862+
</>
863+
);
864+
865+
const transport = ReactNoopFlightServer.render(testCases, {
866+
onError(x) {
867+
if (__DEV__) {
868+
return 'a dev digest';
869+
}
870+
if (x instanceof Error) {
871+
return `digest("${x.message}")`;
872+
} else if (Array.isArray(x)) {
873+
return `digest([])`;
874+
} else if (typeof x === 'object' && x !== null) {
875+
return `digest({})`;
876+
}
877+
return `digest(Error: ${String(x)})`;
878+
},
879+
});
880+
881+
await act(() => {
882+
startTransition(() => {
883+
ReactNoop.render(ReactNoopFlightClient.read(transport));
884+
});
885+
});
886+
});
887+
779888
it('should trigger the inner most error boundary inside a Client Component', async () => {
780889
function ServerComponent() {
781890
throw new Error('This was thrown in the Server Component.');

packages/react-server/src/ReactFlightServer.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1677,8 +1677,11 @@ function emitErrorChunk(
16771677
message = String(error.message);
16781678
// eslint-disable-next-line react-internal/safe-string-coercion
16791679
stack = String(error.stack);
1680+
} else if (typeof error === 'object' && error !== null) {
1681+
message = 'Error: ' + describeObjectForErrorMessage(error);
16801682
} else {
1681-
message = 'Error: ' + (error: any);
1683+
// eslint-disable-next-line react-internal/safe-string-coercion
1684+
message = 'Error: ' + String(error);
16821685
}
16831686
} catch (x) {
16841687
message = 'An error occurred but serializing the error message failed.';

0 commit comments

Comments
 (0)