From 18b4a8d0dd7f8c9aac343cbdc95eb5053626903b Mon Sep 17 00:00:00 2001 From: Gert Pellin - GPE Date: Sun, 11 Oct 2026 19:24:30 +0200 Subject: [PATCH] CLI: cadcraft-cli exits quietly when stdout is closed early (#568) Squash of PR #568 by @switch87. Co-Authored-By: Claude Opus 5.5 --- apps/cadcraft-cli/src/main.rs | 55 +++++++++++++++++++++++++++++----- apps/cadcraft-cli/tests/cli.rs | 53 ++++++++++++++++++++++++++++++++ docs/architecture.md | 5 +++- 3 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 apps/cadcraft-cli/tests/cli.rs diff --git a/apps/cadcraft-cli/src/main.rs b/apps/cadcraft-cli/src/main.rs index 5f497bc7..25099da3 100644 --- a/apps/cadcraft-cli/src/main.rs +++ b/apps/cadcraft-cli/src/main.rs @@ -11,12 +11,48 @@ //! ``` #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +/// `println!` that doesn't panic when stdout is closed (`cadcraft-cli commands | head`) with +/// "failed printing to stdout: Broken pipe (os error 32)". Once stdout has failed, later output is +/// dropped and the work goes on: see `stdout_failed`. +macro_rules! outln { + ($($arg:tt)*) => {{ + use std::io::Write as _; + if !$crate::stdout_gone() { + if let Err(e) = writeln!(std::io::stdout(), $($arg)*) { + $crate::stdout_failed(e); + } + } + }}; +} + use std::process::ExitCode; use cadcraft_engine::doc::Space; use cadcraft_engine::geom::{Bounds2, Vec2}; use cadcraft_engine::{Session, View}; use serde_json::{Value, json}; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// Set once a write to stdout has failed; later output is dropped. +static STDOUT_GONE: AtomicBool = AtomicBool::new(false); +/// Set when stdout failed for a reason other than a closed reader: the exit status becomes 1. +static STDOUT_ERROR: AtomicBool = AtomicBool::new(false); + +fn stdout_gone() -> bool { + STDOUT_GONE.load(Ordering::Relaxed) +} + +/// stdout went away. The program doesn't stop there: the rest of `run` (later `--cmd` results, +/// `--save`, `--export`) still happens, so `cadcraft-cli run … --save out.dxf | head` keeps the +/// drawing. A reader that stopped early (a closed pipe) is not an error, as in ripgrep; any other +/// write error is reported and makes the exit status 1. +fn stdout_failed(e: std::io::Error) { + if STDOUT_GONE.swap(true, Ordering::Relaxed) || e.kind() == std::io::ErrorKind::BrokenPipe { + return; + } + eprintln!("cadcraft-cli: can't write to stdout: {e}"); + STDOUT_ERROR.store(true, Ordering::Relaxed); +} const USAGE: &str = "usage: cadcraft-cli info FILE.dxf @@ -172,7 +208,7 @@ fn info(path: &str) -> Result<(), String> { let mut s = Session::empty(); open(&mut s, path)?; let v = s.execute("drawing.inspect", &json!({ "entities": false })).map_err(|e| e.to_string())?; - println!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()); + outln!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()); Ok(()) } @@ -277,7 +313,7 @@ fn run(args: &[String]) -> Result<(), String> { let params: Value = serde_json::from_str(json).map_err(|e| format!("--cmd {id}: {e}"))?; let r = s.execute(id, ¶ms).map_err(|e| e.to_string())?; if !r.is_null() { - println!("{r}"); + outln!("{r}"); } } } @@ -430,10 +466,10 @@ fn perf(args: &[String]) -> Result<(), String> { let back = cadcraft_io::read(&bytes, "perf.dxf").map_err(|e| e.to_string())?; rows.push(("DXF read".into(), ms(t), format!("{} entities", back.entity_count()))); - println!("{:<40} {:>12} notes", "operation", "ms"); - println!("{}", "-".repeat(80)); + outln!("{:<40} {:>12} notes", "operation", "ms"); + outln!("{}", "-".repeat(80)); for (name, t, note) in rows { - println!("{name:<40} {t:>12.2} {note}"); + outln!("{name:<40} {t:>12.2} {note}"); } Ok(()) } @@ -521,6 +557,11 @@ mod mcp_args_tests { } fn main() -> ExitCode { + let code = cli(); + if STDOUT_ERROR.load(Ordering::Relaxed) { ExitCode::FAILURE } else { code } +} + +fn cli() -> ExitCode { install_io(); let args: Vec = std::env::args().skip(1).collect(); let rest: Vec = args.iter().skip(1).cloned().collect(); @@ -538,11 +579,11 @@ fn main() -> ExitCode { .filter(|c| f.as_ref().is_none_or(|f| c.id.contains(f.as_str()) || c.label.to_ascii_lowercase().contains(f.as_str()))) .map(|c| serde_json::to_value(c.info(&s)).unwrap_or_default()) .collect(); - println!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()); + outln!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()); }), Some("mcp") => mcp(&rest), Some("--version" | "-V") => { - println!("cadcraft-cli {}", env!("CARGO_PKG_VERSION")); + outln!("cadcraft-cli {}", env!("CARGO_PKG_VERSION")); Ok(()) } _ => Err(USAGE.into()), diff --git a/apps/cadcraft-cli/tests/cli.rs b/apps/cadcraft-cli/tests/cli.rs new file mode 100644 index 00000000..2dbf1c54 --- /dev/null +++ b/apps/cadcraft-cli/tests/cli.rs @@ -0,0 +1,53 @@ +//! `cadcraft-cli` against a stdout nobody reads any more (#537). + +use std::io::Read; +use std::process::{Command, Output, Stdio}; + +/// `cadcraft-cli commands | head` panicked with "failed printing to stdout: Broken pipe (os error +/// 32)" and exit status 101: `commands` prints a few thousand lines, which no pipe buffer holds, +/// so the reader is gone before the last write. +#[test] +fn closed_stdout_ends_quietly() { + for args in [&["commands"][..], &["commands", "ellipse"], &["--version"]] { + let mut child = Command::new(env!("CARGO_BIN_EXE_cadcraft-cli")).args(args).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + let mut first = [0u8; 10]; + child.stdout.as_mut().unwrap().read_exact(&mut first).unwrap(); + drop(child.stdout.take()); + let out = child.wait_with_output().unwrap(); + let err = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success() && err.is_empty(), "{args:?}: {:?}: {err}", out.status); + } +} + +/// The CLI keeps working once stdout is gone: `run` still executes its commands and still saves, +/// so `cadcraft-cli run … --save out.dxf | head` keeps the drawing. +fn run_with_closed_stdout(args: &[&str]) -> Output { + let mut child = Command::new(env!("CARGO_BIN_EXE_cadcraft-cli")).args(args).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + drop(child.stdout.take()); + child.wait_with_output().unwrap() +} + +#[test] +fn closed_stdout_still_runs_the_commands_and_saves() { + let dir = std::env::temp_dir().join(format!("cc-cli-pipe-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let drawing = dir.join("piped.dxf"); + let out = run_with_closed_stdout(&[ + "run", + "--metric", + "--cmd", + "ellipse {\"center\":[0,0],\"major\":[10,0],\"ratio\":0.5}", + "--cmd", + "drawing.inspect {}", + "--save", + drawing.to_str().unwrap(), + ]); + assert!(out.status.success(), "{:?}: {}", out.status, String::from_utf8_lossy(&out.stderr)); + let saved = std::fs::read_to_string(&drawing).unwrap(); + assert!(saved.contains("ELLIPSE"), "the drawing is saved all the same: {}", &saved[..saved.len().min(200)]); + + // A failing command is still a failure, whether or not anyone reads stdout. + let out = run_with_closed_stdout(&["run", "--metric", "--cmd", "no.such.command {}"]); + assert_eq!(out.status.code(), Some(1)); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/architecture.md b/docs/architecture.md index 0b60cbe1..17ea6e36 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -75,7 +75,10 @@ name. Detail and gaps: [file-format-parity.md](file-format-parity.md). `engine.execute`, `ui.screenshot`, `ui.render` …); see [control-protocol.md](control-protocol.md). - **MCP:** `cadcraft-cli mcp` (headless) or `--connect HOST:PORT` (drive the running app); see [mcp.md](mcp.md). -- **CLI:** `cadcraft-cli run --script … --export out.png` for headless batch work. +- **CLI:** `cadcraft-cli run --script … --export out.png` for headless batch work. A reader that + closes stdout early (`… | head`) drops the rest of the output but keeps the work — later + commands, `--save`, `--export` — and the exit status still reports failures; another stdout + write error is reported and makes it 1. ## Safety