How to get rejected promise instead of output.stderr

[GaikwadPratik]

trafficstars

@jedwards1211 ,

This is more like a question/suggestion than issue.

Currently, the library returns an object with {stderr: string| undefined, stdout: string|undefined}. I think it would make more sense, since it is promise based, if it returns a rejected promise instead of stderr. That way, the consumer does not have to look for stderr specifically but they can use a catch block instead and get a single string in stdout instead of object. I think that would simplify a lot of code and would improve readability.

So question is, was there there any specific reason it is not implemented? Can this be taken under consideration and implemented?

Also this section says, it will return rejected promise in case of error which I think is misleading/confusing.

[jedwards1211]

That section says:

The child process promise will only resolve if the process exits with a code of 0. If it exits with any other code, is killed by a signal, or emits an 'error' event, the promise will reject.

It's hard for me to tell how you interpreted that section...'error' events are emitted if there was some problem even spawning the process, not if it exited or got killed or outputted to stderr. The process can write to stderr even if it exits with 0 (and the promise resolves), or it could exit 1 or get killed without writing anything to stderr. So stderr output doesn't necessarily indicate an error.

However, you have a point, it is tempting to make the promise resolve to just stdout (string or Buffer) like how execSync returns only stdout. I hadn't considered this; Node's async exec callback receives both stdout and stderr as arguments so there's sort of a mismatch. Since this would be a breaking change though and could cause disruption, I guess I should figure out a way to have users of this package vote on it.

[GaikwadPratik]

I understand your premise about stderr being populated even child_process exits quietly. but I think, from node docs, stderr will always be filled when an error is generated. Although you are right, it would be tough to decide what to do if spawning fails.

You make a good point about APIs breaking changes. But I would have it sooner than later as it simplifies coding design a whole lot simpler. :)

[douglascayers]

Throwing my two cents here...

I agree with @jedwards1211 that a child process can write to stdout, stderr, both, or neither. The exit code is the most reliable indicator if the child process actually had an error.

A convention with shell scripts is to log information to stderr while returning the actual result for piping to other commands to stdout. As long as the script exits with code 0 then all good. Any other exit code indicates an error and then it's up to the documentation/api of the script being executed on what stdout and stderr might even mean.

For CI/CD scripts, I use stderr extensively for logging details for troubleshooting and showing progress but only write to stdout if the script is expected to return a value. I would not want my script to be considered failed or rejected because I wrote to stderr since my exit code was 0 (success).

[jedwards1211]

Yeah don't worry, I would never make it reject just because stderr gets written to. As far as the other aspect of what OP was saying though, would you consider it more convenient if await exec(...) resolved to just the stdout? I guess if I made that change I could add an instance method you can await to get both stdout and stderr.

[jedwards1211]

@GaikwadPratik

from node docs, stderr will always be filled when an error is generated

Can you give me a link to where you read about this?

I'm assuming you read that node will output to stderr if there is an uncaught exception or unhandled promise rejection. This doesn't imply that anything that could cause the process to exit with a nonzero code is guaranteed to write to stderr: a program could catch an error and process.exit(1) without writing to stderr, and the exit code of 1 would still mean something went wrong.

[douglascayers]

Yeah don't worry, I would never make it reject just because stderr gets written to.

👍

As far as the other aspect of what OP was saying though, would you consider it more convenient if await exec(...) resolved to just the stdout? I guess if I made that change I could add an instance method you can await to get both stdout and stderr.

Personally, I think the API is fine how you have it. Using destructuring I can grab just stdout, stderr, or the whole response in one statement.

const response = await exec(...);

const { stdout, stderr } = await exec(...);

const { stdout } = await exec(...);

[jedwards1211]

Right, it would basically save a few keystrokes. I am about saving keystrokes, but... another thing is it's possible to use a wrapper function to return stdout directly (maybe I should export one).