Include the command that failed in the error

[gibfahn]

Using duct has been a great improvement for shell-heavy Rust, but one issue I end up running into a fair bit is that when a user of my CLI hits an error that isn't the command returning a non-zero exit status, the error doesn't tell you what command was run.

As an example, if you run this (as a main.rs in a cargo new after a cargo add duct):

use duct::cmd;

fn main() {
    dbg!(&cmd!("/bin/sh", "-c", "false").run());

    dbg!(&cmd!("non_existent_file").run());

    let tempdir = std::env::temp_dir();

    let permission_denied_path = tempdir.join("permission_denied_path");
    std::fs::write(&permission_denied_path, "#!/bin/sh\ntrue").unwrap();
    dbg!(&cmd!(permission_denied_path).run());

    let bad_interpreter_path = tempdir.join("duct_cmd_test");
    // Note the `\r`, which is illegal on Unix systems:
    std::fs::write(&bad_interpreter_path, "#!/bin/sh\r\ntrue").unwrap();
    cmd!("chmod", "+x", &bad_interpreter_path).run().unwrap();
    dbg!(&cmd!(bad_interpreter_path).run());
}

Then you get the following output:

[src/main.rs:4] &cmd!("/bin/sh", "-c", "false").run() = Err(
    Custom {
        kind: Other,
        error: "command [\"/bin/sh\", \"-c\", \"false\"] exited with code 1",
    },
)
[src/main.rs:6] &cmd!("non_existent_file").run() = Err(
    Os {
        code: 2,
        kind: NotFound,
        message: "No such file or directory",
    },
)
[src/main.rs:12] &cmd!(permission_denied_path).run() = Err(
    Os {
        code: 13,
        kind: PermissionDenied,
        message: "Permission denied",
    },
)
[src/main.rs:17] &cmd!(bad_interpreter_path).run() = Err(
    Os {
        code: 2,
        kind: NotFound,
        message: "No such file or directory",
    },
)

When the command returns a non-zero exit code you get an error that tells you what command was run, but in other failure modes you don't.

It would be really useful to always have the command in the error message, no matter what the error was.

[oconnor663]

Good point, we should fix this. While we're at it, how do you feel about output like "command [\"/bin/sh\", \"-c\", \"false\"] exited with code 1"? If we printed something like "command [/bin/sh -c false] exited with code 1" instead, would that be better for 99% of people? Does anyone really care about quoting whitespace properly in error messages? (Even if we did care, we could backslash-escape spaces instead of quoting.)

[gibfahn]

how do you feel about output like "command [\"/bin/sh\", \"-c\", \"false\"] exited with code 1"?

I always use color-eyre in my binary crates, and that one actually prints it much more nicely.

Error:
   0: command ["/bin/sh", "-c", "false"] exited with code 1

Having said that, it would be really nice to be able to copy the error message command and paste it into a terminal directly. For that purpose I ended up adding a wrapper so I could log every command being run:

/// Wrapper around `duct::cmd` function that lets us log the command we're running.
pub fn cmd_log<T, U>(l: log::Level, program: T, args: U) -> duct::Expression
where
    T: duct::IntoExecutablePath + Clone,
    U: IntoIterator + Clone,
    U::Item: Into<OsString>,
{
    let mut formatted_cmd = format!(
        "Running cmd: {program}",
        program = shell_escape::escape(program.clone().to_executable().to_string_lossy())
    );
    for arg in args.clone() {
        write!(
            formatted_cmd,
            " {arg}",
            arg = shell_escape::escape(arg.into().to_string_lossy())
        )
        .unwrap();
    }

    log::log!(l, "{formatted_cmd}");
    duct::cmd(program, args)
}

This prints something like:

INFO Running command: /bin/sh -c 'echo normal string'
INFO Running command: /bin/sh -c 'echo double quoted "foo" string'
INFO Running command: /bin/sh -c false

Which avoids using quotes unless you actually need them (and which I find nicer to read).

The need to .clone() makes me sad, but don't think that can be fixed without this existing in duct itself.

Does anyone really care about quoting whitespace properly in error messages?

I find that spaces are quite common in commands I end up running (especially on macOS, where lots of system paths have spaces in them).