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

Improve error handling #68

Merged
merged 5 commits into from
Mar 23, 2019
Merged

Improve error handling #68

merged 5 commits into from
Mar 23, 2019

Conversation

1dustindavis
Copy link
Owner

This PR improves error handling of panics by recovering from any expected panics, printing the error, and then calling os.Exit(1).

Current situations where we expect a panic:

  • Unable to write log file
  • Unable to download a catalog
  • Unable to parse a catalog
  • Unable to download a manifest
  • Unable to parse a manifest

My current thinking is that these are all situations that, if we continued past the error, may cause undesired or unknown effects, so our best option is to exit.

This would resolve #65 and #57. It would also contribute towards #15.

This PR also adjusts the arguments passed to choco so that it will output more details and we will catch those in our logs, which should address #58.

@1dustindavis
Copy link
Owner Author

1dustindavis commented Mar 21, 2019

Here are examples of the output when the download fails(404)

Normal Output (not verbose):

> gorilla
Unable to retrieve manifest: employee.yaml : Download status code: 404

Verbose Output

> gorilla -v
Retrieving manifest: employee
Manifest Url: https://example.com/gorilla/manifests/employee.yaml
Unable to retrieve manifest: employee.yaml : Download status code: 404

@1dustindavis
Copy link
Owner Author

While testing, I found a bad bug that the a previous PR created. We were only using item.Uninstaller.Location, not item.Installer.Location, so all installs failed 😑.

The latest commit to this PR fixes that issue, but I don't want to merge this until new tests are written to validate the URL.

@1dustindavis 1dustindavis merged commit a074339 into master Mar 23, 2019
@1dustindavis 1dustindavis deleted the error-handling branch March 23, 2019 16:51
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.

403 causes panic
1 participant