
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: . . . ``` Example 3: ``` try: raise ExceptionGroup(asyncio.CancelledError) except Exception: . . . ``` Now let's tabulate the outcome for each of the proposals. Does the except clause get executed? ``` Example 1 Example 2 Example 3 Current PEP No No No BaseExceptionGroup + ExceptionGroup Yes No No Inherit from Exception Yes No Yes Change to RuntimeError Yes No(*) Yes ``` (*) 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. -- --Guido van Rossum (python.org/~guido) *Pronouns: he/him **(why is my pronoun here?)* <http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...>