Recover from a 'foreign await' by throwing in the TypeError

[oremanj]

This improves debuggability (since the traceback points to the location of the error, rather than just the nursery whose task was at fault) and fixes #552 by ensuring we don't abandon the child tasks of the errant one.

[njsmith]

Nice!

• I think we have a subtle invariant that the send/throw function has to implemented in C, to make tracebacks work right. Probably worth mentioning this in the comment with all the invariants?

• Can we add a regression test with something like this code in it? Our code for recovering after a "foreign await" needs to be reworked #552 (comment)

[codecov]

Codecov Report

Merging #1178 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1178      +/-   ##
==========================================
+ Coverage   99.56%   99.57%   +<.01%     
==========================================
  Files         106      106              
  Lines       12776    12800      +24     
  Branches      981      983       +2     
==========================================
+ Hits        12721    12745      +24     
  Misses         35       35              
  Partials       20       20

Impacted Files	Coverage Δ
trio/_core/_run.py	99.72% <100%> (ø)	⬆️
trio/_core/tests/test_run.py	100% <100%> (ø)	⬆️
trio/tests/test_util.py	100% <0%> (ø)	⬆️
trio/_util.py	100% <0%> (ø)	⬆️

[oremanj]

Added the requested test. I don't think having send() written in C is a fundamental invariant; what in your model would require that? A Python implementation will add an extraneous traceback frame at each task boundary, but that seems like a reasonable accounting of what really happened. I tried running Trio's testsuite with _next_send_fn set to a Python wrapper around the C send(), and only one test failed (test_traceback_frame_removal which directly makes assertions about which frame comes first in the traceback).

[oremanj]

(Not sure what codecov is on about -- it looks green on the website.)

[njsmith]

Codecov is complaining that this line is "partially" covered:

    assert any(entry.name == "child_xyzzy" for entry in excinfo.traceback)

I guess it's something to do with the embedded loop – maybe because it always exits early?

I don't think having send() written in C is a fundamental invariant; what in your model would require that? A Python implementation will add an extraneous traceback frame at each task boundary, but that seems like a reasonable accounting of what really happened. I tried running Trio's testsuite with _next_send_fn set to a Python wrapper around the C send(), and only one test failed (test_traceback_frame_removal which directly makes assertions about which frame comes first in the traceback).

I guess I had two things in mind:

• We go to quite some trouble to only show user frames in user tracebacks, which is what that test is checking. I guess if for some reason we wanted to start supporting other methods besides CoroutineType.send and CoroutineType.throw, then we'd have to somehow update the traceback code?

• The KeyboardInterrupt protection logic is super subtle, and this method is a delicate moment: it needs to atomically transition from "trio internal mode" to "user mode". I guess any way of resuming the task has to have some atomic moment when it both puts the user's frame on the stack and also resumes the user's try/except blocks, even if you wrapped send/throw inside some Python wrapper, so maybe it's not an issue? But only using C functions here definitely makes the analysis simpler :-)