sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Handle AggregateErrors

Open mortargrind opened this issue 3 years ago • 4 comments

Problem Statement

Hi,

I think both on the JS SDK side and Sentry side things like AggregateError needs to be handled a bit more specifically in a built-in way. Basically AggregateError is a type of error that groups other kinds of errors together and can have an errors property on it, which itself is an array of errors.

On the SDK side: they should be sent in the report request and in the Sentry UI they should be shown in a given error detail page. I think these can be handled in the same or similar way to linked errors.

Also in Python we have ExceptionGroup, in .NET we have AggregateException, and for the languages that does not have this kind of thing directly; something similar can be implemented in the user-land.

Solution Brainstorm

Most probably a new integration like AggregatedErrors would do the job on the SDKs sides via both handling the language specific built-in ones and/or allowing custom implementations (so people that dont care about them dont need to enable/add it to their SDK instance).

mortargrind avatar Jul 26 '22 15:07 mortargrind

Hey, thanks for writing in. I agree this would be a cool integration to include, backlogging for now, but PRs are welcome for those who would like to help out!

It would probably work similar to the LinkedErrors integration: https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/linkederrors.ts

AbhiPrasad avatar Jul 26 '22 15:07 AbhiPrasad

I directly opened a feature request instead of a pull request because I think there are some product & design decisions to be made for the Sentry itself first. Because having only a functionality on the JS SDK means nothing by itself without the intention and clear expectations on the UI side since this requires a new way of displaying the aggregated errors, different than but compatible with the current linked errors: otherwise you wouldn't be able to differentiate linked and aggregated errors in the event detail page.

As for the implementation ideas:

It would work similar to LinkedErrors, but not exactly the same. I think it needs to work in harmony with LinkedErrors. See a complex but possible scenario below:

Legend:

  • Error1 => Error2 means Error1 caused by Error2 (handled by the existing LinkedErrors)
  • Error1 [Error2, Error3] means Error1 aggregates Error2 and Error3 (would be handled by the hypothetical AggregatedErrors)

Sample error chain:

HighLevelError => AggregateError [LowLevelError1, LowLevelError2 => LowerLevelError1, LowLevelError3] => LowestLevelError

If Sentry team is down with these suggestions & possibilities there in and has an idea about what kind of updates will/would need to happen on the UI side then I can try to create PoC PR and you can use it to implement & test the UI? I can most probably even open a PR for the Sentry UI itself but still would need to know what kind of a UI is needed first.

mortargrind avatar Jul 26 '22 19:07 mortargrind

Great point. We don't have support for this at the moment, since our event schema expects a list of Exceptions, which means we can only represent chained exceptions. A POC for this would be more involved, since we would have to make event ingestion changes, and think about how we index this also (I assume you would like to query for these exceptions 😄).

https://develop.sentry.dev/sdk/event-payloads/exception

I'm going to circle this around internally and see what folks think! Thank you for the quick response :)

AbhiPrasad avatar Jul 26 '22 19:07 AbhiPrasad

Hey as an update - we are looking to try to solve this across all the SDKs internally. We’ll update this issue when we have more information related to that!

AbhiPrasad avatar Jul 27 '22 23:07 AbhiPrasad

We're tracking this at Sentry: https://github.com/getsentry/sentry/issues/37716 as it requires some product changes. Won't be prioritized anytime soon, but will be in the future!

AbhiPrasad avatar Aug 11 '22 17:08 AbhiPrasad

Hi. Just wanted to ask anyone who's interested in this topic to chime in on the other issue, starting here: https://github.com/getsentry/sentry/issues/37716#issuecomment-1472884462. Thanks.

mattjohnsonpint avatar Mar 17 '23 00:03 mattjohnsonpint