Improve error message for askpass.#407
Improve error message for askpass.#407k8s-ci-robot merged 1 commit intokubernetes:masterfrom briantkennedy:response
Conversation
|
Welcome @briantkennedy! |
thockin
left a comment
There was a problem hiding this comment.
Can I get an equivalent PR against the master (v4 dev) branch?
| }() | ||
| if resp.StatusCode != 200 { | ||
| return fmt.Errorf("auth URL returned status %d", resp.StatusCode) | ||
| errMessage, err2 := ioutil.ReadAll(resp.Body) |
There was a problem hiding this comment.
you can shadow err here - it's OK, unless some linter is complaining?
| if err2 != nil { | ||
| return fmt.Errorf("auth URL returned status %d, failed to read body: %w", resp.StatusCode, err2) | ||
| } | ||
| return fmt.Errorf("auth URL returned status %d body %s", resp.StatusCode, string(errMessage)) |
There was a problem hiding this comment.
nit: "%d, body: %s" or even %q ?
briantkennedy
left a comment
There was a problem hiding this comment.
I don't see a v4 branch, do you mean the release-3.x branch?
| if err2 != nil { | ||
| return fmt.Errorf("auth URL returned status %d, failed to read body: %w", resp.StatusCode, err2) | ||
| } | ||
| return fmt.Errorf("auth URL returned status %d body %s", resp.StatusCode, string(errMessage)) |
When endpoint returns non-200 status, include the body in the error message since it can contain useful information for debugging. Also defer closing the response body ReadCloser as this may have leaked in the past.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: briantkennedy, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
When endpoint returns non-200 status, include the body in the error
message since it can contain useful information for debugging. Also
defer closing the response body ReadCloser as this may have leaked in
the past.