Description
Problem Statement
In #1672, we implemented logic for dealing with AggregateException
by flattening the tree of inner exceptions and their children into a single list of exceptions. At the same time, we added an option KeepAggregateException
that controls whether the AggregateException
objects would remain in that list or be stripped out. The default is false
, meaning to strip them out.
Since then, we also ran in to several bugs caused by this approach:
- Nested inner exceptions are not filtered correctly #1916
- Stack traces missing from inner exceptions of aggregate exceptions #1917
- UnobservedTaskExceptionIntegration could provide a stack trace (but doesn't) #2065
Despite those fixes, we still have several problems with these workarounds not fitting the Sentry design - which is that multiple exceptions represent a single chain of events.
Consider this example:
AggregateException
A
1
B
2
Currently, we'd flatten that in to one big list and remove the aggregate (by default). Sentry reverses the ordering, and presently we sort the inner exceptions in ascending order, so the list sent would be [1,A,2,B]
. An issue would be titled with the details of B
, showing 2
as the cause of B
(correct), then A
as the cause of 2
(incorrect), then 1
as the cause of A
(correct).
Besides the incorrect ordering, this also has other more serious consequences:
- An issue will not get created for
A
. If it happened to be a different exception type thanB
, it could go unnoticed. - The issue grouping rules will take the whole chain of exceptions into account. Thus if there's another error raised independently for
B
, it wouldn't be grouped with the instance ofB
that was part of the aggregate. - Similarly, the quantity of exceptions can affect the grouping. Say one has code that performs parallel tasks, but might have a different number of tasks each time.
[A,A]
would be grouped separately from[A,A,A]
or[A,A,A,A,A,A]
(etc.) - each issue having an identical title and cause. This also messes up any alerting rules, and can resurrect issues that were intentionally ignored. - Within the SDK itself, we have issues with processing aggregate exceptions:
- Exception filters will only filter out an `AggregateException if all of the inner exceptions were supposed to be filtered.
- Exception processors, event processors, and
BeforeSend
all present interfaces that require special handling ofAggregateException
in each implementation. - When we strip away an aggregate exception, if there was no stack trace on the inner exception then we copy the one from the aggregate. This is usually appropriate - so that one can have some sense of where to start debugging. However, the stack trace now no longer applies to the exception being viewed - so it's a bit disingenuous.
Ultimately, the applied workaround is not very robust. It works best when there's only one inner exception in the aggregate, but has problems when there are multiple.
Without the workarounds, the above example would be sent as [1,A,AggregateException]
. All issues with an aggregate would be titled the same, and we'd lose any information about B
. So this is also not a good plan.
Solution Brainstorm
The latest version of Python (3.11) had a similar feature added called ExceptionGroup
. There's also AggregateError
in JavaScript, and several others. Together these have increased the demand for a properly designed solution.
We're tracking the new design in getsentry/sentry#37716. When ready, we should undo the previous .NET workarounds and implement the new unified design. Please add feedback about the new design over there.
We'll leave this issue open to track the work in .NET, and to comment on any other issues specifically related to AggregateException
or the workarounds we've currently implemented.
Metadata
Assignees
Labels
Type
Projects
Status
Done
Activity