[Zac-HD]

When a cancel scope around an async with on an @as_safe_channel generator fires while the generator's own code is running, the body and the generator each raise a Cancelled. Which one propagated was whichever reached the nursery first, which is effectively random. If the generator had already raised and was still awaiting in its cleanup, its Cancelled carries that exception as __context__, so the exception was missing from the traceback about half the time (under a TooSlowError, say). A plain async for over the same generator always keeps it.

This remembers a Cancelled that comes out of agen.__anext__(), and raises that one when the nursery's group holds nothing but Cancelled. A group raised by the body still propagates whole, as before. The new test uses a shield in the generator's cleanup to pin the order that lost the exception, so it fails reliably without the fix.

Why not change raise_single_exception_from_group to prefer a Cancelled with a __context__? That would pick the background task's Cancelled in ordinary timeouts where the generator is parked at a yield. That Cancelled has a WouldBlock context, because Semaphore.acquire parks inside except WouldBlock, and picking it would drop the consumer's frames from the traceback.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (654ba84) to head (61696cc).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@               Coverage Diff               @@
##                 main        #3554   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             128          128           
  Lines           19475        19497   +22     
  Branches         1323         1325    +2     
===============================================
+ Hits            19475        19497   +22     

Files with missing lines	Coverage Δ
src/trio/_channel.py	100.00000% <100.00000%> (ø)
src/trio/_tests/test_channel.py	100.00000% <100.00000%> (ø)
src/trio/_util.py	100.00000% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[A5rocks]

Hm I'm not that happy about Cancelled actually having state and it being important which one we pick. More generally I don't see why there couldn't be two Cancelleds that matter at this point?

Should we make a new Cancelled where the context is a synthesized ExceptionGroup? (I guess it's also true people have been poking at __context__ so maybe that's a bad idea. And I just came up with the idea so it probably doesn't work ^^;)

[Zac-HD]

Hm I'm not that happy about Cancelled actually having state and it being important which one we pick. More generally I don't see why there couldn't be two Cancelleds that matter at this point?

Yeah, I don't like it either, but it happens to be the case that a Cancelled coming out of agen.__anext__() can have important debugging info in the .__context__, and so I want to preserve that.

If there's another Cancelled, it has to be coming from the body of @as_safe_channel, and that can only be because of an external timeout, which I think means that there can't be interesting context attached to it.

[A5rocks]

Nevermind, I forgot we did some work to make sure only the generator or the body is running at once! I guess it's kind of annoying that this doesn't compose well (only works with Cancelled, not with groups of Cancelled), but tbh I don't think any variant of this idea would compose well :^).