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

Exit status should never be zero on failure #21

Closed
lassik opened this issue Jun 27, 2018 · 7 comments
Closed

Exit status should never be zero on failure #21

lassik opened this issue Jun 27, 2018 · 7 comments
Assignees
Labels

Comments

@lassik
Copy link
Contributor

lassik commented Jun 27, 2018

When the unibeautify CLI encounters an exception that causes a stack trace, it currently exits with exit code zero (which conventionally all Unix tools use to indicate a successful exit). We should have some countermeasure so that exit code is never zero on failure. Otherwise users will easily lose valuable source code when an editor plugin or script thinks that source code formatting was successful and trusts the output from unibeautify.

@Glavin001
Copy link
Member

💯 % agree. I consider this a bug. Good catch, @lassik .

@Glavin001 Glavin001 added the bug label Jun 27, 2018
@stevenzeck
Copy link
Contributor

@lassik with the exception of #32, everything should exit with the proper code now. Please give it a try and let us know.

P.S. the package was renamed to @unibeautify/cli, please uninstall unibeautify-cli and use that.

@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

Thanks 😄 I was able to confirm that these now exit nonzero:

$ unibeautify
$ unibeautify -x
$ unibeautify --config-json '{' -l C

Is it the switch to yargs (60b86ee) that addressed this?

I wonder what would be a foolproof way to catch all situations including the likes of #32? Unfortunately, this would be a critically important thing to get right, since editor plugins and especially batch jobs (like CI builds that run automatically on a server so a human doesn't get the chance to verify that the output is correct) will rely on correct code to stdout when the exit code is zero.

It would actually be a reasonable policy to write the program so it always exits nonzero by default, except in the specific few instances where we call exit(0). But I don't know much about Node/JS so I don't know how to go about this.

@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

In any case, we could add safeguards to editor plugins to check that if stdout from CLI is zero bytes long, and the stdin that went to the CLI contained characters other than space/tab/newline/return, then that's an error. At least those Node stack traces seem to go into stderr so stdout stays blank.

@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

Another good thing would be to ensure that errors always use exit code 2 or bigger. Exit codes 0/1 are sometimes useful for indicating true/false (see e.g. issue #19 and "The Cantrip Pattern" in The Art of Unix Programming).

Sorry for being so picky about this, but I know from experience that good use of exit codes makes scripting massively easier. I think now is the best time to establish strict and clean conventions while we can still do so without breaking backwards compatibility 😄

@stevenzeck
Copy link
Contributor

Yep, this is very WIP. I'll close this, and when new things are implemented they will use the proper status codes.

@Glavin001
Copy link
Member

Sorry for being so picky about this, but I know from experience that good use of exit codes makes scripting massively easier.

No apologizes necessary. I 💯% agree and the exit codes should be appropriate to allow for scripting and automation 👍 .

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

No branches or pull requests

3 participants