reproc::drain doesn't capture stderr of the child process

[yurivict]

I run the testcase with separate capture of stdout and stderr:

[yuri@yv /usr/home/yuri/testcase]$ ./a.out ./exec.sh 
Hello! (stderr)
stdout: Hello! (stdout)
stderr: [yuri@yv /usr/home/testcase]$ 

stderr from the child process instead goes straight to the calling process' stderr.

testcase.zip

[DaanDeMeyer]

This is documented behavior (see reproc.h, most of the library is documented there since I don't wanna repeat all the documentation in reproc.hpp). The idea here is to make it very "in your face" when a process writes on stderr since that usually indicates something might be going wrong and might ease debugging. That's why we default to writing to the parent stderr for the child process stderr.

However, this can easily be overwritten:

reproc::options options;
options.redirect.err.type = reproc::redirect::pipe;

process.start(args, options);

Now, the child process stderr will write to a pipe just like stdout.

[yurivict]

Okay.

You could make it even more explicit by requiring that reproc::drain's third argument match the state of redirectedness of the stderr pipe. When it is redirected, a valid sink should be required, and when it is not redirected - an empty sink would be required.

For example, by default:

ec = reproc::drain(process, sink, sink);

would throw an exception, but

ec = reproc::drain(process, sink, { });

would succeed.

This would make the user aware of this behavior. Currently you supply a valid sink for stderr but it doesn't even write to it because it isn't redirected. This creates a wrong impression.

[DaanDeMeyer]

Good idea, I'll see what I can do.

[DaanDeMeyer]

Hmm, thinking this through a bit more, doing this would make drain a bit more awkward to use when you don't know for sure if stderr is redirected or not. In some cases, it's quite useful to be able to pass a sink to drain without having to care whether it will receive any data or not.

I added a comment to the drain examples instead, mentioning that stderr is not redirected by default. (Doing that actually uncovered another bug that I promptly fixed)

[yurivict]

But why wouldn't the user know if it's redirected or not?

[DaanDeMeyer]

Imagine you're building some helper function on top of reproc that takes a reproc process and calls reproc::drain on it. It could receive all kinds of processes, some that are redirected and others that are not redirected. Currently, it doesn't need to know whether a process that it receives is redirected or not, it can safely call reproc::drain on it. If we made passing a sink to an non-redirected stream an error, the helper function would now have to do extra checks to see if the process has redirected streams before passing sinks to it.

[yurivict]

Ok, if you think so it's fine.