Tpt-tolerance: more lowlevel trio CRE/BRE -> TransportClosed translations

[goodboy]

More or less finishing-up/refining the new tractor.ipc exc
machinery introduced in #375 such that all "lowlevel trio" source
exception types are re-raise-propagated out of the
.ipc.Channel/.MsgTransport layers as TransportClosed, which is
generally handled with more tolerance (at least for expected graceful
comms disconnect cases) throughout the higher-level RPC machinery.

This patch includes tolerating now both,

• trio.BrokenResourceError (BRE)
• trio.ClosedResourceError (CRE)

raised out of the underlying transport protocol interfaces similarly
depending on its exc.message[0] content, special cases are remapped
as TransportClosed (tpt-closed) with appropriate explanatory msgs.

---

Summary of tweaks

• adjustments to various test suites to match, namely handling
  tpt-closed where we were handling CRE/BRE previous.
• removal of CRE/BRE from various internal layers which generally
  call-into/reside-on-top-of the .ipc primitives.
• various doc-str updates in surrounding mods i needed to read or
  tweak as part of the above.
• ofc styling adjustements inline with my recent prefs for py code,
  namely multi-line styling for fn-sigs and multi-expr-logics
  for if branching.

[Copilot]

Pull request overview

This PR tightens IPC transport fault-tolerance by translating additional low-level trio transport exceptions into TransportClosed, and adjusts RPC error-relay/debugging behavior around transport disconnects.

Changes:

• Re-enable translation of a specific trio.ClosedResourceError (“this socket was already closed”) into TransportClosed in the transport send path.
• Add an explicit TransportClosed handler in _errors_relayed_via_ipc() and refine debugger-entry gating/logging.
• Change _invoke()’s hide_tb default and add notes/comments around final result sending.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
tractor/ipc/_transport.py	Adds another trio-exception → TransportClosed translation during send() failures.
tractor/_rpc.py	Updates RPC error relay to treat transport disconnects specially; tweaks debugger-entry behavior and changes _invoke() defaults/comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment suggests Channel.send() “already doesn't raise on tpt-closed”, but Channel.send() re-raises TransportClosed after logging (see tractor/ipc/_chan.py). This note is misleading and may cause future changes to incorrectly assume final result sends are safe from disconnects; consider correcting/removing the comment or clarifying which layer is expected to suppress TransportClosed (if any).

Suggested change

	await chan.send(return_msg)
	# ?TODO, remove the below since .send() already
	# doesn't raise on tpt-closed?
	# try:
	# await chan.send(return_msg)
	# except TransportClosed:
	# log.exception(
	# f"Failed send final result to 'parent'-side of IPC-ctx!\n"
	# f'\n'
	# f'{chan}\n'
	# f'Channel already disconnected ??\n'
	# f'\n'
	# f'{pretty_struct.pformat(return_msg)}'
	# )
	# # ?TODO? will this ever be true though?
	# if chan.connected():
	# raise
	# NOTE: `Channel.send()` may still raise `TransportClosed` on
	# disconnect. We currently let that propagate rather than
	# treating final-result sends as best-effort.
	await chan.send(return_msg)

[Copilot]

Catching TransportClosed here only handles disconnects raised directly from the wrapped invoke body. A TransportClosed raised later while attempting to ship/relay a boxed RPC error (e.g., via Channel.send() inside error-relay code paths) will not be caught by this clause and can still escape the context manager. If the goal is to tolerate disconnects during error relay, consider also handling TransportClosed around the error-shipment path (or updating the helper that ships errors to treat TransportClosed like other transport failures).

Suggested change

	except TransportClosed:
	log.exception('Tpt disconnect during remote-exc relay?')
	raise
	except TransportClosed as err:
	log.exception('Tpt disconnect during remote-exc relay?')
	rpc_err = err

[Copilot]

log.exception() will always emit a traceback here, which can be very noisy if TransportClosed is a relatively common/expected condition (and it looks like TransportClosed already carries its own loglevel + reporting via report_n_maybe_raise()). Consider logging at the transport-appropriate level (or deferring reporting to the outer message-loop handler) to avoid duplicate stack traces for normal disconnect scenarios.

[Copilot]

This not isinstance(err, TransportClosed) guard looks redundant now that TransportClosed is already handled by the preceding except TransportClosed: block. Consider removing it to simplify the condition (or, if you intend to handle TransportClosed here, restructure the exception handling so it can actually reach this branch).

Suggested change

	and
	not isinstance(err, TransportClosed)

[Copilot]

Typo in comment: "psuedo" -> "pseudo".

Suggested change

	# don't REPL any psuedo-expected tpt-disconnect
	# don't REPL any pseudo-expected tpt-disconnect

[Copilot]

Changing _invoke(..., hide_tb=...) default from True to False is a user-visible behavior change (it will start showing internal runtime frames by default in tracebacks for RPC tasks). If this wasn’t intentional for this PR, consider keeping the previous default and only forcing hide_tb=False at specific call sites where you want extra visibility (e.g., in debug mode / transport-error scenarios).

Suggested change

	hide_tb: bool = False,
	hide_tb: bool = True,

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

tractor/ipc/_transport.py:495

• In the except (BrokenResourceError, ClosedResourceError) block, the default case _: re-raises the original Trio exception. Higher layers in this PR are being updated to only tolerate TransportClosed, so letting raw Trio transport exceptions escape here can lead to unexpected crashes. Consider always translating these low-level transport errors into TransportClosed (possibly with a generic message/loglevel) instead of re-raising trans_err.

                    # unless the disconnect condition falls under "a
                    # normal operation breakage" we usualy console warn
                    # about it.
                    case _:
                        log.exception(
                            f'{tpt_name} layer failed pre-send ??\n'
                        )
                        raise trans_err

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The teardown loop uses while ...: ... else: ptyproc.kill(SIGKILL), but since the loop has no break, the else branch will always run whenever the loop exits normally (including when the process has already exited). This risks sending SIGKILL unnecessarily (and can raise if the process is already gone). Consider replacing the while/else with an explicit timeout loop and then if spawned.isalive(): kill(SIGKILL) after the loop; also _time_took := (time.time() - start) < timeout assigns a boolean, so the name is misleading and the elapsed time isn't preserved.

Suggested change

	(_time_took := (time.time() - start) < timeout)
	):
	ptyproc.kill(signal.SIGINT)
	time.sleep(0.01)
	else:
	ptyproc.kill(signal.SIGKILL)
	(time.time() - start) < timeout
	):
	ptyproc.kill(signal.SIGINT)
	time.sleep(0.01)
	if spawned and spawned.isalive():
	try:
	ptyproc.kill(signal.SIGKILL)
	except OSError:
	# Process may have exited between isalive() and kill()
	pass

[goodboy]

another good catch!

[Copilot]

Spelling in docstring: ephemercal should be ephemeral.

Suggested change

	expected-graceful cases, normally ephemercal
	expected-graceful cases, normally ephemeral

[Copilot]

Spelling/grammar in docstring: deliverd -> delivered, alo -> also, and same a -> same as.

[Copilot]

Spelling in comment: FURHTER should be FURTHER.

Suggested change

	# FURHTER, we should never really have to handle these
	# FURTHER, we should never really have to handle these

[Copilot]

by_whom can be None when the error message isn't in the mapping, but it's annotated as str. This is a type mismatch and can mask the fact that the subsequent match branch depends on optionality; annotate as str | None (or provide a non-None default) so type checkers and readers get the correct contract.

Suggested change

	by_whom: str = {
	by_whom: str | None = {

[Copilot]

This assertion doesn't do what it intends: (isinstance(exc, TransportClosed) for exc in excs) creates a generator which is always truthy, so the test will pass even when excs contains non-TransportClosed exceptions. Use all(isinstance(exc, TransportClosed) for exc in excs) (and consider asserting the expected exception types/count explicitly).

Suggested change

	(isinstance(exc, TransportClosed)
	for exc in excs)
	all(
	isinstance(exc, TransportClosed)
	for exc in excs
	)

[goodboy]

woo good catch ma fren!