-
Notifications
You must be signed in to change notification settings - Fork 7
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
Adding timeout vault option and better error handling #5
Conversation
bartlettc22
commented
Feb 26, 2019
- Adding config value vault-timeout (default 15 seconds)
- Adding better error handling for Vault operations
) | ||
|
||
// Error is the custom error type for this package | ||
type Error struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel like this should be named something more specific, to easy to confuse error and Error and mismatch them. I would say something like StimError, or even StimVaultError
@@ -11,17 +10,17 @@ func (v *Vault) GetSecretKey(path string, key string) (string, error) { | |||
|
|||
secret, err := v.client.Logical().Read(path) | |||
if err != nil { | |||
return "", err | |||
return "", v.parseError(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume you want to return Error (or whatever we might change it too) so if someone wants to handle it a different way farther down (say another stimpack calling it) they would not have to see if its castable.
MessageParts []string | ||
OriginalError error | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to add a String() func to this so if someone outputs it in a log things look correct. https://tour.golang.org/methods/17
All in all looks good, added a few comments you might want to look through. |