diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index 753f0626..540a5463 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -36,10 +36,54 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +use std::io::Write; + use effectcraft_automation::tools::{self, Reply}; use effectcraft_automation::{Backend, McpServer}; use serde_json::{Value, json}; +/// `println!` that never panics: see [`write_line`]. +macro_rules! say { + ($($t:tt)*) => { + write_line(format_args!($($t)*)) + }; +} + +/// `eprintln!` that never panics: a diagnostic that can't be written is dropped. +macro_rules! note { + ($($t:tt)*) => {{ + let _ = writeln!(std::io::stderr(), $($t)*); + }}; +} + +/// Why stdout stopped taking output, once it has. +static STDOUT_FAILED: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Write a line to stdout. Once that fails, later output is dropped but the command still +/// finishes its work (a `run … --save` sequence isn't cut short). A reader that closes stdout +/// early (`info --json | head`) has read what it wanted; any other failure is reported and makes +/// the exit status 1 ([`stdout_broken`]). +fn write_line(text: std::fmt::Arguments) { + if STDOUT_FAILED.get().is_some() { + return; + } + let r = { + let mut out = std::io::stdout().lock(); + out.write_fmt(text).and_then(|()| out.write_all(b"\n")) + }; + if let Err(e) = r { + let _ = STDOUT_FAILED.set(e.kind()); + if e.kind() != std::io::ErrorKind::BrokenPipe { + note!("effectcraft-cli: cannot write to stdout: {e}"); + } + } +} + +/// Whether stdout failed for a reason other than its reader closing it early. +fn stdout_broken() -> bool { + STDOUT_FAILED.get().is_some_and(|k| *k != std::io::ErrorKind::BrokenPipe) +} + const USAGE: &str = "usage: effectcraft-cli [args] [--json] info project + engine summary commands [--filter TEXT] [--enabled] list engine commands @@ -173,11 +217,11 @@ fn value_arg(s: &str) -> Value { fn main() { let raw: Vec = std::env::args().skip(1).collect(); if matches!(raw.first().map(String::as_str), Some("--version" | "-V" | "version")) { - println!("effectcraft-cli {}", env!("CARGO_PKG_VERSION")); + say!("effectcraft-cli {}", env!("CARGO_PKG_VERSION")); return; } if raw.is_empty() || raw.iter().any(|a| a == "-h" || a == "--help") { - println!("{USAGE}"); + say!("{USAGE}"); std::process::exit(if raw.is_empty() { 2 } else { 0 }); } let mut args = match Args::parse(raw) { @@ -190,20 +234,21 @@ fn main() { } let cmd = args.pos.remove(0); match run(&cmd, &args, json_out) { + Ok(()) if stdout_broken() => std::process::exit(1), Ok(()) => {} Err(Failure::Usage(e)) => fail_usage(&e), Err(Failure::Error(e)) => { if json_out { - println!("{}", json!({"error": e})); + say!("{}", json!({"error": e})); } - eprintln!("effectcraft-cli {cmd}: {e}"); + note!("effectcraft-cli {cmd}: {e}"); std::process::exit(1); } } } fn fail_usage(e: &str) -> ! { - eprintln!("effectcraft-cli: {e}\n{USAGE}"); + note!("effectcraft-cli: {e}\n{USAGE}"); std::process::exit(2) } @@ -292,13 +337,13 @@ fn script_cmd(args: &Args, json_out: bool) -> Result<(), Failure> { emit(&o, true); } else { if let Some(out) = r["output"].as_str().filter(|o| !o.is_empty()) { - println!("{out}"); + say!("{out}"); } if !r["result"].is_null() && r["error"].is_null() { - println!("{}", r["result"]); + say!("{}", r["result"]); } if let Some(p) = saved { - eprintln!("saved {p}"); + note!("saved {p}"); } } if let Some(e) = r.get("error").filter(|e| !e.is_null()) { @@ -319,9 +364,9 @@ fn script_cmd(args: &Args, json_out: bool) -> Result<(), Failure> { /// Print a result: compact JSON with `--json`, else pretty JSON. fn emit(v: &Value, json_out: bool) { if json_out { - println!("{v}"); + say!("{v}"); } else { - println!("{}", serde_json::to_string_pretty(v).unwrap_or_default()); + say!("{}", serde_json::to_string_pretty(v).unwrap_or_default()); } } @@ -344,19 +389,19 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { if json_out { emit(&info, true); } else { - println!( + say!( "EffectCraft {} ({}), {} commands, {} effects", info["version"].as_str().unwrap_or(""), info["mode"].as_str().unwrap_or(""), info["commands"], info["effects"] ); - println!("project: {}", p["path"].as_str().unwrap_or("(unsaved)")); + say!("project: {}", p["path"].as_str().unwrap_or("(unsaved)")); for i in p["items"].as_array().into_iter().flatten() { - println!(" item {:>3} {:<12} {}", i["id"], i["type"].as_str().unwrap_or(""), i["name"].as_str().unwrap_or("")); + say!(" item {:>3} {:<12} {}", i["id"], i["type"].as_str().unwrap_or(""), i["name"].as_str().unwrap_or("")); } if let Some(c) = comp { - println!( + say!( "active comp {} \"{}\" {}x{} @ {} fps, {} s", c["id"], c["name"].as_str().unwrap_or(""), @@ -366,7 +411,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { c["duration"] ); for l in c["layers"].as_array().into_iter().flatten() { - println!(" #{:<3} id {:<4} {:<10} {}", l["index"], l["id"], l["type"].as_str().unwrap_or(""), l["name"].as_str().unwrap_or("")); + say!(" #{:<3} id {:<4} {:<10} {}", l["index"], l["id"], l["type"].as_str().unwrap_or(""), l["name"].as_str().unwrap_or("")); } } } @@ -422,7 +467,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { if json_out { emit(&v, true); } else { - println!("layer {} \"{}\" ({}) at {} s", v["id"], v["name"].as_str().unwrap_or(""), v["type"].as_str().unwrap_or(""), v["time"]); + say!("layer {} \"{}\" ({}) at {} s", v["id"], v["name"].as_str().unwrap_or(""), v["type"].as_str().unwrap_or(""), v["time"]); for p in v["properties"].as_array().into_iter().flatten() { let mut extra = String::new(); if let Some(k) = p.get("keys") { @@ -431,7 +476,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { if let Some(e) = p.get("expression").and_then(Value::as_str) { extra += &format!(" expr: {e}"); } - println!(" {:<44} {:<8} {}{extra}", p["path"].as_str().unwrap_or(""), p["type"].as_str().unwrap_or(""), p["value"]); + say!(" {:<44} {:<8} {}{extra}", p["path"].as_str().unwrap_or(""), p["type"].as_str().unwrap_or(""), p["value"]); } } } @@ -481,12 +526,16 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { if json_out { emit(&info, true); } else { - eprintln!("rendered {}x{} at {:.3}s in {ms:.1} ms -> {out}", f.width, f.height, f.time); + note!("rendered {}x{} at {:.3}s in {ms:.1} ms -> {out}", f.width, f.height, f.time); } } "mcp" => { let b = backend(args, false)?; - McpServer::new(b).serve_stdio().map_err(|e| e.to_string())?; + // A client that closes its end of stdout has ended the session. + match McpServer::new(b).serve_stdio() { + Err(e) if e.kind() != std::io::ErrorKind::BrokenPipe => return Err(e.to_string().into()), + _ => {} + } } other => return usage_err(&format!("unknown subcommand `{other}`")), } @@ -577,7 +626,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { let _ = s.execute("renderQueue.setRender", json!({"index": k + 1, "render": false})); } let r = s.execute("renderQueue.add", p).map_err(err)?; - eprintln!( + note!( "{} → {} ({}×{}, {} frames, {})", r["compName"].as_str().unwrap_or("?"), r["outputPath"].as_str().unwrap_or("?"), @@ -593,7 +642,8 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { std::thread::sleep(std::time::Duration::from_millis(100)); if tty && let Some(p) = s.render_progress() { let left = p.remaining.map(|r| format!(", ~{r:.1}s left")).unwrap_or_default(); - eprint!( + let _ = write!( + std::io::stderr(), " [{}/{}] frame {}/{} {:.1}s{left}\x1b[K", (p.items_done + 1).min(p.items_total), @@ -607,7 +657,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { } s.poll_render(); if tty { - eprintln!(); + note!(); } let mut results = vec![]; let mut failed = vec![]; @@ -615,7 +665,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { let name = s.project.item(it.comp).map(|i| i.name.clone()).unwrap_or_else(|| "?".into()); match &it.status { RenderStatus::Done => { - eprintln!("done: {name} → {} in {:.2}s", it.last_output.as_deref().unwrap_or("?"), it.render_time.unwrap_or(0.0)); + note!("done: {name} → {} in {:.2}s", it.last_output.as_deref().unwrap_or("?"), it.render_time.unwrap_or(0.0)); results.push(json!({"comp": name, "output": it.last_output, "seconds": it.render_time})); } RenderStatus::Failed(e) => failed.push(format!("{name}: {e}")), @@ -640,7 +690,7 @@ fn list_commands_cmd(args: &Args, json_out: bool) -> Result<(), Failure> { emit(&v, true); } else { for c in v.as_array().into_iter().flatten() { - println!( + say!( "{:<36} {:<36} {:<16} {}", c["id"].as_str().unwrap_or(""), c["label"].as_str().unwrap_or(""), @@ -708,7 +758,7 @@ fn bench_ops(args: &Args) -> Result<(), Failure> { m.extend(effectcraft_ui_egui::bench::ui_ops(|| { let mut s = effectcraft_host::session(); if let Err(e) = s.execute("file.open", json!({"path": path})) { - eprintln!("bench --ops: {e}"); + note!("bench --ops: {e}"); } s })); @@ -716,9 +766,9 @@ fn bench_ops(args: &Args) -> Result<(), Failure> { if args.flag("--json") { emit(&json!({"spec": format!("{spec:?}"), "measures": m.iter().map(perf::Measure::json).collect::>()}), true); } else { - println!("bench --ops: {} comps, {} layers in Main, {} footage items", spec.comps, spec.main_layers, spec.footage); + say!("bench --ops: {} comps, {} layers in Main, {} footage items", spec.comps, spec.main_layers, spec.footage); for x in &m { - println!(" {:<22} {:>10.2} ms {}", x.name, x.ms, x.note); + say!(" {:<22} {:>10.2} ms {}", x.name, x.ms, x.note); } } Ok(()) @@ -788,7 +838,7 @@ fn bench(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) { } } } - eprintln!( + note!( "bench (no cache): {n} runs at scale {}: wall min {:.2} ms, median {:.2} ms; CPU (all threads) min {:.2} ms", opts.scale, min(&totals), @@ -796,9 +846,9 @@ fn bench(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) { min(&cpus) ); for (d, name, p, c, fx) in per_layer { - eprintln!(" {:indent$}{name:<28} process {:8.2} ms composite {:8.2} ms (min)", "", min(&p), min(&c), indent = d * 2); + note!(" {:indent$}{name:<28} process {:8.2} ms composite {:8.2} ms (min)", "", min(&p), min(&c), indent = d * 2); for (id, v) in fx { - eprintln!(" {:indent$} fx {id:<30} {:8.2} ms", "", min(&v), indent = d * 2); + note!(" {:indent$} fx {id:<30} {:8.2} ms", "", min(&v), indent = d * 2); } } } @@ -819,7 +869,7 @@ fn bench_play(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) { let mut v: Vec = runs.iter().map(|r| r.0).collect(); let mean = v.iter().sum::() / n as f64; let cpu = runs.iter().map(|r| r.1).sum::() / n as f64; - eprintln!( + note!( "bench-play ({label}): {n} frames at scale {}: wall mean {mean:.2} ms, median {:.2} ms, min {:.2} ms; CPU mean {cpu:.2} ms", opts.scale, median(&mut v), @@ -827,7 +877,7 @@ fn bench_play(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) { ); } let st = cache.stats(); - eprintln!(" layer cache: {} hits, {} misses, {} entries, {:.1} MB", st.hits, st.misses, st.entries, st.bytes as f64 / 1e6); + note!(" layer cache: {} hits, {} misses, {} entries, {:.1} MB", st.hits, st.misses, st.entries, st.bytes as f64 / 1e6); } /// Median wall time (ms) of `n` runs of `f` after one warm-up run. @@ -1008,10 +1058,21 @@ fn bench_gpu(s: &Session, args: &Args) -> Result<(), Failure> { } }; let project = &project; - eprintln!("GPU: {} — median of {n} runs (ms/frame)", effectcraft_render::Accelerator::name(&gpu)); - eprintln!( + note!("GPU: {} — median of {n} runs (ms/frame)", effectcraft_render::Accelerator::name(&gpu)); + note!( "{:<28} {:>5} {:>10} {:>9} {:>9} {:>9} {:>9} {:>9} {:>8} {:>9} {:>9} {:>11}", - "comp", "res", "size", "cpu cold", "gpu cold", "cpu warm", "gpu warm", "gpu view", "speedup", "auto", "gpu≠cpu", "up/dn MB" + "comp", + "res", + "size", + "cpu cold", + "gpu cold", + "cpu warm", + "gpu warm", + "gpu view", + "speedup", + "auto", + "gpu≠cpu", + "up/dn MB" ); for cid in comps { let Some(comp) = project.comp(cid) else { continue }; @@ -1077,7 +1138,7 @@ fn bench_gpu(s: &Session, args: &Args) -> Result<(), Failure> { .count(); let pct = 100.0 * off as f64 / a.data.len().max(1) as f64; let speedup = cpu_warm / gpu_warm.max(1e-9); - eprintln!( + note!( "{name:<28} {label:>5} {size:>10} {cpu_cold:>9.2} {gpu_cold:>9.2} {cpu_warm:>9.2} {gpu_warm:>9.2} {view:>9.2} {speedup:>7.2}x {auto:>9.2} {pct:>8.3}% {traffic:>11}" ); } diff --git a/apps/effectcraft-cli/tests/cli.rs b/apps/effectcraft-cli/tests/cli.rs index 6ec33992..1247ee22 100644 --- a/apps/effectcraft-cli/tests/cli.rs +++ b/apps/effectcraft-cli/tests/cli.rs @@ -130,6 +130,32 @@ fn errors_are_json() { assert_eq!(out.status.code(), Some(2)); } +/// A reader that closes stdout before the CLI writes (`| head`) is not a crash: the command +/// still does its work and exits 0, without a panic (#167). +#[test] +fn a_closed_stdout_is_not_a_crash() { + let proj = tmp("closed-stdout.ecproj"); + let p = proj.to_str().unwrap(); + for args in [&["info", "--json"][..], &["exec", "--list"], &["run", "comp.new", r#"{"name":"Piped"}"#, "project.summary", "--empty", "--save-as", p]] { + let mut child = bin().args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + drop(child.stdout.take()); + let out = child.wait_with_output().unwrap(); + let err = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(0), "{args:?}: {err}"); + assert!(!err.contains("panicked"), "{args:?}: {err}"); + } + assert!(std::fs::read_to_string(&proj).unwrap().contains("Piped"), "the sequence ran to the end and saved"); + // The MCP server ends quietly when its client closes stdout. + let mut child = bin().arg("mcp").stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + drop(child.stdout.take()); + { + let mut stdin = child.stdin.take().unwrap(); + let _ = writeln!(stdin, "{}", json!({"jsonrpc": "2.0", "id": 1, "method": "ping"})); + } + let out = child.wait_with_output().unwrap(); + assert_eq!(out.status.code(), Some(0), "{}", String::from_utf8_lossy(&out.stderr)); +} + #[test] fn mcp_over_stdio() { let mut child = bin().args(["mcp", "--demo"]).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::null()).spawn().unwrap();