machinebox / graphql

Simple low-level GraphQL HTTP client for Go
https://blog.machinebox.io/a-graphql-client-library-for-go-5bffd0455878
Apache License 2.0
933 stars 217 forks source link

Do response code checks earlier #47

Closed cheshire137 closed 1 year ago

cheshire137 commented 4 years ago

This builds on @erutherford's change in https://github.com/machinebox/graphql/pull/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 πŸ™‡β€β™€

necrophonic commented 4 years ago

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 commented 4 years ago

When can we have this in a release, please? https://github.com/machinebox/graphql/blob/3a92531802258604bd12793465c2e28bc4b2fc85/graphql.go#L142

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

marwan-at-work commented 4 years ago

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 commented 4 years ago

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

JanDonnermayer commented 4 years ago

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 commented 1 year ago

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

matryer commented 1 year ago

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

karelbilek commented 2 months ago

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