On Thu, Feb 25, 2021 at 9:40 PM Nathaniel Smith <
njs@pobox.com> wrote:
[...]
Yury/I/others did discuss the idea of a
BaseExceptionGroup/ExceptionGroup split a lot, and I think the general
feeling is that it could potentially work, but feels like a
complicated and awkward hack, so no-one was super excited about it.
Oh, I didn't realize (or recall) the idea was considered before. Given the pseudo code I showed I don't think it's particularly complicated or awkward, although it's of course more complicated than the design that's currently in our PEP.
For a while we also had a compromise design where only
BaseExceptionGroup was built-in, but we left it non-final specifically
so asyncio could define an ExceptionsOnlyExceptionGroup.
I did notice that. Given how many distinct use cases we've discovered by now I think this is a dead end.
Another somewhat-related awkward part of the API is how ExceptionGroup
and plain-old 'except' should interact *in general*. The intuition is
that if you have 'except ValueError' and you get an
'ExceptionGroup(ValueError)', then the user's code has some kind of
problem and we should probably do.... something? to let them know? One
idea I had was that we should raise a RuntimeError if this happens,
sort of similar to PEP 479. But I could never quite figure out how
this would help (gunicorn crashing with a RuntimeError isn't obviously
better than gunicorn crashing with an ExceptionGroup).
I'm not sure I agree with the intuition that such user code definitely has a problem. Irit tried to give an example using AttributeError, and while that particular example is easily countered, I think it's not as straightforward as you state. For example there could be a framework exception that we know statically will not be wrapped in ExceptionGroup. Basically, if we have some code that can either raise some exceptions of category A wrapped in ExceptionGroup, or a bare exception of category B (without overlap between the categories), writing code that catches category B using `except B:` should not affect the propagation of ExceptionGroup.
Or what about code that uses this idiom?
```
try:
. . .
except Exception:
. . .
raise
```
There should be no need to switch this to `except *` just to prevent the ExceptionGroup from becoming a RuntimeError.
== NEW IDEA THAT MAYBE SOLVES BOTH PROBLEMS ==
Proposal:
- any time an unwinding ExceptionGroup encounters a traditional
try/except, then it gets replaced with a RuntimeError whose __cause__
is set to the original ExceptionGroup and whose first traceback entry
points to the offending try/except block
- CUTE BIT I ONLY JUST THOUGHT OF: this substitution happens right
*before* we start evaluating 'except' clauses for this try/except
So for example:
If an ExceptionGroup hits an 'except Exception': The ExceptionGroup is
replaced by a RuntimeError. RuntimeError is an Exception, so the
'except Exception' clause catches it. And presumably logs it or
something. This way your log contains both a notification that you
might want to switch to except* (from the RuntimeError), *along with*
the full original exception details (from the __cause__ attribute). If
it was an ExceptionGroup(KeyboardInterrupt), then it still gets caught
and that's not so great, but at least you get the RuntimeError to
point out that something has gone wrong and tell you where?
If an ExceptionGroup(ValueError) hits an 'except ValueError': it
doesn't get caught, *but* a RuntimeError keeps propagating out to tell
you you have a problem. And when that RuntimeError eventually hits the
top of your program or ends up in your webserver logs or whatever,
then the RuntimeError's traceback will point you to the 'except
ValueError' that needs to be fixed.
If you write 'except ExceptionGroup': this clause is a no-op that will
never execute, because it's impossible to still have an ExceptionGroup
when we start matching 'except' clauses. (We could additionally emit a
diagnostic if we want.)
If you write bare 'except:', or 'except BaseException': the clause
always executes (as before), but they get the RuntimeError instead of
the ExceptionGroup. If you really *wanted* the ExceptionGroup, you can
retrieve it from the __cause__ attribute. (The only case I can think
of where this would be useful is if you're writing code that has to
straddle both old and new Python versions *and* wants to do something
clever with ExceptionGroups. I think this would happen if you're
implementing Trio, or implementing a higher-level backport library for
catching ExceptionGroups, something like that. So this only applies to
like half a dozen users total, but they are important users :-).)
This is all very clever, but something feels off with it. Perhaps it improves this:
```
try:
raise ExceptionGroup(ValueError) # Hidden in more complex code
except ValueError:
. . .
```
(Though how? The RuntimeError wouldn't be caught.)
But I feel strongly that if the ExceptionGroup doesn't wrap any exceptions that would be caught by the except clauses of a given try statement, the ExceptionGroup should bubble through unchanged. As you said yourself, "gunicorn crashing with a RuntimeError isn't obviously better than gunicorn crashing with an ExceptionGroup."
In fact, if you're happy with RuntimeError here, making ExceptionGroup inherit from Exception (instead of BaseException) would do just as well -- after all RuntimeError is pretty arbitrary (in fact, it's wrong, because the problem is a static bug in the code, not something that went wrong at run time).
Let's see how this compares to the alternatives. First let's define three key examples.
Example 1:
```
try:
raise ExceptionGroup(ValueError)
except Exception:
. . .
```
Example 2:
```
try:
raise ExceptionGroup(ValueError)
except ValueError:
. . .
```
. . .
Now let's tabulate the outcome for each of the proposals. Does the except clause get executed?
(*) Spoils things for outer frames that use `except *` correctly.
So example 2 mostly doesn't help discriminating between examples. What we arguably want there is `Yes`, but that would require adding more logic to the exception matching code (and there are other reasons why we don't want that).
I feel that the intended result for Example 3 is No (because CancelledError inherits directly from BaseException), so if we want a Yes for Example 1, BaseExceptionGroup + ExceptionGroup still looks more attractive than any other proposal.