fix(core): Classify custom AggregateErrors as exception groups#19053
fix(core): Classify custom AggregateErrors as exception groups#19053
AggregateErrors as exception groups#19053Conversation
Lms24
changed the title
AggregateErrors as exception groupsAggregateErrors as exception groups
| @@ -0,0 +1,16 @@ | |||
| class CustomAggregateError extends AggregateError { | |||
There was a problem hiding this comment.
this test primarily demonstrates that this works for errors with the actual builtin AggregateError class
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| exception.mechanism = { | ||
| handled: true, | ||
| type: 'auto.core.linked_errors', | ||
| ...(isExceptionGroup(error) && { is_exception_group: true }), |
There was a problem hiding this comment.
Order change allows mechanism to override is_exception_group
Low Severity
The spread order in applyExceptionGroupFieldsForParentException changed from setting is_exception_group: true AFTER ...exception.mechanism to BEFORE it. Previously, the flag would always override for exception groups. Now, if exception.mechanism already contains an is_exception_group value (even false), it will override the duck-typed detection. This could cause exception groups to be incorrectly classified if a custom exceptionFromErrorImplementation sets is_exception_group in the mechanism.
There was a problem hiding this comment.
yeah but I think this is actually more correct. Theoretically users could set this beforehand and I don't think it hurts us to flip this in the default case. Gonna leave it as-is.
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
3 participants
This PR fixes a bug where classes extending from
AggregateErrorwould not correctly be identified as exception groups in Sentry because theis_exception_groupflag was missing in the error's mechanism.Implementation note: We can't easily check for
instanceOf AggregateErrorbecause we don't have that type available in Core (and it's not supported in every platform). Which is why for now I opted to "duck type" check if theerror.errorsproperty is an array. Which is the same check we already use for chaining errors together in the first place.Closes #19014