-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(normalize): Treat Infinity as NaN both are non-serializable numbers #13406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
size-limit report 📦
|
@@ -224,6 +225,10 @@ function stringifyValue( | |||
return '[NaN]'; | |||
} | |||
|
|||
if (typeof value === 'number' && !Number.isFinite(value)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, we can improve this even further :D we can combine this with the check above for nan, as (NaN).toString()
also returns NaN
. And isFinite(NaN) === false
. So we can remove this completely:
if (typeof value === 'number' && value !== value) {
return '[NaN]';
}
and only keep the new check you added!
@@ -10,5 +10,5 @@ sentryTest('should set extras from multiple consecutive calls', async ({ getLoca | |||
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url); | |||
|
|||
expect(eventData.message).toBe('consecutive_calls'); | |||
expect(eventData.extra).toMatchObject({ extra: [], Infinity: 2, null: null, obj: { foo: ['bar', 'baz', 1] } }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value
under null
key
is Infinity
Line 8 in f664a5f
Sentry.setExtras({ [null]: Infinity }); |
@krystofwoldrich this was released with https://github.com/getsentry/sentry-javascript/releases/tag/8.31.0 |
RN SDK uses the normalize function before passing data over the RN Bridge, which only accepts serializable data.
Infinity causes -> getsentry/sentry-react-native#4024