Repository navigation
fix(cli): exit 2 instead of panicking on stderr, args or --timeout - #604
Conversation
The CLI promises exit codes 0, 1 and 2, but several inputs ended in a panic, exit 101 (vyncint#594): - every diagnostic and the inspect trailer used eprintln!, which panics when stderr is closed or full. Writes now go through a checked helper; a lost diagnostic keeps its exit 2, and a trailer that cannot be written is exit 2. - std::env::args panics on an argument that is not UTF-8. Arguments are read with args_os: termlens's own options and operands must be UTF-8 and are refused with exit 2, while the program inspect runs and its arguments pass through untouched. - Instant::now() + --timeout overflowed for a huge value after the program was spawned. A timeout the clock cannot add is refused by name. - render --out "" printed the filesystem's complaint; it now says --out needs a PATH argument, as --out= already did (vyncint#451). The inspect example gets the same fixes, including its println! of --help under | true. Signed-off-by: anyingiit <49945850+anyingiit@users.noreply.github.com>
The test closes its read end of termlens's stderr pipe and expects the write to find no reader. In cli.rs, a sibling test that forks while the pipe exists holds a copy of the read end until its child execs, so the write succeeds and inspect exits 0. The stress workflow caught it once at 16 threads (iteration 5 of 25); a sibling whose child waits 3 s before exec reproduces it every time. Like the library's descriptors.rs, a test about the process's descriptor table now runs alone. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
vyncint
left a comment
vyncint
left a comment
There was a problem hiding this comment.
Thanks — careful work, and I agree with refusing non-UTF-8 for termlens's own paths. Beyond your tests I checked a huge --idle against a running program (bounded by the deadline, no panic) and that a non-UTF-8 byte reaches the program untouched. One change pushed on top: the stress workflow caught a_closed_stderr_is_exit_two_not_a_panic exiting 0 once at 16 threads. A sibling test that forks while the pipe exists holds the read end until its child execs, so the write succeeds (a pre_exec-sleep sibling reproduces it every time). The test now runs in a binary of its own, closed_stderr.rs, like the library's descriptors.rs; stress is 15/15 with it. About the attribution selftest: it passes 10/10 here, so a GIT_AUTHOR_EMAIL exported in your shell is the likely cause (it overrides the -c user.email the bot-identity case sets).
What & why
The CLI promises exit codes 0, 1 and 2, but the three inputs in #594 ended in a panic and exit 101. This fixes all three plus the
--out ""message the issue mentions:2>&1 | true,2>/dev/full): diagnostics and theinspecttrailer went througheprintln!, which panics on a failed write. They now go through a small checked helper. A lost diagnostic keeps its exit 2, and a trailer that cannot be written is exit 2, since the run's result never reached the caller.args_os. termlens's own options and operands must be UTF-8 and are refused with exit 2 ("\xff" is not valid UTF-8; termlens's own arguments must be), while the programinspectruns gets its arguments untouched, sotermlens inspect -- prog $'\xff'works.--timeout: the deadline is computed withchecked_addbefore the program is spawned, so a timeout the clock cannot represent is refused by name instead of panicking after the spawn.render --out ""now says--out needs a PATH argument, as--out=already did (termlens render --out=fails withtermlens: : No such file or directory, naming neither the flag nor the file #451).The
inspectexample gets the same fixes, including its--help/--versionunder| true.Closes #594
Motivation / disclosure: I had some spare AI-assistant quota (Claude Code) and am using it to try to help projects with open good-first-issues. The change was prepared with Claude Code and verified as listed below; per CONTRIBUTING §6 the commit carries no AI attribution and I'm the author of record under the DCO. If it doesn't fit, isn't up to your bar, or you'd simply rather not take it, please feel free to close it, no hard feelings at all.
Checklist
crates/termlens-cli/tests/cli.rs(closed stderr,/dev/fullon Linux, non-UTF-8 arguments on Unix, an unrepresentable--timeout,render --out ""). All five fail onmainand pass with the change.cargo fmt --allandcargo clippy --workspace --all-targets --all-featuresare clean;cargo test --workspacepassescheck-dco.sh main..HEAD: OK)check-no-ai-attribution.sh main..HEAD: OK)CHANGELOG.mdupdated under[Unreleased]→Fixedcargo insta review, not blind-accepted (none in this PR)Not done: I didn't run
cargo-semver-checks(not installed here); the change is binary-only plus an example, with no library API change. I haven't run the stress workflow (stress.yml); four of the new tests spawninspect, so they add PTY spawns, which CONTRIBUTING §3 says must pass it. I can trigger it on my fork, or a maintainer can. I also didn't run the Windows clippy target.One behaviour to flag: termlens's own path arguments (
--cwd, and thediff/renderfile operands) must now be UTF-8, so a non-UTF-8 path is refused with exit 2 rather than accepted. Before this it panicked. The issue asks for that for termlens's own options; if you'd rather accept non-UTF-8 paths, I can carry those asOsString. Intools/preflight.shthe attribution-gate selftest's "bot author identity" case fails on an untouchedmainin my environment too, so I don't think it relates to this change.