utils: Capture stderr, add async

We want to capture stderr by default in these methods
so we provide useful errors.

Also add an async variant.

Signed-off-by: Colin Walters <[email protected]>

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}");
+            "<failed to read stderr>".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());
 }