Skip to content

Contain backend OSErrors at the command-execution boundary (closes #9) - #11

Open
ninowalker wants to merge 1 commit into
dbreunig:mainfrom
ninowalker:fix/oserror-command-boundary
Open

Contain backend OSErrors at the command-execution boundary (closes #9)#11
ninowalker wants to merge 1 commit into
dbreunig:mainfrom
ninowalker:fix/oserror-command-boundary

Conversation

@ninowalker

Copy link
Copy Markdown

Closes #9. Companion to #10 — both come from writing a network-backed IFileSystem, and neither depends on the other.

The problem

Commands catch a narrow set: cat catches exactly FileNotFoundError, IsADirectoryError, PermissionError, and across the command tree there are only a handful of except OSError. A backend that raises anything else propagates out of bash.exec() as a Python exception rather than a nonzero exit.

For a network-backed filesystem there is no exception to fail through at all — a connect timeout mid-cat becomes a traceback out of the sandbox instead of cat: /x: Connection timed out, exit 1. For the agent use case that breaks containment: a sandbox that raises through its own boundary isn't one, and every caller needs its own try/except around exec() to avoid crashing on what should have been a failed command.

The redirect path already gets this right — _apply_redirections wraps each redirect broadly and yields exit 1 — so the command path is the inconsistency, not the new behaviour.

The change

One except OSError clause on the existing try/finally in _execute_simple_command. That block already spans builtin dispatch, command dispatch and output redirection, so the clause needs no new structure and sees only errors no command handled — existing messages are unchanged. It reports <cmd>: <filename>: <strerror> and exit 1, matching how the coreutils phrase it.

OSError and not Exception, deliberately. Three things follow from that choice:

  • TimeoutError and ConnectionError are OSError subclasses, so network-backed backends get a natural way to fail.
  • The interpreter's own control flow (ExitError, BreakError, ReturnError, …) derives from InterpreterError, not OSError, so it cannot be swallowed here.
  • Anything that isn't an OSError is a bug in the backend rather than a condition a shell can report, and should keep crashing loudly.

The README gains a Custom Filesystems section stating the taxonomy, since #9 asked for either the fix or the documentation and the fix is only usable if implementers know the contract.

Tests

tests/test_interpreter/test_fs_error_boundary.py — 10 tests over a backend that raises on demand: each of the three recognized errors still produces its existing message (no regression in phrasing), a bare OSError/TimeoutError/ConnectionError becomes exit 1 with the detail on stderr, a non-OSError still propagates, and control-flow exceptions are unaffected.

Full suite on this branch: 700 failed, 4548 passed, 183 skipped.
Full suite on main (48d111c): 700 failed, 4538 passed, 183 skipped.

Same 700 pre-existing failures (the awk/jq spec files), +10 from this PR. ruff check on the touched source file reports the same 35 pre-existing findings before and after, so nothing new is added there either.

A custom filesystem backend that raises anything outside the three errors
the commands recognize propagates out of bash.exec() as a Python
exception instead of a nonzero exit. Network-backed backends have no
exception to fail through at all.

Add one except OSError clause to the try/finally in
_execute_simple_command, which spans builtin dispatch, command dispatch
and output redirection. It reports "<cmd>: <filename>: <strerror>" and
exit 1, and only sees errors no command handled, so existing messages are
unchanged. OSError only: the interpreter's control-flow exceptions derive
from InterpreterError and cannot be swallowed, and a non-OSError means the
backend is broken and should crash rather than become exit 1.

Document the expected exception taxonomy for backend authors in the README.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backend exceptions outside {FileNotFoundError, IsADirectoryError, PermissionError} escape

1 participant