python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
5.98k stars 325 forks source link

Document strict ExceptionGroups as the natural default #2984

Closed Zac-HD closed 2 months ago

Zac-HD commented 2 months ago

Closes https://github.com/python-trio/trio/issues/2929, by ensuring that our docs present strict_exception_groups=True as the natural default (and it is indeed the default in current Trio, as well as the only behavior of asyncio and anyio).

Following discussion on that issue, I've also written up some notes on three ways of handling ExceptionGroups before raising to user code: (1) defer-to, (2) single-error, optionally handling background and foreground tasks differently, and (3) simply letting it propagate to the caller.

2b might actually be a neat option to add to trio.open_nusery() itself, but I'd want to see a proof-of-concept judged useful downstream before we committed to supporting it there.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.64%. Comparing base (564907b) to head (4440706). Report is 7 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2984 +/- ## ========================================== - Coverage 99.64% 99.64% -0.01% ========================================== Files 117 117 Lines 17594 18101 +507 Branches 3171 3362 +191 ========================================== + Hits 17532 18036 +504 - Misses 43 45 +2 - Partials 19 20 +1 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/2984?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio) | Coverage Δ | | |---|---|---| | [src/trio/\_core/\_run.py](https://app.codecov.io/gh/python-trio/trio/pull/2984?src=pr&el=tree&filepath=src%2Ftrio%2F_core%2F_run.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3J1bi5weQ==) | `99.57% <ø> (-0.10%)` | :arrow_down: | | [src/trio/\_subprocess.py](https://app.codecov.io/gh/python-trio/trio/pull/2984?src=pr&el=tree&filepath=src%2Ftrio%2F_subprocess.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3N1YnByb2Nlc3MucHk=) | `100.00% <ø> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/python-trio/trio/pull/2984/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio)
clint-lawrence commented 2 months ago

I'm not sure if this is directly relevant to these docs or not.

While working on #2972 I came across a subtle different between trio and asyncio. In asyncio, if a SystemExit or KeyboardInterrupt is what leads to a task group being cancelled, that individual exception will be re-raised, rather than being wrapped into an exception group. My understanding is that in trio, a nursery (with strict_exception_groups=True) will wrap those into a exception group.

Highlighting that difference would be nice, but I'm not sure where that fits best in the docs.

Zac-HD commented 2 months ago

In asyncio, if a SystemExit or KeyboardInterrupt is what leads to a task group being cancelled, that individual exception will be re-raised, rather than being wrapped into an exception group. My understanding is that in trio, a nursery (with strict_exception_groups=True) will wrap those into a exception group.

That matches my understanding on the Trio side. I think this should probably be a separate issue, where we can decide whether to match asyncio (probably yes?) and then implement + document whatever we do.

I've reached my timebox for this PR though, so if people agree it's an improvement on the status quo I'll merge it 🙂

A5rocks commented 2 months ago

Should the deprecation message for strict_exception_groups=False link to this handling exception groups section? https://github.com/python-trio/trio/blob/59b2eaca688510d478b0ed072f3d71b2938d96d9/src/trio/_core/_run.py#L1011

(if that gets updated, please also update the version number to be 0.25.0!!)

A5rocks commented 2 months ago

Oops, I totally missed this:

I've reached my timebox for this PR though, so if people agree it's an improvement on the status quo I'll merge it 🙂

I'll add commits then! (was about to ask if I could without causing any sort of merge conflict with local changes)