Always `Cancelled`-unmask ctx endpoint excs
To resolve the recently added and failing `test_remote_exc_relay::test_unmasked_remote_exc`: never allow `trio.Cancelled` to mask an underlying user-code exception, ever. Our first real-world (runtime internal) use case for the new `.trionics.maybe_raise_from_masking_exc()` such that the failing test now passes with an properly relayed remote RTE unmasking B) Details, - flip the `Context._scope_nursery` to the default strict-eg behaviour and instead stack its outer scope with a `.trionics.collapse_eg()`. - wrap the inner-most scope (after `msgops.maybe_limit_plds()`) with a `maybe_raise_from_masking_exc()` to ensure user-code errors are never masked by `trio.Cancelled`s. Some err-reporting refinement, - always capture any `scope_err` from the entire block for debug purposes; report it in the `finally` block's log. - always capture any suppressed `maybe_re`, output from `ctx.maybe_raise()`, and `log.cancel()` report it.moar_eg_smoothing
Tyler Goodlet
parent
bad42734db
commit
4bc443ccae
|
|
@ -37,6 +37,7 @@ import warnings
|
|||
|
||||
import trio
|
||||
from trio import (
|
||||
Cancelled,
|
||||
CancelScope,
|
||||
Nursery,
|
||||
TaskStatus,
|
||||
|
|
@ -52,10 +53,14 @@ from ._exceptions import (
|
|||
ModuleNotExposed,
|
||||
MsgTypeError,
|
||||
TransportClosed,
|
||||
is_multi_cancelled,
|
||||
pack_error,
|
||||
unpack_error,
|
||||
)
|
||||
from .trionics import (
|
||||
collapse_eg,
|
||||
is_multi_cancelled,
|
||||
maybe_raise_from_masking_exc,
|
||||
)
|
||||
from .devx import (
|
||||
debug,
|
||||
add_div,
|
||||
|
|
@ -616,32 +621,40 @@ async def _invoke(
|
|||
# -> the below scope is never exposed to the
|
||||
# `@context` marked RPC function.
|
||||
# - `._portal` is never set.
|
||||
scope_err: BaseException|None = None
|
||||
try:
|
||||
tn: trio.Nursery
|
||||
rpc_ctx_cs: CancelScope
|
||||
async with (
|
||||
trio.open_nursery(
|
||||
strict_exception_groups=False,
|
||||
# ^XXX^ TODO? instead unpack any RAE as per "loose" style?
|
||||
|
||||
) as tn,
|
||||
msgops.maybe_limit_plds(
|
||||
ctx=ctx,
|
||||
spec=ctx_meta.get('pld_spec'),
|
||||
dec_hook=ctx_meta.get('dec_hook'),
|
||||
),
|
||||
):
|
||||
ctx._scope_nursery = tn
|
||||
rpc_ctx_cs = ctx._scope = tn.cancel_scope
|
||||
task_status.started(ctx)
|
||||
|
||||
# TODO: better `trionics` tooling:
|
||||
# TODO: better `trionics` primitive/tooling usage here!
|
||||
# -[ ] should would be nice to have our `TaskMngr`
|
||||
# nursery here!
|
||||
# -[ ] payload value checking like we do with
|
||||
# `.started()` such that the debbuger can engage
|
||||
# here in the child task instead of waiting for the
|
||||
# parent to crash with it's own MTE..
|
||||
#
|
||||
tn: Nursery
|
||||
rpc_ctx_cs: CancelScope
|
||||
async with (
|
||||
collapse_eg(),
|
||||
trio.open_nursery() as tn,
|
||||
msgops.maybe_limit_plds(
|
||||
ctx=ctx,
|
||||
spec=ctx_meta.get('pld_spec'),
|
||||
dec_hook=ctx_meta.get('dec_hook'),
|
||||
),
|
||||
|
||||
# XXX NOTE, this being the "most embedded"
|
||||
# scope ensures unasking of the `await coro` below
|
||||
# *should* never be interfered with!!
|
||||
maybe_raise_from_masking_exc(
|
||||
tn=tn,
|
||||
unmask_from=Cancelled,
|
||||
) as _mbme, # maybe boxed masked exc
|
||||
):
|
||||
ctx._scope_nursery = tn
|
||||
rpc_ctx_cs = ctx._scope = tn.cancel_scope
|
||||
task_status.started(ctx)
|
||||
|
||||
# invoke user endpoint fn.
|
||||
res: Any|PayloadT = await coro
|
||||
return_msg: Return|CancelAck = return_msg_type(
|
||||
cid=cid,
|
||||
|
|
@ -744,38 +757,48 @@ async def _invoke(
|
|||
BaseException,
|
||||
trio.Cancelled,
|
||||
|
||||
) as scope_error:
|
||||
) as _scope_err:
|
||||
scope_err = _scope_err
|
||||
if (
|
||||
isinstance(scope_error, RuntimeError)
|
||||
and scope_error.args
|
||||
and 'Cancel scope stack corrupted' in scope_error.args[0]
|
||||
isinstance(scope_err, RuntimeError)
|
||||
and
|
||||
scope_err.args
|
||||
and
|
||||
'Cancel scope stack corrupted' in scope_err.args[0]
|
||||
):
|
||||
log.exception('Cancel scope stack corrupted!?\n')
|
||||
# debug.mk_pdb().set_trace()
|
||||
|
||||
# always set this (child) side's exception as the
|
||||
# local error on the context
|
||||
ctx._local_error: BaseException = scope_error
|
||||
ctx._local_error: BaseException = scope_err
|
||||
# ^-TODO-^ question,
|
||||
# does this matter other then for
|
||||
# consistentcy/testing?
|
||||
# |_ no user code should be in this scope at this point
|
||||
# AND we already set this in the block below?
|
||||
|
||||
# if a remote error was set then likely the
|
||||
# exception group was raised due to that, so
|
||||
# XXX if a remote error was set then likely the
|
||||
# exc group was raised due to that, so
|
||||
# and we instead raise that error immediately!
|
||||
ctx.maybe_raise()
|
||||
maybe_re: (
|
||||
ContextCancelled|RemoteActorError
|
||||
) = ctx.maybe_raise()
|
||||
if maybe_re:
|
||||
log.cancel(
|
||||
f'Suppressing remote-exc from peer,\n'
|
||||
f'{maybe_re!r}\n'
|
||||
)
|
||||
|
||||
# maybe TODO: pack in come kinda
|
||||
# `trio.Cancelled.__traceback__` here so they can be
|
||||
# unwrapped and displayed on the caller side? no se..
|
||||
raise
|
||||
raise scope_err
|
||||
|
||||
# `@context` entrypoint task bookeeping.
|
||||
# i.e. only pop the context tracking if used ;)
|
||||
finally:
|
||||
assert chan.uid
|
||||
assert chan.aid
|
||||
|
||||
# don't pop the local context until we know the
|
||||
# associated child isn't in debug any more
|
||||
|
|
@ -802,6 +825,9 @@ async def _invoke(
|
|||
descr_str += (
|
||||
f'\n{merr!r}\n' # needed?
|
||||
f'{tb_str}\n'
|
||||
f'\n'
|
||||
f'scope_error:\n'
|
||||
f'{scope_err!r}\n'
|
||||
)
|
||||
else:
|
||||
descr_str += f'\n{merr!r}\n'
|
||||
|
|
|
|||
Loading…
Reference in New Issue