-
Notifications
You must be signed in to change notification settings - Fork 41
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
Extract sub-exceptions from Exception Groups #332
Conversation
|
||
assert exceptions[0] == { | ||
'message': 'the message of the group (4 sub-exceptions)', | ||
'errorClass': 'builtins.ExceptionGroup', |
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.
Our fully_qualified_class_name
is appending builtins.
here, which is not technically wrong but is unexpected
Fixing this is trivial but I've split this into a separate task as:
- I'm not sure why this happens for
ExceptionGroup
but not other built-in exceptions fully_qualified_class_name
is also used when applyingignore_classes
, so we need to be careful not to change its behaviour in a way that would break ignoring errors
# here because there's a big risk of that leading to a huge number of | ||
# exceptions, which is difficult to reason about | ||
if isinstance(exception, BaseExceptionGroup): | ||
for sub_exception in exception.exceptions: # type: ignore |
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.
This type: ignore
comment is because we run mypy
on Python 3.5 (as well as 3.11) where it has no knowledge of BaseExceptionGroup
, so complains about using exception.exceptions
Goal
Python 3.11 added Exception Groups (see PEP 654) as a way of aggregating several unrelated exceptions into a single Exception-compatible object
bugsnag-python currently supports reporting Exception Groups, but there's no visibility of the sub-exceptions contained within the group. This is problematic as often the group won't contain enough information to debug the problems that caused it — the contained exceptions should be unrelated, so it's difficult to go from the group → its sub-exceptions
This PR extracts the sub-exceptions from Exception Groups, so that they are reported to bugsnag along with the group. This is very similar to how we report
AggregateExceptions
in the .NET notifierDesign
In order to avoid truly gigantic lists of errors in the dashboard, we don't recurse into sub-exception groups or their cause/context. The error reporting API also doesn't support tree-shaped hierarchies of errors, so this structure would be difficult to deal with in bugsnag anyway
For example, this exception group:
will be reported as:
This should still contain enough information to debug the errors and, if not, the exceptions can be reported separately by catching the
ExceptionGroup
itself