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

fix: exit with generic non-zero code when there is a general error #161

Merged
merged 1 commit into from
Jan 27, 2023
Merged

fix: exit with generic non-zero code when there is a general error #161

merged 1 commit into from
Jan 27, 2023

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Jan 22, 2023

Addresses something I noticed in #94 - I was expecting it to require more hoop jumping but realised this should be a pretty robust/correct solution.

Right now there is only one primary cases where an error is printed but the actual error does not result in the function bailing out, though this will be expanded a little by the changes in #94.

(this'll need rebasing when #158 is landed)

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Looks good, there are some printed errors in the parser code that does not go through the reporter, but I think the fix for that is to just have it use the reporter as well.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Jan 23, 2023

Yeah those errors are really "soft panics" - I've resisted using the reporter on lockfile for those because it would be a substantial API change (both in terms of the actual signatures + the fact that it means anyone using the parsers as a library would then-on have to pass this whole other structure, which feels very unwieldy), and for the most part they represent situations that are theoretical but can't be represented in Go currently (i.e. parsing strings that are the result of regexps that select only numbers, into numbers)

If you feel really strongly about them, I'd recommend changing them to actual panics.

@another-rex another-rex merged commit e569787 into google:main Jan 27, 2023
@G-Rath G-Rath deleted the improve-exit-codes branch January 27, 2023 00:48
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
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

Successfully merging this pull request may close these issues.

2 participants