Query and Mutate surface raw HTTP errors instead of *HTTPError
- Dominant language
- Go
- Stars
- 438
- Forks
- 94
- Avg merge
- 5d 13h
- Merged PRs (30d)
- 6
Description
### Bug
GraphQL calls through `GraphQLClient.Query`/`Mutate` turn non-2xx HTTP responses into a raw, unstructured error:
```
non-200 OK status code: 401 Unauthorized body: "{\"message\":\"Bad credentials\",...}"
```
This is visible to users as, e.g., `gh pr status` / `gh gist list` / `gh release list` failing with that raw string when the token is invalid (see cli/cli#8434). The caller cannot parse the status code, headers, or body, because the underlying GraphQL client (cli/shurcooL-graphql) formats the whole response into a plain `fmt.Errorf` string.
The `Do`/`DoWithContext` methods handle this properly via `HandleHTTPError` (returning `*HTTPError`); only the named `QueryNamed`/`MutateNamed` path leaks the raw error.
### Proposed approach
- Wrap the inner GraphQL HTTP client's transport so that responses outside the 2xx range become an `*HTTPError` (existing `HandleHTTPError`), so the underlying GraphQL client never formats the raw response.
- In `QueryWithContext`/`MutateWithContext`, unwrap the `*url.Error` that `net/http` adds around round-trip failures and return the bare `*HTTPError`, consistent with the `Do` path.
After the change, an unauthenticated `gh pr status` shows:
```
HTTP 401: Bad credentials (https://api.github.com/graphql)
Try authenticating with: gh auth login
```
This matches the approach suggested by @williammartin in cli/cli#8434 ("capture this error in go-gh and wrap correctly").
I have an implementation ready with tests (query/mutate/do HTTP-error cases) that I'm happy to submit as a PR if you give the go-ahead.
*Implementation note: prepared with assistance from an AI coding tool; the change and its tests were manually verified against the repository.*
Contributor guide
Research direction
Start by reading QueryWithContext and MutateWithContext alongside Do/DoWithContext and the existing HandleHTTPError path. Trace QueryNamed/MutateNamed through the GraphQL client's transport and inspect the query, mutate, and do HTTP-error tests mentioned in the issue. Done means non-2xx responses return a bare *HTTPError with structured status, headers, and body, including the unauthenticated GraphQL case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, graphql
- Domain
- api
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100