std::process::Command output() method error handling hazards

[ijackson]

Hi. Normally, Rust makes it difficult to accidentally write buggy code. This is one of its great strengths. However, the API of output() on std::process::Command API has two serious error handling hazards:

1. It requires the user to explicitly fish out the program's exit status and check it.
2. It requires the user to explicitly deal, somehow, with the program's stderr output (if any).

See the example below.

I think this is very difficult to fix with the current return value from output(). It seems to me that there should be a new function whose behaviour is as follows:

• Unless the user explictly called .stderr(...) when building the Command, any nonempty stderr is treated as an error, giving an Err return value (whose Debug impl prints the stderr output).
• Nonzero exit status is treated as an error, giving an Err return value,
• In case of nonzero exit status together with nonempty stderr output (the usual case) the error object contains both (and its Debug impl displays both).
• The returned value is purely the actual stdout.

I don't know what this should be called. Unfortunately the name output has been taken for the more hazardous, raw, function. Which we must retain because if you want to run a command that sometimes succeeds returning nonzero (eg, diff) you need something like it.

(See also #70186 which is about the return value from spawn(). I am about to file another issue about the return value from wait())

use std::process::*;
use std::io::stdout;
use std::io::Write;

fn main() {
    let command = ["ls","--no-such-option"];
    
    let output = Command::new(command[0])
                     .args(&command[1..])
                     .output()
                     .expect("output() failed");
    println!("Output from {:?}:", &command);
    stdout().write_all(output.stdout.as_slice()).expect("write failed");
    
    println!("All went well!")
}

(Playground)

Actual output:

   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.72s
     Running `target/debug/playground`

Standard Output

Output from ["ls", "--no-such-option"]:
All went well!

Expected output:

Some kind of compiler warning. Or something in the docs to say not to use .output() (and, therefore, something convenient to use instead).

[joshtriplett]

Another option would be to add a method to the Command builder, along the lines of .fail_on_stderr(). This would set stderr to a pipe, and also arrange to read it and verify it produces no output. You could then just call Command::new(...).args(...).fail_on_stderr().output(), which seems rather self-documenting.

I was one of the people who commented on the pre-RFC thread, and while I don't think the default for output should change, I absolutely think it'd be reasonable to have a method like this to handle commands for which this is the correct behavior.

Also, in the specific case of ls, I think that code snippet should be checking the exit code of ls; ls does correctly return an error in this case. Do you have examples of common commands that fail and print to stderr but still produce a 0 exit code?

[ijackson]

Another option would be to add a method to the Command builder, along the lines of .fail_on_stderr().

I think this is a good idea.

Also, in the specific case of ls, I think that code snippet should be checking the exit code of ls; ls does correctly return an error in this case. Do you have examples of common commands that fail and print to stderr but still produce a 0 exit code?

My example is an example of code that is wrong precisely because it doesn't check the exit status, but which compiles without warnings. Ie, it demonstrates the hazard that I am complaining about in this issue.

[ijackson]

My example is an example of code that is wrong precisely because it doesn't check the exit status, but which compiles without warnings. Ie, it demonstrates the hazard that I am complaining about in this issue.

I've edited things to make this clearer.

[Zooce]

Just came across this and wanted to mention that there are some programs that return non-zero exit codes in some success scenarios like this: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy#exit-return-codes

It might be good to consider an addition to this where success or failure exit codes can be specified.

[ijackson]

See RFC: rust-lang/rfcs#3362

[Dylan-DPC]

Closing this as it is blocked on a rfc which seems to be stalled, so better to close this and wait and see if the rfc gets accepted and then have a tracking issue out of it