[Flight] Improve error message when it's not a real Error object #28327

sebmarkbage · 2024-02-14T18:16:24Z

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.

react-sizebot · 2024-02-14T18:20:35Z

Comparing: dc31781...fee168e

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	176.83 kB	176.83 kB	=	55.11 kB	55.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	178.83 kB	178.83 kB	=	55.69 kB	55.69 kB
facebook-www/ReactDOM-prod.classic.js	=	592.18 kB	592.18 kB	=	104.43 kB	104.43 kB
facebook-www/ReactDOM-prod.modern.js	=	575.46 kB	575.46 kB	=	101.43 kB	101.43 kB
test_utils/ReactAllWarnings.js	Deleted	66.26 kB	0.00 kB	Deleted	16.32 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable-semver/react-server/cjs/react-server-flight.development.js	+0.30%	69.74 kB	69.95 kB	+0.15%	16.71 kB	16.73 kB
oss-stable/react-server/cjs/react-server-flight.development.js	+0.30%	69.74 kB	69.95 kB	+0.15%	16.71 kB	16.73 kB
oss-experimental/react-server/cjs/react-server-flight.development.js	+0.27%	79.44 kB	79.65 kB	+0.12%	18.67 kB	18.70 kB
facebook-www/ReactFlightDOMServer-dev.modern.js	+0.25%	87.92 kB	88.14 kB	+0.14%	18.75 kB	18.77 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js	+0.20%	103.95 kB	104.16 kB	+0.12%	24.50 kB	24.53 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js	+0.20%	103.95 kB	104.16 kB	+0.12%	24.50 kB	24.53 kB
test_utils/ReactAllWarnings.js	Deleted	66.26 kB	0.00 kB	Deleted	16.32 kB	0.00 kB

Generated by 🚫 dangerJS against fee168e

gnoff

Should we do the same for encodeErrorForBoundary in Fizz?

Also, seems like in Prod the error boundary isn't catching the error? Don't understand how that could be

sebmarkbage · 2024-02-14T22:25:20Z

Should we do the same for encodeErrorForBoundary in Fizz?

Looks like we look for the .message property there which isn't right, so yea. Probably.

Also deals with symbols.

sebmarkbage · 2024-02-14T22:28:20Z

It sucks that DevTools doesn't print the .cause of an error. Otherwise we could encode a richer error wrapped in another error.

However this whole DEV-only thing is weird. We probably shouldn't even expose it to programmatic code and only DevTools can actually see the hidden internals just like console.createTask().

) 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. DiffTrain build for [a7144f2](a7144f2)

sebmarkbage · 2024-02-15T00:39:44Z

packages/react-server/src/ReactFlightServer.js

      } else {
-        message = 'Error: ' + (error: any);


This prefix actually doesn't really make sense because this prefix already comes as part of the .stack property with the constructor's name.

Same as #28327 but for Fizz. One thing that's weird about this recoverable error is that we don't send the regular stack for it, just the component stack it seems. This is missing some potential information and if we move toward integrated since stacks it would be one thing.

Same as #28327 but for Fizz. One thing that's weird about this recoverable error is that we don't send the regular stack for it, just the component stack it seems. This is missing some potential information and if we move toward integrated since stacks it would be one thing. DiffTrain build for [2e470a7](2e470a7)

@acdlite

…ebook#28327) Also deals with symbols. Alternative to facebook#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.

) Same as facebook#28327 but for Fizz. One thing that's weird about this recoverable error is that we don't send the regular stack for it, just the component stack it seems. This is missing some potential information and if we move toward integrated since stacks it would be one thing.

) 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. DiffTrain build for commit a7144f2.

Same as #28327 but for Fizz. One thing that's weird about this recoverable error is that we don't send the regular stack for it, just the component stack it seems. This is missing some potential information and if we move toward integrated since stacks it would be one thing. DiffTrain build for commit 2e470a7.

sebmarkbage requested a review from gnoff February 14, 2024 18:16

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 14, 2024

sebmarkbage changed the title ~~[Flight] Improve error message when it's not a real message object~~ [Flight] Improve error message when it's not a real Error object Feb 14, 2024

sebmarkbage force-pushed the flightobjecterror branch from 5a33d8e to 6b66058 Compare February 14, 2024 18:22

gaearon approved these changes Feb 14, 2024

View reviewed changes

gnoff reviewed Feb 14, 2024

View reviewed changes

sebmarkbage force-pushed the flightobjecterror branch from 6b66058 to 4c64861 Compare February 14, 2024 22:21

Improve error message when it's not a real message object

fee168e

Also deals with symbols.

sebmarkbage force-pushed the flightobjecterror branch from 4c64861 to fee168e Compare February 14, 2024 22:26

sebmarkbage mentioned this pull request Feb 14, 2024

Thrown object in RSC to be shown as stringified text not [Object object] in client side (next.js) #28312

Closed

sebmarkbage merged commit a7144f2 into facebook:main Feb 14, 2024
36 checks passed

sebmarkbage mentioned this pull request Feb 14, 2024

[Fizz] Align recoverable error serialization in dev mode #28340

Merged

sebmarkbage commented Feb 15, 2024

View reviewed changes

This was referenced Feb 20, 2024

Bump vendored react versions vercel/next.js#62302

Closed

Upgrade vendored react vercel/next.js#62326

Merged

ztanner mentioned this pull request Mar 19, 2024

(backport) Update React from 60a927d04 to 14898b6a9 vercel/next.js#63442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Flight] Improve error message when it's not a real Error object #28327

[Flight] Improve error message when it's not a real Error object #28327

sebmarkbage commented Feb 14, 2024

react-sizebot commented Feb 14, 2024 •

edited

Loading

gnoff left a comment

sebmarkbage commented Feb 14, 2024

sebmarkbage commented Feb 14, 2024

sebmarkbage Feb 15, 2024

[Flight] Improve error message when it's not a real Error object #28327

[Flight] Improve error message when it's not a real Error object #28327

Conversation

sebmarkbage commented Feb 14, 2024

react-sizebot commented Feb 14, 2024 • edited Loading

Critical size changes

Significant size changes

gnoff left a comment

Choose a reason for hiding this comment

sebmarkbage commented Feb 14, 2024

sebmarkbage commented Feb 14, 2024

sebmarkbage Feb 15, 2024

Choose a reason for hiding this comment

react-sizebot commented Feb 14, 2024 •

edited

Loading