Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to not throw on non-zero exit code #60

Open
vjpr opened this issue Feb 26, 2019 · 6 comments
Open

Option to not throw on non-zero exit code #60

vjpr opened this issue Feb 26, 2019 · 6 comments

Comments

@vjpr
Copy link

vjpr commented Feb 26, 2019

It seems to throw on non-zero exit code. I would like to avoid wrapping in try/catch, and read the code.

The documentation has an example showing usage of the {code} in the result. This is confusing because you cannot read the code it if throws.

@vjpr
Copy link
Author

vjpr commented Feb 26, 2019

The default behavior is fine as it matches Node's promisify behaviour: https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

If this method is invoked as its util.promisify()ed version, it returns a Promise for an Object with stdout and stderr properties. In case of an error (including any error resulting in an exit code other than 0), a rejected promise is returned, with the same error object given in the callback, but with an additional two properties stdout and stderr.

Accessing the code from the result should be removed from the docs, or an example should be added showing the need to try/catch on failure.

@jedwards1211
Copy link
Member

jedwards1211 commented Feb 26, 2019

@vjpr Yeah I need to improve the docs, thanks for pointing this out. If it throws, you can read error.code, error.signal, just like child_process.exec. This lib also attaches stdout and stderr to the error it throws. Instead of wrapping in try/catch, you can add .catch(e => e) (even within an async function) and then read code, signal, stdout, and stderr on the result regardless of whether it threw or not.
However, I should also attach these properties in the case the ChildProcess emitted an error...
And I should add code/signal to the result if it succeeded as well.

@vjpr
Copy link
Author

vjpr commented Feb 26, 2019

.catch(e => e) was what I was needing. Thanks!

However, I should also attach these properties

Would be good.

@ranasaria
Copy link

ranasaria commented Oct 11, 2019

would the following code work to capture exit code in all success and failure cases?

child.on('exit', (code) => {
       doWhateverWithCode(code);
});

@jedwards1211
Copy link
Member

@ranasaria Well if the process exits because of a signal, code will be undefined, instead the signal argument will be defined. Otherwise that should work.

@mattalxndr
Copy link

@vjpr Yeah I need to improve the docs, thanks for pointing this out.

I read your docs, noticed that you were returning the exit code, and assumed that I would not need to catch an exception if the child process exited non-zero. I would recommend changing either the behavior or the docs.

Thanks for the lib 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants