python / cpython

The Python programming language
https://www.python.org
Other
63.23k stars 30.28k forks source link

Add `.subgroup()` and `.split()` methods to `BaseException` #125825

Open Zac-HD opened 6 days ago

Zac-HD commented 6 days ago

As we scale up our use of except* and ExceptionGroups, we've noticed that isinstance() checks get noticeably more clumsy:

# often the isinstance is implicit due to an except-statement
not_found = isinstance(err, HTTPError) and err.status_code == 404

# becomes

if isinstance(err, BaseExceptionGroup):
    # often we'd want .split() to do something with the other case, but for simplicity:
    not_found = err.subgroup(lambda e: isinstance(e, HTTPError) and e.status_code == 404) is not None
else:
    # as above

Obviously we use except T as err: or except* T ... wherever possible, but there are cases where we need to inspect metadata beyond the error type and duplicating the logic often gets messy.

By adding .subgroup() and .split() methods to BaseException, which for non-groups act as if the exception was wrapped in a single-member group, we can avoid duplication and write only the necessarily-more-complicated code for groups to cover both cases.

(opening issue as discussed with @gpshead and @njsmith elsewhere)

Linked PRs

picnixz commented 6 days ago

Are you planning to work on a PR or may I take on that one?

Zac-HD commented 5 days ago

If you'd like to work on it, please do! (and I'll get back to pep-789)

iritkatriel commented 4 days ago

BaseException is a builtin, I don't think you can add methods to it without a PEP.

picnixz commented 4 days ago

Oh. I can work on this tomorrow then if needed (it'd a nice introduction to writing PEPs since this one shouldn't really be long IMO?)

iritkatriel commented 4 days ago

I wonder why you believe it makes sense to add this as a method of BaseException rather than a utility function (say add this to the stdlib somewhere)?

def exc_subgroup(err, func):
    if isinstance(err, BaseExceptionGroup):
        # often we'd want .split() to do something with the other case, but for simplicity:
        return err.subgroup(func)
    else:
        return exc if func(exc) else None
picnixz commented 4 days ago

When I implemented the PoC, what I did is essentially equivalent to wrap the leaf exception in an exception group and call the method on it directly.

I wonder why you believe it makes sense to add this as a method of BaseException rather than a utility function (say add this to the stdlib somewhere

I think we would like to preserve the return type of err.subgroup but that could also be done via a utility function maybe?

def wraps(exc):
    if isinstance(exc, BaseExceptionGroup):
        return exc
    return BaseExceptionGroup(str(exc), [exc])

# Usage: wraps(exc).split(func) or wraps(exc).subgroup(func)
Zac-HD commented 4 days ago

I suggested a method because both .split() and .subgroup() are already methods, and I think that makes sense as an interface which preserves the "returns a group" invariant.

Users can already define their own helpers if they know to try this (e.g. below), but I think it's worth giving the stronger nudge towards the ExceptionGroup methods as a convenient and safer way to handle errors when some might be groups.

def as_group(exc):
    if isinstance(exc, BaseExceptionGroup):
        return exc
    return BaseExceptionGroup("", [exc]))

as_group(...).split(...)
iritkatriel commented 4 days ago

I see where you're coming from, but I'm uneasy about adding Group semantics to BaseException. Definitely needs a PEP, IMO.

picnixz commented 2 days ago

I've drafted a first initial document: https://github.com/picnixz/peps/commit/cf62a2859c969f07bd10fb6b762ff0906d64c028. I'd be happy to continue working on it with @Zac-HD if you want (but we'd need to find a sponsor if we are going the PEP route).

iritkatriel commented 2 days ago

Could start by raising it on the ideas forum.