diff --git a/apps/deckcraft-cli/src/help.rs b/apps/deckcraft-cli/src/help.rs index 4227905f..0d3fd400 100644 --- a/apps/deckcraft-cli/src/help.rs +++ b/apps/deckcraft-cli/src/help.rs @@ -180,7 +180,10 @@ A presentation is edited by running commands on it: deckcraft-cli run --cmd 'slide.new={{\"title\":\"Q3 results\"}}' --save q3.deckcraft `deckcraft-cli help SUBCOMMAND`, or `deckcraft-cli SUBCOMMAND --help`, explains one subcommand; -`deckcraft-cli help run` explains how to write a command.", +`deckcraft-cli help run` explains how to write a command. + +If an output reader closes early, run, render, render-transition and convert continue; their exit +status reflects the work, not output delivery. Stop the process to cancel them.", list.join("\n") ) } diff --git a/apps/deckcraft-cli/src/main.rs b/apps/deckcraft-cli/src/main.rs index a3c03ce2..b97ea353 100644 --- a/apps/deckcraft-cli/src/main.rs +++ b/apps/deckcraft-cli/src/main.rs @@ -7,23 +7,38 @@ mod automation_args; mod help; use std::process::ExitCode; +use std::sync::atomic::{AtomicBool, Ordering}; use deckcraft_engine::Session; use serde_json::{Value, json}; +static STDOUT_CLOSED: AtomicBool = AtomicBool::new(false); + macro_rules! outln { ($($arg:tt)*) => {{ - use std::io::Write as _; - if let Err(e) = writeln!(std::io::stdout(), $($arg)*) { + if !STDOUT_CLOSED.load(Ordering::Relaxed) && let Err(e) = stdout_line(format_args!($($arg)*)) { if e.kind() == std::io::ErrorKind::BrokenPipe { - std::process::exit(0); + STDOUT_CLOSED.store(true, Ordering::Relaxed); + } else { + eprintln!("deckcraft-cli: can't write to stdout: {e}"); + std::process::exit(1); } - eprintln!("deckcraft-cli: can't write to stdout: {e}"); - std::process::exit(1); } }}; } +fn stdout_line(args: std::fmt::Arguments<'_>) -> std::io::Result<()> { + use std::io::Write as _; + #[cfg(unix)] + let mut stdout = { + use std::os::fd::AsFd as _; + std::fs::File::from(std::io::stdout().as_fd().try_clone_to_owned()?) + }; + #[cfg(not(unix))] + let mut stdout = std::io::stdout(); + stdout.write_all(format!("{args}\n").as_bytes()) +} + fn main() -> ExitCode { let args: Vec = std::env::args().skip(1).collect(); let rest = args.get(1..).unwrap_or(&[]); diff --git a/apps/deckcraft-cli/tests/it/closed_stdout.rs b/apps/deckcraft-cli/tests/it/closed_stdout.rs new file mode 100644 index 00000000..aeb20dba --- /dev/null +++ b/apps/deckcraft-cli/tests/it/closed_stdout.rs @@ -0,0 +1,110 @@ +use std::fs::{self, File}; +use std::os::fd::OwnedFd; +use std::os::unix::net::UnixStream; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; +use std::time::{SystemTime, UNIX_EPOCH}; + +struct Scratch(PathBuf); + +impl Scratch { + fn new(name: &str) -> Self { + let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos(); + let path = std::env::temp_dir().join(format!("deckcraft-closed-stdout-{name}-{}-{nonce}", std::process::id())); + fs::create_dir(&path).unwrap(); + Self(path) + } +} + +impl Drop for Scratch { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn closed_stdout() -> Stdio { + let (writer, reader) = UnixStream::pair().unwrap(); + drop(reader); + Stdio::from(OwnedFd::from(writer)) +} + +fn run(dir: &Path, args: &[&str], stdout: Stdio) -> Output { + Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).current_dir(dir).args(args).stdout(stdout).stderr(Stdio::piped()).output().unwrap() +} + +#[test] +fn broken_pipe_does_not_skip_later_commands_or_save() { + let dir = Scratch::new("save"); + let out = run( + &dir.0, + &[ + "run", + "--cmd", + r#"slide.new={"layout":"titleOnly","title":"First"}"#, + "--cmd", + r#"slide.new={"layout":"titleOnly","title":"Later"}"#, + "--print", + "--save", + "out.deckcraft", + ], + closed_stdout(), + ); + assert!(out.status.success(), "exit {:?}: {}", out.status, String::from_utf8_lossy(&out.stderr)); + let saved = dir.0.join("out.deckcraft"); + assert!(saved.is_file(), "requested save was skipped"); + let info = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).arg("info").arg(saved).output().unwrap(); + assert!(info.status.success(), "{}", String::from_utf8_lossy(&info.stderr)); + let data: serde_json::Value = serde_json::from_slice(&info.stdout).unwrap(); + let slides = data["slides"].as_array().unwrap(); + assert!(slides.iter().any(|slide| slide["title"] == "Later"), "later command did not persist: {data}"); +} + +#[test] +fn broken_pipe_does_not_hide_a_later_command_failure() { + let dir = Scratch::new("command-failure"); + let out = run( + &dir.0, + &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--cmd", r#"slide.go={"index":999}"#, "--print", "--save", "out.deckcraft"], + closed_stdout(), + ); + assert!(!out.status.success(), "a later command failure returned success"); + assert!(!dir.0.join("out.deckcraft").exists(), "failed run unexpectedly saved"); + assert!(String::from_utf8_lossy(&out.stderr).contains("no slide 999"), "missing command failure diagnostic"); +} + +#[test] +fn broken_pipe_does_not_hide_a_save_failure() { + let dir = Scratch::new("save-failure"); + fs::create_dir(dir.0.join("out.deckcraft")).unwrap(); + let out = run(&dir.0, &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--print", "--save", "out.deckcraft"], closed_stdout()); + assert!(!out.status.success(), "a failed save returned success"); + assert!(!out.stderr.is_empty(), "missing save failure diagnostic"); +} + +#[test] +fn broken_pipe_does_not_skip_later_rendered_slides() { + let dir = Scratch::new("render-all"); + let baseline = run(&dir.0, &["render", "--sample", "--all", "--scale", "0.1", "normal"], Stdio::piped()); + assert!(baseline.status.success(), "{}", String::from_utf8_lossy(&baseline.stderr)); + let closed = run(&dir.0, &["render", "--sample", "--all", "--scale", "0.1", "closed"], closed_stdout()); + assert!(closed.status.success(), "{}", String::from_utf8_lossy(&closed.stderr)); + let names = |folder: &str| { + let mut files: Vec<_> = fs::read_dir(dir.0.join(folder)).unwrap().map(|entry| entry.unwrap().file_name()).collect(); + files.sort(); + files + }; + let expected = names("normal"); + assert!(expected.len() > 1, "sample must have multiple slides"); + assert_eq!(names("closed"), expected, "rendering stopped after stdout closed"); +} + +#[test] +fn other_stdout_write_errors_fail() { + let dir = Scratch::new("write-failure"); + let path = dir.0.join("readonly"); + fs::write(&path, b"").unwrap(); + let readonly = File::open(path).unwrap(); + let out = run(&dir.0, &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--print"], Stdio::from(readonly)); + assert!(!out.status.success(), "a non-broken stdout error returned success"); + assert!(String::from_utf8_lossy(&out.stderr).contains("can't write to stdout")); +} diff --git a/apps/deckcraft-cli/tests/it/main.rs b/apps/deckcraft-cli/tests/it/main.rs index c6cdca9b..f1e1c089 100644 --- a/apps/deckcraft-cli/tests/it/main.rs +++ b/apps/deckcraft-cli/tests/it/main.rs @@ -1,4 +1,6 @@ mod cli; +#[cfg(unix)] +mod closed_stdout; mod convert; mod damaged; // Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md).