On Wed, Apr 21, 2021 at 3:26 PM Nathaniel Smith <
njs@pobox.com> wrote:
On Tue, Apr 20, 2021 at 3:15 AM Irit Katriel <iritkatriel@googlemail.com> wrote:
> On Tue, Apr 20, 2021 at 2:48 AM Nathaniel Smith <njs@pobox.com> wrote:
>>
>>
>> The problem is that most of the time, even if you're using concurrency
>> internally so multiple things *could* go wrong at once, only one thing
>> actually *does* go wrong. So it's unfortunate if some existing code is
>> prepared for a specific exception that it knows can be raised, that
>> exact exception is raised... and the existing code fails to catch it
>> because now it's wrapped in an EG.
>
> Yes, this was discussed at length on this list. Raising an exception group is an API-breaking change. If a function starts raising exception groups its callers need to be prepared for that. Realistically we think exception groups will be raised by new APIs. We tried and were unable to define exception group semantics for except that would be reasonable and backwards compatible. That's why we added except*.
Sure. This was in my list of reasons why the backwards compatibility
tradeoffs are forcing us into awkward compromises. I only elaborated
on it b/c in your last email you said you didn't understand why this
was a problem :-). And except* is definitely useful. But I think there
are options for 'except' that haven't been considered fully.
Do you have any suggestions, or are you just telling us to think harder? Because we've already thought as hard as we could and within all the constraints (backwards compatibility and otherwise) we just couldn't think of a better one.
Saying that you have to make a new API every time you start using
concurrency inside a function is extremely restrictive. The whole
point of structured concurrency (and structured programming in
general) is that function callers don't need to know about the control
flow decisions inside a function. So right now, the EG proposal is
like saying that if you every have a function that doesn't contain a
'for' loop, and then you want to add a 'for' loop, then you have to
define a whole new API instead of modifying the existing one.
That analogy doesn't fly. If "what exceptions can this raise" is not part of the function spec (so users who catch exceptions are already flying blind or guessing based on what they've seen happen in the past), then you can certainly switch to using EGs, and this *is* like adding a for-loop. But if the exceptions that a function raises *are* part of its spec, and users are catching them, then switching to concurrency (without catching and converting EGs in the function) *is* an API change, akin to taking a function that returns an int and changing it to (sometimes?) returning a list of ints.
I absolutely get why the proposal looks like that. I'm just making the
point that we should make sure we've exhausted all our options before
settling for that as a compromise.
But if you're not able to make constructive proposals I have to conclude that we *have* exhausted our options, and we should move on.
>> > It is easy enough to write a denormalize() function in traceback.py that constructs this from the current EG structure, if you need it (use the leaf_generator from the PEP). I'm not sure I see why we should trouble the interpreter with this.
>>
>> In the current design, once an exception is wrapped in an EG, then it
>> can never be unwrapped, because its traceback information is spread
>> across the individual exception + the EG tree around it. This is
>> confusing to users ("how do I check errno?"), and makes the design
>> more complicated (the need for topology-preserving .split(), the
>> inability to define a sensible EG.__iter__, ...). The advantage of
>> making the denormalized form the native form is that now the leaf
>> exceptions would be self-contained objects like they are now, so you
>> don't need EG nesting at all, and users can write intuitive code like:
>>
>> except OSError as *excs:
>> remainder = [exc for exc in excs if exc.errno != ...]
>> if remainder:
>> raise ExceptionGroup(remainder)
>
>
> We have this precise example in the PEP:
> match, rest = excs.split(lambda e: e.errno != ...)
>
> You use split() instead of iteration for that. split() preserves all __context__, __cause__ and __traceback__ information, on all leaf and non-leaf exceptions.
Well, yeah, I know, I'm the one who invented split() :-). My point was
to compare these two options: in the flat EG example, most Python
users could write and read that code without knowing anything except
"there can be multiple exceptions now". It's all old, well-known
constructs used in the obvious way.
For the .split() version, you have to write a lambda (which is allowed
to access parts of the exception object, but not all of it!), and use
this idiosyncratic method that only makes sense if you know about EG
tree structures. That's a lot more stuff that users have to understand
and hold in their head.
But it *is* a tree structure. The interior nodes of the tree have a meaning: they represent join points where exceptions were grouped together, and the selection process needs to traverse the tree.
Or here's another example. Earlier you suggested:
> If you are using concurrency internally and don't want to raise EGs externally, then surely you will catch EGs, select one of the exceptions to raise and throw away all the others
But with nested EGs, this is difficult, because you *can't* just pull
out one of the exceptions to raise. The leaf exceptions aren't
standalone objects; you need some obscure traceback manipulation to do
this. I guarantee that users will get this wrong, even if we provide
the tools, because the explanation about when and why you need the
tools is complicated and people won't internalize it.
With flat EGs, this is trivial: it's just `raise
ExceptionGroup[the_selected_exception_index]`.
But it's generally wrong to raise just one of the exceptions anyway, so you want to make it easier to get a wrong result?
I'd rather make this an option for asyncio.gather() or its TaskGroup equivalent, then the user doesn't have to do anything. gather() already has as its default behavior to raise the first exception it encounters, which seems good enough.
>> > For display purposes, it is probably nicer to look at a normalized traceback where common parts are not repeated.
>>
>> Yeah, I agree; display code would want to re-normalize before
>> printing. But now it's only the display code that needs to care about
>> figuring out shared parts of the traceback, rather than something that
>>
>> has to be maintained as an invariant everywhere.
>
> If you *do* want iteration, we show in the PEP how to write a leaf_generator() that gives you the leaf exceptions with their tracebacks (as a list of chunks). It is easy to copy the chunks into a single flat traceback. We didn't propose to add it to traceback.py yet because the use case is unclear but if people need it we're talking about 10-15 lines in traceback.py.
>
> So for your suggestion:
>
> Pros:
> 1. Those who want a denormalized traceback (if they exist) won't need to call traceback.denormalize().
>
> Cons:
> 1. A significant change in the interpreter that will make it less efficient (both time and space).
In terms of interpreter complexity, the straightforward implementation
would be pretty simple. Whenever the exception handling code goes to
write back a traceback to an exception's __traceback__ attribute,
check if the exception is an ExceptionGroup; if so, then loop over
each exception in the group and push a clone of the traceback onto
that exception's __traceback__. It takes code of course, everything
does, but I'd be surprised if it was more than, say, 20 lines of C. I
think that's a pretty good tradeoff to get a simpler user-level
language semantics. Python is supposed to fit in your head :-).
Time/space efficiency is interesting, because yeah, with the nested EG
approach then common parts of the traceback are stored once, while the
flat EG approach needs to store a separate copy for each exception.
But:
- The extra cost is 1 traceback object per common stack frame, per
exception. So the worst case is if you have, like, a 1000 exceptions
in a single group, and then that group travels together for 1000 stack
frames. But... is that realistic? Very few programs recurse down 1000
levels and *then* spawn a bunch of tasks. And very few programs have
1000 tasks that all fail simultaneously with different exceptions. And
even if that did happen, 1000*1000 = 1 million traceback objects take
~56 megabytes of RAM and ~90 ms to instantiate on my laptop, which is
not a big deal. [2]
- This is assuming the most simple/naive implementation, where we
simply duplicate every traceback entry onto every exception. If it
turns out to be unacceptable, there are lots of tricks we could use to
speed it up, e.g. by updating __traceback__ lazily or letting
traceback entry objects be shared between tracebacks.
[2] Estimated based on 'sys.getsizeof(tb_object)' and '%timeit
types.TracebackType(None, frame, 0, 0)'.
The real cost here is that we would need a new "TracebackGroup" concept, since the internal data structures and APIs keep the traceback chain and the exception object separated until the exception is caught. In our early design stages we actually explored this and the complexity of the data structures was painful. We eventually realized that we didn't need this concept at all, and the result is much clearer, despite what you seem to think.
> 2. Display code will need to normalize the traceback, which is much more complicated than denormalizing because you need to discover the shared parts.
>
> Am I missing something?
I think re-normalizing is very cheap and simple? It's just a common
prefix problem. Given a tree-in-progress and a traceback, walk down
each of them in parallel until you run out of matching entries, then
insert a branch node and append the rest of the traceback.
>> >
>> > It sounds like you want some way to enrich exceptions. This should be optional (we wouldn't want EG to require additional metadata for exceptions) so yeah, I agree it should sit on the leaf exception and not on the group. In that sense it's orthogonal to this PEP.
>>
>> Well, the extra metadata would specifically be at "join" points in the
>> traceback, which are a thing that this PEP is creating :-). And it's
>> useful for every user of EGs, since by definition, an EG is
>> multiplexing exceptions from multiple sources, so it's nice to tell
>> the user which sources those are.
>>
>> That said, you're right, if we want to handle this by defining a new
>> kind of traceback entry that code like Trio/asyncio/hypothesis can
>> manually attach to exceptions, then that could be written as a
>> separate-but-complementary PEP. In my original design, instead of
>> defining a new kind of traceback entry, I was storing this on the EG
>> itself, so that's why I was thinking about it needing to be part of
>> this PEP.
>
>
> You can also create an ExceptionGroup subclass with whatever extra data you want to include.
Unfortunately, that doesn't work either, because this is data that
should be included in traceback displays, similar to __cause__ and
__context__...
Fortunately in the flat design there wouldn't be a need for such metadata anyway...