-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
maintain underlying error structs to allow for type conversion #293
Changes from 4 commits
eb4cd51
800d8fd
3f13428
d34941c
4b60aa1
47cac6c
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 |
---|---|---|
|
@@ -144,15 +144,16 @@ func (c *Client) handleErrorResp(resp *http.Response) error { | |
var errRes ErrorResponse | ||
err := json.NewDecoder(resp.Body).Decode(&errRes) | ||
if err != nil || errRes.Error == nil { | ||
reqErr := RequestError{ | ||
reqErr := &RequestError{ | ||
HTTPStatusCode: resp.StatusCode, | ||
Err: err, | ||
} | ||
if errRes.Error != nil { | ||
reqErr.Err = errRes.Error | ||
} | ||
return fmt.Errorf("error, %w", &reqErr) | ||
return reqErr | ||
} | ||
|
||
errRes.Error.HTTPStatusCode = resp.StatusCode | ||
return fmt.Errorf("error, status code: %d, message: %w", resp.StatusCode, errRes.Error) | ||
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. I wonder if the previous version was working with the 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. yes you are right. I ran into the issue because I was using type conversion instead of errors.As. I suppose the extra information in the documentation would have negated the need for this PR. However I still think it is an improvement for the error type to handle its own messaging and formatting. Also it doesn't hurt to also allow type conversion even if it is no longer the idiomatic methodology |
||
return errRes.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.
Could we please wrap this into
<details>
tag as it is done below?