diff --git a/lib/src/utils.rs b/lib/src/utils.rs index 5ceae99d..9bc8598f 100644 --- a/lib/src/utils.rs +++ b/lib/src/utils.rs @@ -1,5 +1,5 @@ use std::future::Future; -use std::io::Write; +use std::io::{Read, Seek, Write}; use std::os::fd::BorrowedFd; use std::process::Command; use std::time::Duration; @@ -15,17 +15,72 @@ pub(crate) trait CommandRunExt { fn run(&mut self) -> Result<()>; } +/// Somewhat like String::from_utf8_lossy() but just ignores +/// invalid UTF-8 instead of inserting the replacement character, +/// as otherwise we may end up with that at the start of a string. +fn bytes_to_utf8_ignore_invalid(v: &[u8]) -> String { + let mut s = String::new(); + for chunk in v.utf8_chunks() { + s.push_str(chunk.valid()); + } + s +} + +/// If the exit status signals it was not successful, return an error. +/// Note that we intentionally *don't* include the command string +/// in the output; we leave it to the caller to add that if they want, +/// as it may be verbose. +fn exit_with_stderr(st: std::process::ExitStatus, mut stderr: std::fs::File) -> Result<()> { + if st.success() { + return Ok(()); + } + // u16 since we truncate to just the trailing bytes here + // to avoid pathological error messages + const MAX_STDERR_BYTES: u16 = 1024; + let size = stderr + .metadata() + .inspect_err(|e| tracing::warn!("failed to fstat: {e}")) + .map(|m| m.len().try_into().unwrap_or(u16::MAX)) + .unwrap_or(0); + let size = size.min(MAX_STDERR_BYTES); + let seek_offset = -(size as i32); + let mut stderr_buf = Vec::with_capacity(size.into()); + // We should never fail to seek()+read() really, but let's be conservative + let stderr_buf = match stderr + .seek(std::io::SeekFrom::End(seek_offset.into())) + .and_then(|_| stderr.read_to_end(&mut stderr_buf)) + { + Ok(_) => bytes_to_utf8_ignore_invalid(&stderr_buf), + Err(e) => { + tracing::warn!("failed seek+read: {e}"); + "".into() + } + }; + anyhow::bail!(format!("Subprocess failed: {st:?}\n{stderr_buf}")) +} + impl CommandRunExt for Command { /// Synchronously execute the child, and return an error if the child exited unsuccessfully. fn run(&mut self) -> Result<()> { - let st = self.status()?; - if !st.success() { - // Note that we intentionally *don't* include the command string - // in the output; we leave it to the caller to add that if they want, - // as it may be verbose. - anyhow::bail!(format!("Subprocess failed: {st:?}")) - } - Ok(()) + let stderr = tempfile::tempfile()?; + self.stderr(stderr.try_clone()?); + exit_with_stderr(self.status()?, stderr) + } +} + +/// Helpers intended for [`tokio::process::Command`]. +#[allow(dead_code)] +pub(crate) trait AsyncCommandRunExt { + async fn run(&mut self) -> Result<()>; +} + +impl AsyncCommandRunExt for tokio::process::Command { + /// Asynchronously execute the child, and return an error if the child exited unsuccessfully. + /// + async fn run(&mut self) -> Result<()> { + let stderr = tempfile::tempfile()?; + self.stderr(stderr.try_clone()?); + exit_with_stderr(self.status().await?, stderr) } } @@ -212,6 +267,43 @@ fn test_sigpolicy_from_opts() { #[test] fn command_run_ext() { + // The basics Command::new("true").run().unwrap(); assert!(Command::new("false").run().is_err()); + + // Verify we capture stderr + let e = Command::new("/bin/sh") + .args(["-c", "echo expected-this-oops-message 1>&2; exit 1"]) + .run() + .err() + .unwrap(); + similar_asserts::assert_eq!( + e.to_string(), + "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected-this-oops-message\n" + ); + + // Ignoring invalid UTF-8 + let e = Command::new("/bin/sh") + .args([ + "-c", + r"echo -e 'expected\xf5\x80\x80\x80\x80-foo\xc0bar\xc0\xc0' 1>&2; exit 1", + ]) + .run() + .err() + .unwrap(); + similar_asserts::assert_eq!( + e.to_string(), + "Subprocess failed: ExitStatus(unix_wait_status(256))\nexpected-foobar\n" + ); +} + +#[tokio::test] +async fn async_command_run_ext() { + use tokio::process::Command as AsyncCommand; + let mut success = AsyncCommand::new("true"); + let mut fail = AsyncCommand::new("false"); + // Run these in parallel just because we can + let (success, fail) = tokio::join!(success.run(), fail.run(),); + success.unwrap(); + assert!(fail.is_err()); }