-
Notifications
You must be signed in to change notification settings - Fork 849
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
Check errors #431
Check errors #431
Changes from 10 commits
8285212
6eb4fe0
00a0145
40fb143
7478058
e01c89c
f177877
a7de7ad
1c94249
9ca7400
9048767
ef112da
bf23f32
ef160cf
f7e4e42
3fa71a0
3b3aa5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,12 @@ import ( | |
"time" | ||
) | ||
|
||
func (c Client) computeHmac256(message string) string { | ||
func (c Client) computeHmac256(message string) (string, error) { | ||
h := hmac.New(sha256.New, c.accountKey) | ||
h.Write([]byte(message)) | ||
return base64.StdEncoding.EncodeToString(h.Sum(nil)) | ||
if _, err := h.Write([]byte(message)); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kinda mixed feelings about changing this. the hmac.inner == sha256 type's Write() will never return an error as everything is in-memory. 😕 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah this change causes many changes in the upstream callers whereas the error can be just discarded. wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And we can then get rid of |
||
return "", fmt.Errorf("Failed computing to hmac256 : %v", err) | ||
} | ||
return base64.StdEncoding.EncodeToString(h.Sum(nil)), nil | ||
} | ||
|
||
func currentTimeRfc1123Formatted() string { | ||
|
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.
if you will have test -z, set -e is not necessary.
instead of this grep hack, perhaps you can do:
can you please check all callers of
getRequestBody()
make sure that error from the request is nil and then go on reading the body? (if error from http.Do/Get/... is non-nil, then resp.Body will be nil, causing the panic indefer resp.Body.Close()
You have
grep -v 'resp.body.Close()'
but I thinkbody
should beBody
?