Repository navigation
fix(core): stop normalize() from throwing on symbol and null-prototype errors - #25967
Draft
rjmcgee53192 wants to merge 1 commit into
Draft
rjmcgee53192 wants to merge 1 commit into
rjmcgee53192 wants to merge 1 commit into
Conversation
…e errors normalize() built its **non-serializable** marker by interpolating the caught value into a template literal. That conversion throws for a symbol and for an object with no usable toString, so a property getter throwing either one made normalize() throw instead of returning the marker its docs promise. Convert the caught value with String() inside a try/catch and fall back to fixed text when even that throws. Covers both catch blocks in normalize.ts (normalize and stringifyValue) and adds regression tests for a getter throwing a symbol, a getter throwing a null-prototype object, and stringifyValue failing with a symbol. Fixes getsentry#25920
This branch has not been deployed
This file contains hidden or 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
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.
Why:
normalize()is documented to always return a value, using the**non-serializable**marker when something goes wrong — but it could throw itself. Both places that build that marker interpolated the caught value into a template literal, and that conversion throws for a symbol (Cannot convert a Symbol value to a string) and for an object with no usabletoString. A property getter throwing either one is enough to makenormalize()throw during event preparation. The caught value is now converted withString()inside a try/catch, falling back to fixed text, so callers only ever get the marker.How to review: One small helper in
packages/core/src/utils/normalize.ts, used by the two catch blocks there. The new tests innormalize.test.tscover a getter throwing a symbol, a getter throwing a null-prototype object, and thestringifyValuecatch failing with a symbol. I verified the fix by running the issue's reproduction against the source directly (both cases threw before, both return the marker after, ordinary inputs unchanged). I could not complete a full localpnpm installin my environment, so CI will be the first full suite run — flagging that honestly rather than claiming a green local run.Unsure about: Opening this as a draft without a prior maintainer go-ahead on the issue — happy to adjust the approach if you'd rather handle it differently.
Closes #25920