cli / cli/go-gh

Query and Mutate surface raw HTTP errors instead of *HTTPError

Open
#293 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.