Coping with "finally footguns": avoiding trio.Cancelled exc masking as best we can..

[goodboy]

As critiqued heavily (by me) over the past few months (after
discovering this issue during work on another dependent project) in
the trio chat room, i was able to hack together a near term soln
which at least resolves the bulk of critical potential issues (the
SC-cross-process semantics breaking kind) within the tractor
runtime - namely ensuring error propagation is maintained with
a (remote) child-task started with Portal.open_context() .

---

Summary of underlying "issue" in trio

It's actually been documented and discussed for much longer then
I had thought and is most simply detailed in this comment
which contains the following snippet.

  async def main():
     with trio.CancelScope() as scope:
         try:
             raise RuntimeError("Oooops")
         finally:
             scope.cancel()
             await trio.sleep(0)

Generally speaking python-trio/trio#455
seems to be the primary spot for following all this.

TLDR,

if you exec a checkpoint in a cancelled scope from some
exception handling block, any original propagating exc (in this case
an RTE) will be replaced (what I call "masked") due to normal python
exc control-flow semantics; the issue is that since trio.Cancelled
is often handled "gracefully", i.e. it is swallowed by some parent
scope which was .cancel_called, the original exc will never be
raised..

As you can imagine this is very problematic if you're trying to
relay errors from from one task to another when they're scheduled in
different processes!

Bp

---

Some other related discussions/resources I've found from pretty old
patches/issues in both trio core and/or surrounding ecosys,

• pytest-trio rejected patch: Fix exception swallow in fixture due to parent cancel scope python-trio/pytest-trio#77 (comment)

  • njs counter arg: Fix exception swallow in fixture due to parent cancel scope python-trio/pytest-trio#77 (comment)

• relevant flake8-async linter rules for which apply to for all asyncIO:

  • patch: Warn on finally: cancel_scope.cancel() python-trio/flake8-async#365
  • 102: https://flake8-async.readthedocs.io/en/latest/rules.html#async102
  • 120: https://flake8-async.readthedocs.io/en/latest/rules.html#async120

---

What lands here

• a WIP soln to this problem for the purposes of our
  ._rpc._invoke() machinery, namely ensuring we don't suppress
  @context-endpoint originating exceptions, those raised from our
  "user's code":

  • a new .trionics._taskc.maybe_raise_from_masking_exc() helper
    to, at least, warn about via the new Exception.add_note(), but
    also optionally directly (re)raise a(ny) "masked exception(s)",
    namely by default those masked by a trio.Cancelled (per all the
    preface above).

  • direct use in our ._rpc._invoke()-child side impl to avoid said
    trio.Cancelled masking, working kinda hard to avoid users
    shooting their toes (see test suite parametrizations).

• a minor suite which audits the correctness of the unmasker util-fn:
  tests_trioisms::test_gatherctxs_with_memchan_breaks_multicancelled.

• a more comprehensive suite which audits that we do not suppress
  exceptions raised by user code:
  test_remote_exc_relay::test_unmasked_remote_exc.

---

Deeper explanation why this is a bigger deal for tractor's RPC

Remote exception "masking" is a bigger deal in tractor's error relay
sys/semantics for a couple reasons,

1. since there is no current way to override/customize
   trio.Cancelled and since that forces us to use
   CancelScope.cancel() and a cs.cancelled_caught check to determine if
   a side of a task-RPC-Context was gracefully cancelled (whether
   from a remote or local request), we require that,

   • a task-cancellation (taskc) has very special meaning when
     that taskc-requesting-scope is the Context._scope - the cs
     used in the RPC mgmt part of the runtime to relay/accept remote
     cancellations; this means knowing what scope is the requester of
     a taskc it that much more critical to ensure remote vs. local
     cancels are distinct.

   • when a remote task error's that error should never be masked
     bc otherwise (per the previous bullet) it can then result in
     a critical-remote-error masquerading as
     remote-graceful-cancellation, particularly if there is
     a Portal.cancel_actor()/Context.cancel() call or SIGINT
     emission which arrives out-of-band (meaning not called by a task
     in the Context pair) and propagates across peer actors at the
     same time as said critical-remote-error being masked.

• you can see the various complex and often subtle use cases in our
  • test_context_stream_semantics and,
  • test_inter_peer_cancellation suites.

2. since actors generally share no common state it is much more
   difficult to determine the source of a remote error or cancel
   request including (for the latter) there being multiple "runtime
   scopes of cancellation" as it were (not to overload the scope
   term but..), meaning a cancel request can happen at any of the
   multiple layers,

   • OS via signal (osc),
   • actor via Portal.cancel_actor() (actorc),
   • RPC via Context.cancel() (ctxc),
   • a normal trio.CancelScope (taskc)
   • any of these but relayed through inter-acting code at the python
     function level (for example, in a more complex actor tree setup
     this is a common case) here some remote peer has errored/cancelled and
     that condition is relayed through various multi-actor-spanning
     trio.Task-trees which are "distributed" over the actors/processes/sub-ints
     yet are SC-adhering in terms of task-tree/cancel-scope supervision semantics.

---