From fa02eafd63fefd3e5d9467745cb8b717e01c6aa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Mon, 21 Sep 2026 14:14:38 +0300 Subject: [PATCH 1/2] fix(api): normalize GraphQL HTTP errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Efe Gökdemir --- pkg/api/graphql_client.go | 46 ++++++++++++++++++++------------ pkg/api/graphql_client_test.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/pkg/api/graphql_client.go b/pkg/api/graphql_client.go index 2b12c3b..e202bbe 100644 --- a/pkg/api/graphql_client.go +++ b/pkg/api/graphql_client.go @@ -56,8 +56,11 @@ func NewGraphQLClient(opts ClientOptions) (*GraphQLClient, error) { endpoint = swapHost(endpoint, opts.APIHost) } + graphqlHTTPClient := *httpClient + graphqlHTTPClient.Transport = graphqlErrorRoundTripper{rt: httpClient.Transport} + return &GraphQLClient{ - client: graphql.NewClient(endpoint, httpClient), + client: graphql.NewClient(endpoint, &graphqlHTTPClient), endpoint: endpoint, httpClient: httpClient, }, nil @@ -122,6 +125,10 @@ func (c *GraphQLClient) Do(query string, variables map[string]interface{}, respo // Provided input will be set as a variable named input. func (c *GraphQLClient) MutateWithContext(ctx context.Context, name string, m interface{}, variables map[string]interface{}) error { err := c.client.MutateNamed(ctx, name, m, variables) + return normalizeGraphQLError(err) +} + +func normalizeGraphQLError(err error) error { var graphQLErrs graphql.Errors if err != nil && errors.As(err, &graphQLErrs) { items := make([]GraphQLErrorItem, len(graphQLErrs)) @@ -134,8 +141,14 @@ func (c *GraphQLClient) MutateWithContext(ctx context.Context, name string, m in Type: e.Type, } } - err = &GraphQLError{items} + return &GraphQLError{items} } + + var httpErr *HTTPError + if err != nil && errors.As(err, &httpErr) { + return httpErr + } + return err } @@ -151,21 +164,7 @@ func (c *GraphQLClient) Mutate(name string, m interface{}, variables map[string] // to the GitHub GraphQL schema. func (c *GraphQLClient) QueryWithContext(ctx context.Context, name string, q interface{}, variables map[string]interface{}) error { err := c.client.QueryNamed(ctx, name, q, variables) - var graphQLErrs graphql.Errors - if err != nil && errors.As(err, &graphQLErrs) { - items := make([]GraphQLErrorItem, len(graphQLErrs)) - for i, e := range graphQLErrs { - items[i] = GraphQLErrorItem{ - Message: e.Message, - Locations: e.Locations, - Path: e.Path, - Extensions: e.Extensions, - Type: e.Type, - } - } - err = &GraphQLError{items} - } - return err + return normalizeGraphQLError(err) } // Query wraps QueryWithContext using context.Background. @@ -178,6 +177,19 @@ type graphQLResponse struct { Errors []GraphQLErrorItem } +type graphqlErrorRoundTripper struct { + rt http.RoundTripper +} + +func (rt graphqlErrorRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := rt.rt.RoundTrip(req) + if err != nil || resp.StatusCode >= 200 && resp.StatusCode < 300 { + return resp, err + } + + return resp, HandleHTTPError(resp) +} + func graphQLEndpoint(host string) string { if isGarage(host) { return fmt.Sprintf("https://%s/api/graphql", host) diff --git a/pkg/api/graphql_client_test.go b/pkg/api/graphql_client_test.go index 46c9db3..3680a66 100644 --- a/pkg/api/graphql_client_test.go +++ b/pkg/api/graphql_client_test.go @@ -103,6 +103,54 @@ func TestGraphQLClientMutateError(t *testing.T) { assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } +func TestGraphQLClientQueryHTTPError(t *testing.T) { + testutils.StubConfig(t, testConfig()) + t.Cleanup(gock.Off) + + gock.New("https://api.github.com"). + Post("/graphql"). + MatchHeader("Authorization", "token abc123"). + Reply(http.StatusUnauthorized). + JSON(`{"message":"Bad credentials"}`) + + client, err := DefaultGraphQLClient() + assert.NoError(t, err) + + var res struct{ Viewer struct{ Login string } } + err = client.Query("QUERY", &res, nil) + var httpErr *HTTPError + assert.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusUnauthorized, httpErr.StatusCode) + assert.Equal(t, "Bad credentials", httpErr.Message) + assert.EqualError(t, err, "HTTP 401: Bad credentials (https://api.github.com/graphql)") + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) +} + +func TestGraphQLClientMutateHTTPError(t *testing.T) { + testutils.StubConfig(t, testConfig()) + t.Cleanup(gock.Off) + + gock.New("https://api.github.com"). + Post("/graphql"). + MatchHeader("Authorization", "token abc123"). + Reply(http.StatusForbidden). + JSON(`{"message":"Forbidden"}`) + + client, err := DefaultGraphQLClient() + assert.NoError(t, err) + + var mutation struct { + UpdateRepository struct{ Repository struct{ Name string } } + } + err = client.Mutate("MUTATE", &mutation, nil) + var httpErr *HTTPError + assert.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusForbidden, httpErr.StatusCode) + assert.Equal(t, "Forbidden", httpErr.Message) + assert.EqualError(t, err, "HTTP 403: Forbidden (https://api.github.com/graphql)") + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) +} + func TestGraphQLClientDo(t *testing.T) { testutils.StubConfig(t, "") From 2df8e859f45a53dcf12331126aa90755aff74281 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 27 Oct 2026 04:02:30 +0300 Subject: [PATCH 2/2] fix(api): keep GraphQL HTTP responses in transport --- pkg/api/graphql_client.go | 81 ++++++++++++++++++++++++++++++---- pkg/api/graphql_client_test.go | 41 +++++++++++++++++ 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/pkg/api/graphql_client.go b/pkg/api/graphql_client.go index e202bbe..c4680d8 100644 --- a/pkg/api/graphql_client.go +++ b/pkg/api/graphql_client.go @@ -57,7 +57,7 @@ func NewGraphQLClient(opts ClientOptions) (*GraphQLClient, error) { } graphqlHTTPClient := *httpClient - graphqlHTTPClient.Transport = graphqlErrorRoundTripper{rt: httpClient.Transport} + graphqlHTTPClient.Transport = graphQLHTTPErrorCaptureRoundTripper{rt: httpClient.Transport} return &GraphQLClient{ client: graphql.NewClient(endpoint, &graphqlHTTPClient), @@ -124,11 +124,19 @@ func (c *GraphQLClient) Do(query string, variables map[string]interface{}, respo // to the GitHub GraphQL schema. // Provided input will be set as a variable named input. func (c *GraphQLClient) MutateWithContext(ctx context.Context, name string, m interface{}, variables map[string]interface{}) error { + capture := &graphQLHTTPErrorCapture{} + ctx = context.WithValue(ctx, graphQLHTTPErrorCaptureKey{}, capture) err := c.client.MutateNamed(ctx, name, m, variables) - return normalizeGraphQLError(err) + return normalizeGraphQLError(err, capture) } -func normalizeGraphQLError(err error) error { +func normalizeGraphQLError(err error, capture *graphQLHTTPErrorCapture) error { + if err != nil { + if httpErr := capture.httpError(); httpErr != nil { + return httpErr + } + } + var graphQLErrs graphql.Errors if err != nil && errors.As(err, &graphQLErrs) { items := make([]GraphQLErrorItem, len(graphQLErrs)) @@ -163,8 +171,10 @@ func (c *GraphQLClient) Mutate(name string, m interface{}, variables map[string] // The query argument should be a pointer to struct that corresponds // to the GitHub GraphQL schema. func (c *GraphQLClient) QueryWithContext(ctx context.Context, name string, q interface{}, variables map[string]interface{}) error { + capture := &graphQLHTTPErrorCapture{} + ctx = context.WithValue(ctx, graphQLHTTPErrorCaptureKey{}, capture) err := c.client.QueryNamed(ctx, name, q, variables) - return normalizeGraphQLError(err) + return normalizeGraphQLError(err, capture) } // Query wraps QueryWithContext using context.Background. @@ -177,17 +187,72 @@ type graphQLResponse struct { Errors []GraphQLErrorItem } -type graphqlErrorRoundTripper struct { +type graphQLHTTPErrorCaptureKey struct{} + +type graphQLHTTPErrorCapture struct { + response *http.Response + body bytes.Buffer +} + +func (capture *graphQLHTTPErrorCapture) reset() { + capture.response = nil + capture.body.Reset() +} + +func (capture *graphQLHTTPErrorCapture) errorResponse(resp *http.Response, req *http.Request) { + copy := *resp + copy.Header = resp.Header.Clone() + copy.Body = nil + copy.Request = req + capture.response = © + if resp.Body != nil { + resp.Body = &graphQLHTTPErrorCaptureBody{ReadCloser: resp.Body, capture: capture} + } +} + +func (capture *graphQLHTTPErrorCapture) httpError() *HTTPError { + if capture.response == nil { + return nil + } + + resp := *capture.response + resp.Body = io.NopCloser(bytes.NewReader(capture.body.Bytes())) + httpErr, _ := HandleHTTPError(&resp).(*HTTPError) + return httpErr +} + +type graphQLHTTPErrorCaptureBody struct { + io.ReadCloser + capture *graphQLHTTPErrorCapture +} + +func (body *graphQLHTTPErrorCaptureBody) Read(p []byte) (int, error) { + n, err := body.ReadCloser.Read(p) + if n > 0 { + _, _ = body.capture.body.Write(p[:n]) + } + return n, err +} + +// graphQLHTTPErrorCaptureRoundTripper keeps HTTP errors as responses so the +// GraphQL client can close the body, while retaining details for normalization. +type graphQLHTTPErrorCaptureRoundTripper struct { rt http.RoundTripper } -func (rt graphqlErrorRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { +func (rt graphQLHTTPErrorCaptureRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + capture, _ := req.Context().Value(graphQLHTTPErrorCaptureKey{}).(*graphQLHTTPErrorCapture) + if capture != nil { + capture.reset() + } + resp, err := rt.rt.RoundTrip(req) - if err != nil || resp.StatusCode >= 200 && resp.StatusCode < 300 { + if err != nil || resp == nil || resp.StatusCode >= 200 && resp.StatusCode < 300 || capture == nil { return resp, err } - return resp, HandleHTTPError(resp) + capture.errorResponse(resp, req) + return resp, nil } func graphQLEndpoint(host string) string { diff --git a/pkg/api/graphql_client_test.go b/pkg/api/graphql_client_test.go index 3680a66..30ee140 100644 --- a/pkg/api/graphql_client_test.go +++ b/pkg/api/graphql_client_test.go @@ -3,7 +3,9 @@ package api import ( "context" "errors" + "io" "net/http" + "strings" "testing" "time" @@ -12,6 +14,12 @@ import ( "gopkg.in/h2non/gock.v1" ) +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func TestGraphQLClient(t *testing.T) { testutils.StubConfig(t, testConfig()) t.Cleanup(gock.Off) @@ -151,6 +159,39 @@ func TestGraphQLClientMutateHTTPError(t *testing.T) { assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } +func TestGraphQLErrorRoundTripperReturnsResponseWithoutTransportError(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "https://api.github.com/graphql", nil) + assert.NoError(t, err) + headers := make(http.Header) + headers.Set("Content-Type", "application/json") + headers.Set("X-GitHub-Request-Id", "test-request-id") + resp := &http.Response{ + Status: "401 Unauthorized", + StatusCode: http.StatusUnauthorized, + Header: headers, + Body: io.NopCloser(strings.NewReader(`{"message":"Bad credentials"}`)), + Request: req, + } + capture := &graphQLHTTPErrorCapture{} + req = req.WithContext(context.WithValue(req.Context(), graphQLHTTPErrorCaptureKey{}, capture)) + roundTripper := graphQLHTTPErrorCaptureRoundTripper{rt: roundTripperFunc(func(*http.Request) (*http.Response, error) { + return resp, nil + })} + + got, err := roundTripper.RoundTrip(req) + assert.NoError(t, err) + assert.Same(t, resp, got) + body, err := io.ReadAll(got.Body) + assert.NoError(t, err) + assert.Equal(t, `{"message":"Bad credentials"}`, string(body)) + assert.NoError(t, got.Body.Close()) + + httpErr := capture.httpError() + assert.Equal(t, http.StatusUnauthorized, httpErr.StatusCode) + assert.Equal(t, "Bad credentials", httpErr.Message) + assert.Equal(t, "test-request-id", httpErr.Headers.Get("X-GitHub-Request-Id")) +} + func TestGraphQLClientDo(t *testing.T) { testutils.StubConfig(t, "")