-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Add DOMException.code
as tag if it exists
#3018
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
|
d7839ce
to
18277ed
Compare
DOMException.code
to event message if .name
is undefinedDOMException.code
as tag if it exists
packages/browser/src/eventbuilder.ts
Outdated
@@ -87,6 +87,8 @@ export function eventFromUnknownInput( | |||
|
|||
event = eventFromString(message, syntheticException, options); | |||
addExceptionTypeValue(event, message); | |||
event.tags = domException.code ? { ...event.tags, 'DOMException.code': String(domException.code) } : event.tags; |
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.
eventFromString
has no way to add any tags to the event itself, so this fallback will always produce an event with malformed tags:
const event = {}
console.log(event) // {}
event.tags = event.tags
console.log(event) // {tags:undefined}
Also code
can be 0
for some custom types, which will omit this, as they are using integers, not strings.
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.
Ah, good catch. Thanks.
packages/browser/src/eventbuilder.ts
Outdated
@@ -87,6 +87,8 @@ export function eventFromUnknownInput( | |||
|
|||
event = eventFromString(message, syntheticException, options); | |||
addExceptionTypeValue(event, message); | |||
event.tags = domException.code ? { ...event.tags, 'DOMException.code': String(domException.code) } : event.tags; |
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.
event.tags = domException.code ? { ...event.tags, 'DOMException.code': String(domException.code) } : event.tags; | |
if ('code' in domException) { | |
event.tags = { ...event.tags, 'DOMException.code': `${domException.code}` } | |
} |
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.
I left the "old" tags, just in case we ever decided to for example mark events as being produced by eventFromString
or something. Keep in mind that its safe to destructure undefined values, see:
https://github.com/tc39/proposal-object-rest-spread/blob/master/Spread.md
Null/Undefined Are Ignored
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.
Wait, I'm confused. I did, too...
Though
DOMException.code
is long since deprecated in favor ofDOMException.name
, there are apparently still cases where it shows up. This adds the code as a tag in those cases.Fixes #3005.