Use pester for callback requests #483
Conversation
Signed-off-by: liz <[email protected]>
| if err != nil || resp.StatusCode != http.StatusOK { | ||
| errlog.LogError(errors.Wrapf(err, "could not send error message to master URL (%v)", url)) | ||
| if err == nil && resp.StatusCode != http.StatusOK { | ||
| err = fmt.Errorf("unexpected status code %d", resp.StatusCode) |
There was a problem hiding this comment.
this should be errors.Wrap for consistency and bonus the fmt import goes away
There was a problem hiding this comment.
if err is nil, errors.Wrap() returns nil. Passing nil to errlog.LogError() causes a segfault.
that was a fun bug to track down 😓
There was a problem hiding this comment.
wow what
oh well, in that case, i'd return at this point to avoid the import anyway, but if you want to merge that's fine too. up to you, thanks for the fix.
There was a problem hiding this comment.
or fix the nil handling in the error log, that would be ok too
| responseCodes: []int{500, 200}, | ||
| } | ||
|
|
||
| server := httptest.NewTLSServer(testServer) |
There was a problem hiding this comment.
Can this be moved outside of the for loop?
There was a problem hiding this comment.
no, because the test-server's state needs to be reinitialised for every loop.
There was a problem hiding this comment.
oh, I guess I can just set the handler
| if err != nil || resp.StatusCode != http.StatusOK { | ||
| errlog.LogError(errors.Wrapf(err, "could not send error message to master URL (%v)", url)) | ||
| if err == nil && resp.StatusCode != http.StatusOK { | ||
| err = fmt.Errorf("unexpected status code %d", resp.StatusCode) |
Signed-off-by: liz [email protected]
What this PR does / why we need it:
PesterClient wasn't being used for success retries, only for failure
Which issue(s) this PR fixes
Special notes for your reviewer:
Release note: