refactor(api): typed TmuxError for terminal_tmux (#3576)#6237
Merged
Conversation
Migrate `crates/librefang-api/src/terminal_tmux.rs` off `anyhow::Result` / `anyhow::anyhow!` to a `thiserror`-derived `TmuxError` enum, following the #3711 typed-error pattern. The new enum captures the real failure kinds from the previous call sites: process spawn failure (carrying the underlying `io::Error` on the `source()` chain), subprocess timeout, post-spawn I/O error, non-zero exit (carrying the rendered status and trimmed stderr), pre-spawn argument validation rejection, empty `new-window` output, and the `parse_window_list` field/index parse failures. Every `Display` string is preserved byte-for-byte from the old `anyhow!(…)` text because it reaches daemon logs via `warn!(error = %e, …)` in `routes/terminal.rs`. Callers consume the error purely through `Display` and produce their own HTTP responses, so the route handlers, `server.rs` shutdown cleanup, and the websocket attach path compile unchanged with equivalent behavior. Scoped slice toward #3576 — the bulk of that issue is already covered by the #3711 series (`tool_runner` `ToolError`, kernel `anyhow` count at zero, structured `KernelError`). The issue's "drop KernelError" suggestion is intentionally NOT actioned: #3711 added those structured variants on purpose, so removing them would be a regression. Refs #3576.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scoped slice toward #3576 ("inconsistent error contracts — String / anyhow / LibreFangError / LlmError mixed").
Migrates
crates/librefang-api/src/terminal_tmux.rsoffanyhow::Result/anyhow::anyhow!to athiserror-derivedTmuxErrorenum, following the typed-error pattern established by the #3711 series (tool_runnerToolError).This is a partial — it does NOT close #3576.
The bulk of that issue is already covered by the ongoing #3711 migration:
tool_runneralready uses a typedToolError(crates/librefang-runtime/src/tool_runner/error.rs).anyhowinlibrefang-kernelis already at zero; inlibrefang-runtimeit survives only in code comments.KernelError(crates/librefang-kernel/src/error.rs) now has structured variants (Hand,WasmSandbox,Python, …).The issue's "drop KernelError" suggestion is intentionally NOT actioned: 21 Error enums collapse to String at trait boundaries — restore structured kernel/runtime errors #3711 added those variants on purpose, so removing them would be a regression.
The one genuine remaining lib-crate
anyhowconcentration wasterminal_tmux.rs(~33anyhow!call sites across the tmux helper methods and the freeparse_window_list), which this PR addresses.TmuxErrorvariants#[derive(Debug, thiserror::Error)],#[non_exhaustive]:Spawn { cmd: &'static str, source: std::io::Error }—Command::spawn()failed; the spawnio::Erroris preserved via#[source].cmddistinguishes"tmux"/"tmux has-session"/"tmux new-session".Timeout { what: &'static str }— the subprocess exceeded the hard timeout ("tmux command"/"tmux has-session"/"tmux new-session").Io { source: std::io::Error }— post-spawn wait/output collection failed; source preserved.NonZeroExit { status: String, stderr: String }— tmux ran but exited non-zero, carrying the rendered status and trimmed stderr.InvalidArgument(String)— a window id/name failed validation before any subprocess spawned; preserves the regex-pattern detail.NoOutput—new-window -Psucceeded but produced no parseable line.MissingField { field: &'static str, line: String }— alist-windows -Fline was missing the named column.InvalidIndex { value: String }— thewindow_indextoken did not parse asu32.Every
Displaystring is preserved byte-for-byte from the oldanyhow!(…)text because it reaches daemon logs viawarn!(error = %e, …).The spawn
io::Errorand post-spawnio::Errorare kept on thesource()chain via#[source].Callers touched
All callers consume the error purely through
Display(warn!(error = %e, …)/tracing::debug!("…: {e}")) and produce their own HTTP responses — none inspect the error type or walksource(), so they compile unchanged with equivalent behavior.The route handler keeps its existing status mapping (SERVICE_UNAVAILABLE for
ensure_session, INTERNAL_SERVER_ERROR for list/new, dedicated{"error": …}JSON for delete/rename); no response shape changed.crates/librefang-api/src/routes/terminal.rs— HTTP boundary (list / create / delete / rename window, websocket attach + window-switch paths).crates/librefang-api/src/server.rs— daemon-shutdownkill_sessioncleanup.crates/librefang-api/src/lib.rs—pub mod terminal_tmux;(module declaration; no code change needed).routes/memory.rs's intentionalFrom<anyhow::Error>compat shim is out of scope and left untouched; no other crate is modified.Tests
The
parse_window_listunit tests are kept passing.The two that previously asserted only
.is_err()now assert the typed variant AND the byte-for-byte Display text:parse_malformed_line_returns_error→TmuxError::MissingField { field: "window_active", .. }, Displaymissing window_active in tmux output: "@1|0|editor".parse_bad_index_returns_error→TmuxError::InvalidIndex { .. }, Displayinvalid window index "abc".Verification
Run in the repo's sanctioned Docker dev image (
Dockerfile.rust-dev), scoped tolibrefang-api:cargo check -p librefang-api --lib— clean (Finishedin 1m34s, no warnings).cargo clippy -p librefang-api --all-targets -- -D warnings— clean (Finished, no warnings).cargo test -p librefang-api --lib terminal_tmux—24 passed; 0 failed.cargo fmt --check -p librefang-api— clean (exit 0).Cargo.lockregenerated: one line adds thethiserror 2.0.18dependency edge forlibrefang-api(already in the workspace dependency graph; no new transitive crates).Refs #3576.