Skip to content
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

Add support for ExceptionGroups #2025

Merged
merged 44 commits into from
May 23, 2023
Merged

Conversation

antonpirker
Copy link
Member

@antonpirker antonpirker commented Apr 21, 2023

With Python 3.11 ExceptionGroups was introduced. This adds support for catching them and displaying them in a meaningful way.

See also the related RFC: https://github.com/getsentry/rfcs/blob/main/text/0079-exception-groups.md

Fixes #1788

@antonpirker antonpirker changed the base branch from master to antonpirker/upgrade_linting April 21, 2023 15:15
@antonpirker antonpirker marked this pull request as ready for review April 24, 2023 08:33
Base automatically changed from antonpirker/upgrade_linting to master April 25, 2023 08:52
@sl0thentr0py
Copy link
Member

@antonpirker pls rebase/merge master

@sl0thentr0py sl0thentr0py marked this pull request as draft April 25, 2023 12:57
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple of questions to start, I need to read the PEP to review this properly, will get back to it.

sentry_sdk/utils.py Outdated Show resolved Hide resolved
sentry_sdk/utils.py Outdated Show resolved Hide resolved
sentry_sdk/utils.py Show resolved Hide resolved
sentry_sdk/utils.py Outdated Show resolved Hide resolved
@antonpirker antonpirker marked this pull request as ready for review May 4, 2023 11:51
@mattjohnsonpint
Copy link
Contributor

This can be shipped as soon as getsentry/sentry#48653 is completed and released to production.

Also, for the best experience with exception groups, self-hosted Sentry users should take the corresponding update when it is available.

@antonpirker antonpirker self-assigned this May 8, 2023
sentry_sdk/utils.py Show resolved Hide resolved
sentry_sdk/utils.py Outdated Show resolved Hide resolved
sentry_sdk/utils.py Outdated Show resolved Hide resolved
sentry_sdk/utils.py Show resolved Hide resolved
sentry_sdk/utils.py Show resolved Hide resolved
tests/test_exceptiongroup.py Outdated Show resolved Hide resolved
tests/test_exceptiongroup.py Show resolved Hide resolved
@antonpirker antonpirker enabled auto-merge (squash) May 23, 2023 07:25
@antonpirker antonpirker merged commit 5564011 into master May 23, 2023
@antonpirker antonpirker deleted the antonpirker/exception_groups branch May 23, 2023 07:30
@andyljones
Copy link

Hooray! I'm excited to start using this. I updated to 1.24 and tried it out, and I'm not seeing the nested error in the UI yet; it bottoms out at the nursery still. Is there anything I need to do to switch this on?

import trio


async def f():
    raise ValueError("echo")


async def run():
    async with trio.open_nursery() as nursery:
        nursery.start_soon(f)


trio.run(run)
image image

@mattjohnsonpint
Copy link
Contributor

@andyljones - That's because you're only raising one error, and the default behavior for trio is to only raise an exception group if there's more than one error in the group.

To raise an exception group from Trio, you can do either of the following:

  • Do at least two things that fail:

    async def run():
        async with trio.open_nursery() as nursery:
            nursery.start_soon(f)
            nursery.start_soon(f)
  • Tell Trio that you want to use "strict" mode to always get an exception group regardless of number of errors.

    async def run():
        async with trio.open_nursery(strict_exception_groups=True) as nursery:
            nursery.start_soon(f)

See "Strict" versus "loose" ExceptionGroup semantics in the Trio docs.

The section right before that is also a great introduction to exception groups in Trio.

@mattjohnsonpint
Copy link
Contributor

There's nothing else needed in Sentry, other than updating to 1.24.0 or newer. An event with two ValueErrors raised by Trio looks like this in Sentry:

image

Note that NonBaseMultiError is an exception defined by Trio that inherits from ExceptionGroup.

@mattjohnsonpint
Copy link
Contributor

Also, just to point out some of the extra work that went into this on the Sentry side, note that an error such as the one shown above will be titled and grouped by the first inner exception:

image

That's because all exceptions within the group are of the same type and have similar values. Without this, the error would be titled by the root exception, "NonBaseMultiError: multiple tasks failed" - which could make it difficult to distinguish between issues on the issues overview. Also different quantities of inner exceptions of the same type and similar values will be grouped together, rather than separately. (i.e. having ValueError:echo two, three, or four times in the group shouldn't make them different issues.)

Conversely, when the inner exceptions differ significantly - then we'll use the exception group as the title, and group issues based on the combination of the types and values of the inner exceptions. For example:

image

Will get titled as:

image

If another of the same exception comes through but the KeyError and IndexError exceptions are swapped, it will still get grouped together as another occurrence of the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for ExceptionGroup
4 participants