Do response code checks earlier

[cheshire137]

This builds on @erutherford's change in #19 by moving the status code checks earlier. The library was only checking the response code if there was a problem decoding the response body as JSON, but a non-200 response from the API indicates a problem even if the response body was valid JSON. For example, the GitHub GraphQL API will return the valid JSON {"message":"Bad credentials","documentation_url":"https://developer.github.com/v4"} if you give bad credentials.

cc @matryer 🙇‍♀

[danp]

This should come after defer res.Body.Close(). As mentioned in the http.Client.Do docs:

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close.

Similar below.

[necrophonic]

Is there any chance we could return the response from the server, potentially as part of the error, if there was one?

In your case above (and actually the exact issue I'm having!) it would be helpful to have the message from the server as well as the status code error.

[saopayne]

When can we have this in a release, please?

graphql/graphql.go

Line 142 in 3a92531

return fmt.Errorf("graphql: server returned a non-200 status code: %v", res.StatusCode)

We need this check and it's not available in V2.2. Can master be promoted to a release?

[marwan-at-work]

I think it would be great if we used Go 1.13 primitives to return a friendly error response while still being able to get the un-modified response body and status code from the user side, something like this:

type StatusError struct {
  Code int
  Body []byte
}

func (se *StatusError) Error() string {
  return fmt.Sprintf("unexpected response code: %d", se.Code)
}

// later on
if resp.StatusCode != 200 {
  body, _ := ioutil.ReadAll(resp.Body)
  return &StatusError{Code: resp.StatusCode, Body: body}
}

This way, a user can see a nicely formatted error but also be able to inspect both the code and the body by doing the following:

import "errors"

var statusErr *graphql.StatusError
if errors.As(&statusErr) {
  json.Unmarshal(statusErr.Body, &myCustomStruct) // etc etc
}

Just FYI, users of Go version 1.12 and lower can still leverage the code above by just type-casting the error value.

[sminf]

According to @marwan-at-work's proposal, I wrote a GraphQL client for better error handling.
poohvpn/gqlgo

[JanDonnermayer]

According to @marwan-at-work's proposal, I wrote a GraphQL client for better error handling.
poohvpn/gqlgo

Thx! This client is much better, yet better documented

[cheshire137]

I found this PR still lingering, I'm going to close it out as stale. Thanks all!

[matryer]

Sorry about this. We lost control of the repo, so it’s forked at https://github.com/matryer/graphql which we can control.

[karelbilek]

Who can control the repo? It should be more clearly stated that it's unmaintained