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. 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. 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.
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. 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]`.
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)'.
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__... -n -- Nathaniel J. Smith -- https://vorpus.org