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

makes cli application return non-zero error code on errors #541

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

vroldanbet
Copy link
Contributor

fixes #538

changes the application to return non-zero error codes when an error is returned by the root command.

I attempted to make the output of the CLI "pretty":

  • if the error is a parsing error, command usage will be printed
  • if the error is an application error, the error is logged using zerolog so that it shows up nicely just like any other application log message

If we wanted the flag values to be recognized as a "parsing/cli error" (e.g. unrecognized datastore) we could signal it with a specific error, but we would need to adjust every part of the application that initializes spicedb to return an error deemed to be considered "cli error". I considered that to be out of scope of this PR.

example: unrecognized flag: error code 1, prints command usage

Screen Shot 2022-04-19 at 19 39 22

example: incorrect flag value: error code 1, does not print command usage

Screen Shot 2022-04-19 at 19 25 20

example: SIGTERM: error code 0

Screen Shot 2022-04-19 at 19 29 03

@vroldanbet vroldanbet requested a review from a team as a code owner April 19, 2022 17:42
@github-actions github-actions bot added the area/CLI Affects the command line label Apr 19, 2022
Copy link
Member

@jzelinskie jzelinskie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could also be done with a new error type that wraps the flag error then using errors.As at the Execute() callsite.

I don't think it really makes much of a difference, though.
LGTM

- if the error is a parsing error, command usage will be printed
- if the error is an application error, the error is logged using zerolog
so that it shows up nicely just like any other application log message

examples:
- incorrect flag: error code 1, prints command usage
- incorrect flag value: error code 1, does not print command usage
- SIGTERM: error code 0
@jzelinskie jzelinskie merged commit cf1dc84 into authzed:main Apr 21, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2022
@vroldanbet vroldanbet deleted the non-zero-exit-code branch April 21, 2022 07:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/CLI Affects the command line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spicedb CLI returns error code 0 regardless of application errors
2 participants