fix(core): Be stricter about mechanism values
#4068
Conversation
size-limit report
|
lobsterkatie
force-pushed
the
kmclb-be-stricter-about-mechanism-values
branch
from
d341330 to
515dd2e
Compare
Extracted from #4068, to ensure that it doesn't break current behavior.
lobsterkatie
force-pushed
the
kmclb-be-stricter-about-mechanism-values
branch
from
515dd2e to
707eb5d
Compare
| if (!event.exception || !event.exception.values) { | ||
| return; | ||
| } | ||
| const exceptionValue0 = event.exception.values[0]; |
There was a problem hiding this comment.
feels weird this just updates the first exception.
There was a problem hiding this comment.
To be fair, that's what the function it's replacing did, also (which is reflective of the fact that 99.9% of the time, there is only one exception). But it's a fair point.
That said, it's not obvious to me what the correct fix would be, because I've seen so few examples of there being multiple exceptions. Would they always be caught by the same mechanism (in which case we can just loop) or might the values be different (in which case we'd need to get more complicated and specify which one we're talking about)? TBH, I'm not even 100% sure what could ever cause there to be more than one...
In any case, happy to discuss it but am not going to block on it.
Though we have an existing
Mechanismtype, we haven't actually been using it inaddExceptionMechanism. As a result, I didn't catch the fact that themechanismdata I added in #4046 was malformed, and therefore being ignored by Sentry.This fixes both of those problems (the not using of the type and the malformed data being sent). Using the correct type also allowed
addExceptionMechanismto be streamlined quite a bit.Finally, given that the question of what the different attributes and their values actually mean has come up more than once, I conferred with the team, and added a docstring to the type. Hopefully this will help anyone who needs to add
mechanismdata in the future.