Thrown object in RSC to be shown as stringified text not [Object object] in client side (next.js) #28312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Comparing: 269edb8...a8c57be Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
|
There's nothing to indicate that this is meant as an error just because it has a If anything we could serialize more of the properties optimistically, perhaps as part of the error message which is then turned into an Error object on the client. Like we do with describeObjectForErrorMessage for logged errors. This all only affects DEV anyway. |
|
@sebmarkbage It is a common pattern. See https://stackoverflow.com/questions/41102060/typescript-extending-error-class We extend the error object and throw it as a normal error. This worked fine before Next.js adopted RSC. Now it's broken. We can't seem to get useful error messages when users report with screenshots.. All we see in our logs is |
|
@carlos-dubon All the answers in the linked SO use a proper prototype chain which would return true for Like I said, we could stringify a shallow JSON representation of the error but that doesn't make it a good idea to not do a proper Error. |
|
Thanks for flagging. I'm going to land #28327 instead for now but that one too really needs to expand the context that it provides. |
sebmarkbage
added a commit
that referenced
this pull request
Feb 14, 2024
) 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.
github-actions bot
pushed a commit
that referenced
this pull request
Feb 14, 2024
) 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)
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
…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.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my first PR for
reactand I have completed CLA.Summary
error.messagepassed from a server component that throws object (normally Error) only displays [object Object] in client side.see issue in Next.js below.
vercel/next.js#61902
So I add conditional branch in line 1680 of
react/packages/react-server/src/ReactFlightServer.jsso that not only thrownErrorbut alsoobjectcan be stringified.I am not sure how many people throw object (not Error, usually Error is better since Error can trace stacks) in server component in production, but it would help one user with the issue above.
How did you test this change?
Clone https://github.com/carlos-dubon/nextjs-error-bug and
yarn install. ( don't forget to installnext 14.1.1-canary.48andreact 18.2.0andreact-dom 18.2.0).Then beatify
node_modules/next/dist/compiled/next-server/app-page.runtime.dev.jsand edit line 14534.after that
yarn devand go to/page and, edit line 10 of this file toand you can get the message.
Test cases
yarn test ReactServerandyarn test --prod ReactServerwas passed without fail.