-
-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace MultiError with BaseExceptionGroup #2213
Changes from all commits
31706e8
743354b
6fdd0fe
13de11d
9ecca9b
21757c8
61a1646
ca05efb
0b159e8
e9aaabc
83e054c
370e40f
e2fbec0
95a7b9e
745f3cf
bc043aa
797c234
8a09778
c1200de
30358d1
54eb0ce
f1cbcf7
0f5dc79
36e0eba
20df5d8
6eeb137
2dc1c64
e7faa46
c41517a
50963c6
ba17fbb
8911e0a
dfd084c
b3c79c8
81feff2
bcb442b
ab78e02
9abddfa
efc8b01
9a4f512
6cb5ad9
ee245a0
1e36f8c
034bd32
80a751f
9eb2d1c
fd56492
af6233b
a4876bc
5c09f44
c5af0a4
93c81f0
b4791f8
70bb109
1c403b1
a28bfac
7694592
161c0ab
226184f
748ceb8
dfdda5b
d49abbb
a877191
e8a1b60
4bd09bc
1101ead
cd11213
7088be4
5bc2bd9
e87de6c
6b5e4f6
9ff7a45
2140b70
1da1af4
f9e11fc
1d99b23
2591808
afd798f
8c77459
b621e70
656fad4
6e86da3
e29c00f
16c19b6
c35fccc
7b7acb9
3b97ce1
1075ff0
458f5e2
0f39b3f
dc35213
f9eff8e
30b8f3b
232688b
0b8e908
878af56
cc7d76a
a20e2d8
1aba33e
0c283b7
0364b48
bb31ed6
2fd8309
80889b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -641,7 +641,7 @@ crucial things to keep in mind: | |
|
||
* Any unhandled exceptions are re-raised inside the parent task. If | ||
there are multiple exceptions, then they're collected up into a | ||
single :exc:`MultiError` exception. | ||
single :exc:`BaseExceptionGroup` or :exc:`ExceptionGroup` exception. | ||
|
||
Since all tasks are descendents of the initial task, one consequence | ||
of this is that :func:`run` can't finish until all tasks have | ||
|
@@ -687,6 +687,8 @@ You might wonder why Trio can't just remember "this task should be cancelled in | |
|
||
If you want a timeout to apply to one task but not another, then you need to put the cancel scope in that individual task's function -- ``child()``, in this example. | ||
|
||
.. _exceptiongroups: | ||
|
||
Errors in multiple child tasks | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
|
@@ -709,18 +711,102 @@ limitation. Consider code like:: | |
|
||
``broken1`` raises ``KeyError``. ``broken2`` raises | ||
``IndexError``. Obviously ``parent`` should raise some error, but | ||
what? In some sense, the answer should be "both of these at once", but | ||
in Python there can only be one exception at a time. | ||
what? The answer is that both exceptions are grouped in an :exc:`ExceptionGroup`. | ||
:exc:`ExceptionGroup` and its parent class :exc:`BaseExceptionGroup` are used to | ||
encapsulate multiple exceptions being raised at once. | ||
|
||
To catch individual exceptions encapsulated in an exception group, the ``except*`` | ||
clause was introduced in Python 3.11 (:pep:`654`). Here's how it works:: | ||
|
||
try: | ||
async with trio.open_nursery() as nursery: | ||
nursery.start_soon(broken1) | ||
nursery.start_soon(broken2) | ||
except* KeyError as excgroup: | ||
for exc in excgroup.exceptions: | ||
... # handle each KeyError | ||
except* IndexError as excgroup: | ||
for exc in excgroup.exceptions: | ||
... # handle each IndexError | ||
|
||
If you want to reraise exceptions, or raise new ones, you can do so, but be aware that | ||
exceptions raised in ``except*`` sections will be raised together in a new exception | ||
group. | ||
|
||
But what if you can't use ``except*`` just yet? Well, for that there is the handy | ||
exceptiongroup_ library which lets you approximate this behavior with exception handler | ||
callbacks:: | ||
|
||
from exceptiongroup import catch | ||
|
||
def handle_keyerrors(excgroup): | ||
for exc in excgroup.exceptions: | ||
... # handle each KeyError | ||
|
||
def handle_indexerrors(excgroup): | ||
for exc in excgroup.exceptions: | ||
... # handle each IndexError | ||
|
||
with catch({ | ||
KeyError: handle_keyerror, | ||
IndexError: handle_indexerror | ||
}): | ||
async with trio.open_nursery() as nursery: | ||
nursery.start_soon(broken1) | ||
nursery.start_soon(broken2) | ||
|
||
Trio's answer is that it raises a :exc:`MultiError` object. This is a | ||
special exception which encapsulates multiple exception objects – | ||
either regular exceptions or nested :exc:`MultiError`\s. To make these | ||
easier to work with, Trio installs a custom `sys.excepthook` that | ||
knows how to print nice tracebacks for unhandled :exc:`MultiError`\s, | ||
and it also provides some helpful utilities like | ||
:meth:`MultiError.catch`, which allows you to catch "part of" a | ||
:exc:`MultiError`. | ||
The semantics for the handler functions are equal to ``except*`` blocks, except for | ||
setting local variables. If you need to set local variables, you need to declare them | ||
inside the handler function(s) with the ``nonlocal`` keyword:: | ||
|
||
def handle_keyerrors(excgroup): | ||
nonlocal myflag | ||
myflag = True | ||
|
||
myflag = False | ||
with catch({KeyError: handle_keyerror}): | ||
async with trio.open_nursery() as nursery: | ||
nursery.start_soon(broken1) | ||
|
||
For reasons of backwards compatibility, nurseries raise ``trio.MultiError`` and | ||
``trio.NonBaseMultiError`` which inherit from :exc:`BaseExceptionGroup` and | ||
:exc:`ExceptionGroup`, respectively. Users should refrain from attempting to raise or | ||
catch the Trio specific exceptions themselves, and treat them as if they were standard | ||
:exc:`BaseExceptionGroup` or :exc:`ExceptionGroup` instances instead. | ||
|
||
"Strict" versus "loose" ExceptionGroup semantics | ||
++++++++++++++++++++++++++++++++++++++++++++++++ | ||
|
||
Ideally, in some abstract sense we'd want everything that *can* raise an | ||
`ExceptionGroup` to *always* raise an `ExceptionGroup` (rather than, say, a single | ||
`ValueError`). Otherwise, it would be easy to accidentally write something like ``except | ||
ValueError:`` (not ``except*``), which works if a single exception is raised but fails to | ||
catch _anything_ in the case of multiple simultaneous exceptions (even if one of them is | ||
a ValueError). However, this is not how Trio worked in the past: as a concession to | ||
practicality when the ``except*`` syntax hadn't been dreamed up yet, the old | ||
``trio.MultiError`` was raised only when at least two exceptions occurred | ||
simultaneously. Adding a layer of `ExceptionGroup` around every nursery, while | ||
theoretically appealing, would probably break a lot of existing code in practice. | ||
|
||
Therefore, we've chosen to gate the newer, "stricter" behavior behind a parameter | ||
called ``strict_exception_groups``. This is accepted as a parameter to | ||
:func:`open_nursery`, to set the behavior for that nursery, and to :func:`trio.run`, | ||
to set the default behavior for any nursery in your program that doesn't override it. | ||
|
||
* With ``strict_exception_groups=True``, the exception(s) coming out of a nursery will | ||
always be wrapped in an `ExceptionGroup`, so you'll know that if you're handling | ||
single errors correctly, multiple simultaneous errors will work as well. | ||
|
||
* With ``strict_exception_groups=False``, a nursery in which only one task has failed | ||
will raise that task's exception without an additional layer of `ExceptionGroup` | ||
wrapping, so you'll get maximum compatibility with code that was written to | ||
support older versions of Trio. | ||
|
||
To maintain backwards compatibility, the default is ``strict_exception_groups=False``. | ||
The default will eventually change to ``True`` in a future version of Trio, once | ||
Python 3.11 and later versions are in wide use. | ||
|
||
.. _exceptiongroup: https://pypi.org/project/exceptiongroup/ | ||
|
||
Spawning tasks without becoming a parent | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
@@ -837,104 +923,6 @@ The nursery API | |
See :meth:`~Nursery.start`. | ||
|
||
|
||
Working with :exc:`MultiError`\s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this section be updated for ExceptionGroups instead of being removed? I know it's not really "our" feature anymore, but the existing docs on ExceptionGroups are pretty scant since they're not in a released Python, and we're adopting them as part of our API so it would be nice not to lose friendliness in the transition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there something wrong with the documentation I just added above which specifically shows how to use both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's nothing wrong with it, but it does feel like a downgrade when the previous set of docs had multiple examples, describing more edge cases, exception chaining, output, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I'll restore and update the docs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took the time to evaluate the examples from the removed section, but since the catching API in the backport works in such a different way, I'm not sure what there is to document. The backport doesn't provide any functionality that the |
||
++++++++++++++++++++++++++++++++ | ||
|
||
.. autoexception:: MultiError | ||
|
||
.. attribute:: exceptions | ||
|
||
The list of exception objects that this :exc:`MultiError` | ||
represents. | ||
|
||
.. automethod:: filter | ||
|
||
.. automethod:: catch | ||
:with: | ||
|
||
Examples: | ||
|
||
Suppose we have a handler function that discards :exc:`ValueError`\s:: | ||
|
||
def handle_ValueError(exc): | ||
if isinstance(exc, ValueError): | ||
return None | ||
else: | ||
return exc | ||
|
||
Then these both raise :exc:`KeyError`:: | ||
|
||
with MultiError.catch(handle_ValueError): | ||
raise MultiError([KeyError(), ValueError()]) | ||
|
||
with MultiError.catch(handle_ValueError): | ||
raise MultiError([ | ||
ValueError(), | ||
MultiError([KeyError(), ValueError()]), | ||
]) | ||
|
||
And both of these raise nothing at all:: | ||
|
||
with MultiError.catch(handle_ValueError): | ||
raise MultiError([ValueError(), ValueError()]) | ||
|
||
with MultiError.catch(handle_ValueError): | ||
raise MultiError([ | ||
MultiError([ValueError(), ValueError()]), | ||
ValueError(), | ||
]) | ||
|
||
You can also return a new or modified exception, for example:: | ||
|
||
def convert_ValueError_to_MyCustomError(exc): | ||
if isinstance(exc, ValueError): | ||
# Similar to 'raise MyCustomError from exc' | ||
new_exc = MyCustomError(...) | ||
new_exc.__cause__ = exc | ||
return new_exc | ||
else: | ||
return exc | ||
|
||
In the example above, we set ``__cause__`` as a form of explicit | ||
context chaining. :meth:`MultiError.filter` and | ||
:meth:`MultiError.catch` also perform implicit exception chaining – if | ||
you return a new exception object, then the new object's | ||
``__context__`` attribute will automatically be set to the original | ||
exception. | ||
|
||
We also monkey patch :class:`traceback.TracebackException` to be able | ||
to handle formatting :exc:`MultiError`\s. This means that anything that | ||
formats exception messages like :mod:`logging` will work out of the | ||
box:: | ||
|
||
import logging | ||
|
||
logging.basicConfig() | ||
|
||
try: | ||
raise MultiError([ValueError("foo"), KeyError("bar")]) | ||
except: | ||
logging.exception("Oh no!") | ||
raise | ||
|
||
Will properly log the inner exceptions: | ||
|
||
.. code-block:: none | ||
|
||
ERROR:root:Oh no! | ||
Traceback (most recent call last): | ||
File "<stdin>", line 2, in <module> | ||
trio.MultiError: ValueError('foo',), KeyError('bar',) | ||
|
||
Details of embedded exception 1: | ||
|
||
ValueError: foo | ||
|
||
Details of embedded exception 2: | ||
|
||
KeyError: 'bar' | ||
|
||
|
||
.. _task-local-storage: | ||
|
||
Task-local storage | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any documentation about the exact semantics of
catch
? It's not in 3.11 so you can't just defer to the standard library docs.exceptiongroup.readthedocs.io
seems to exist but it's only a stub. It would be nice to be able to link to something definitive here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the README. Let me know if that's not enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exact semantics have been left undocumented there? It would be easier for me to fill in the holes if I knew what they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT basically the sole documentation for catch() semantics is
It is implied that catch() is a drop-in substitute for
except*
, but it's not:except*
passes an ExceptionGroup (ie the result of split()) whilecatch()
calls the handler once for each "leaf" exception (which is a footgun -- the tracebacks on the passed exception objects are incomplete, because part of the traceback is still attached to the ExceptionGroup which you don't have)except*
understands subclassing, butcatch()
doesn't, i.e.,catch({Exception: foo})
will not catch a ValueErrorI think you either need to make
catch()
a total drop-in replacement forexcept*
, whose semantics can be described totally accurately by reference to the 3.11+except*
semantics plus a mechanical syntactic transformation; or else the semantics ofcatch()
need to be documented in much more detail, so that the above differences are obvious rather than having to be determined through experimentation.Your PR is deprecating all of Trio's existing facilities for doing this stuff. The only supported way to catch part of a multi-error at all after your change is to use the
exceptiongroup.catch()
backport (at least until 3.11 comes out in several more months). I'm sorry to be picky about this but I think if we're only giving our users one option it really needs to be rock-solid and extremely clear, and I don't think its current state lives up to that standard.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the implementation of
exceptiongroup.catch()
to match that ofexcept*
and expanded the documentation on its exact semantics in 1.0.0rc6. The idea was indeed that it should be a drop-in replacement (to the extend that the syntax allows, obviously).