-
Notifications
You must be signed in to change notification settings - Fork 19
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
fix(errors): return actionable errors from API #13
Conversation
I also intend to add a |
case 400: | ||
bodyMap := make(map[string]interface{}) | ||
if err := json.NewDecoder(res.Body).Decode(&bodyMap); err != nil { | ||
return ErrParsing |
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.
perhaps we should also return the response body so the user can determine the error on their own?
if err != nil { | ||
return api.App{}, err | ||
} | ||
// Fix json.Decoder bug in <go1.7 |
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.
Maybe this should be // TODO: Remove when json.Decoder bug is fixed (go >= 1.7)
? It's unfortunate that this has to be repeated everywhere Decode
is used.
Also do you have a reference for this json.Decoder issue? Just curious.
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.
It was caught in google's github-api repo: google/go-github#317
I'll make a PR to reformat the warning to what you suggested everywhere.
This is no longer WIP. @helgi you had some concerns about my approach, could you please review? |
@bacongobbler You accidentally LGTMed this twice ❤️! I removed your second LGTM so somebody else can review it. |
whoops :P |
I'd rather want to fix the API where possible with more identifable information than to parse things like this but good enough for now. An improvement for sure |
This tries to strike a balance between supporting every possible error, which is infeasible because of how many (undocumented) errors exist, and supporting the most useful errors.
The most useful errors have their own type, and the less common errors return a unknown error.