From f1f97bc86cb82096e666aa2d25d37209609906a4 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Sun, 8 May 2022 22:27:28 -0700 Subject: [PATCH 1/3] Export GraphQL error types --- gh_test.go | 26 ++++++++++++++++++++++++++ internal/api/gql_client.go | 22 ++-------------------- pkg/api/client.go | 22 ++++++++++++++++++++++ 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/gh_test.go b/gh_test.go index ade5f1b..a9e5b93 100644 --- a/gh_test.go +++ b/gh_test.go @@ -105,6 +105,32 @@ func TestGQLClient(t *testing.T) { assert.Equal(t, "hubot", res.Viewer.Login) } +func TestGQLClientError(t *testing.T) { + tempDir := t.TempDir() + orig_GH_CONFIG_DIR := os.Getenv("GH_CONFIG_DIR") + orig_GH_TOKEN := os.Getenv("GH_TOKEN") + t.Cleanup(func() { + os.Setenv("GH_CONFIG_DIR", orig_GH_CONFIG_DIR) + os.Setenv("GH_TOKEN", orig_GH_TOKEN) + }) + os.Setenv("GH_CONFIG_DIR", tempDir) + os.Setenv("GH_TOKEN", "GH_TOKEN") + + http := httpmock.NewRegistry(t) + http.Register( + httpmock.GQL("QUERY"), + httpmock.StringResponse(`{"errors":[{"type":"NOT_FOUND","path":["organization"],"message":"Could not resolve to an Organization with the login of 'cli'."}]}`), + ) + + client, err := GQLClient(&api.ClientOptions{Transport: http}) + assert.NoError(t, err) + + vars := map[string]interface{}{} + res := struct{ Organization struct{ Name string } }{} + err = client.Do("QUERY", vars, &res) + assert.EqualError(t, err, "GQL error: Could not resolve to an Organization with the login of 'cli'.") +} + func TestHTTPClient(t *testing.T) { t.Cleanup(gock.Off) tempDir := t.TempDir() diff --git a/internal/api/gql_client.go b/internal/api/gql_client.go index d22d97b..b4d727b 100644 --- a/internal/api/gql_client.go +++ b/internal/api/gql_client.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/http" - "strings" "github.com/cli/go-gh/pkg/api" graphql "github.com/cli/shurcooL-graphql" @@ -75,7 +74,7 @@ func (c gqlClient) Do(query string, variables map[string]interface{}, response i } if len(gr.Errors) > 0 { - return &gqlErrorResponse{Errors: gr.Errors} + return &api.GQLError{Errors: gr.Errors} } return nil @@ -97,22 +96,5 @@ func (c gqlClient) Query(name string, q interface{}, variables map[string]interf type gqlResponse struct { Data interface{} - Errors []gqlError -} - -type gqlError struct { - Type string - Message string -} - -type gqlErrorResponse struct { - Errors []gqlError -} - -func (gr gqlErrorResponse) Error() string { - errorMessages := make([]string, 0, len(gr.Errors)) - for _, e := range gr.Errors { - errorMessages = append(errorMessages, e.Message) - } - return fmt.Sprintf("GQL error: %s", strings.Join(errorMessages, "\n")) + Errors []api.GQLErrorItem } diff --git a/pkg/api/client.go b/pkg/api/client.go index 842988f..5d98b53 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -2,8 +2,10 @@ package api import ( + "fmt" "io" "net/http" + "strings" "time" ) @@ -97,3 +99,23 @@ type GQLClient interface { // to the GitHub GraphQL schema. Query(name string, query interface{}, variables map[string]interface{}) error } + +// GQLError contains GQLErrors from a GraphQ request. +type GQLError struct { + Errors []GQLErrorItem +} + +// GQLErrorItem contains error information from a GraphQL request. +type GQLErrorItem struct { + Type string + Message string +} + +// Error formats all GQLError messages. +func (gr GQLError) Error() string { + errorMessages := make([]string, 0, len(gr.Errors)) + for _, e := range gr.Errors { + errorMessages = append(errorMessages, e.Message) + } + return fmt.Sprintf("GQL error: %s", strings.Join(errorMessages, "\n")) +} From 0ad179fb684cd3656b03453288e64c23074d4a26 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 9 May 2022 07:19:16 -0700 Subject: [PATCH 2/3] Resolve PR feedback --- internal/api/http.go | 4 ++-- pkg/api/client.go | 2 +- pkg/api/http.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/api/http.go b/internal/api/http.go index f7c4bcc..228c9e6 100644 --- a/internal/api/http.go +++ b/internal/api/http.go @@ -149,9 +149,9 @@ func handleHTTPError(resp *http.Response) error { var errString string _ = json.Unmarshal(raw, &errString) messages = append(messages, errString) - httpError.Errors = append(httpError.Errors, api.HttpErrorItem{Message: errString}) + httpError.Errors = append(httpError.Errors, api.HTTPErrorItem{Message: errString}) case '{': - var errInfo api.HttpErrorItem + var errInfo api.HTTPErrorItem _ = json.Unmarshal(raw, &errInfo) msg := errInfo.Message if errInfo.Code != "" && errInfo.Code != "custom" { diff --git a/pkg/api/client.go b/pkg/api/client.go index 5d98b53..ce3e2ad 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -100,7 +100,7 @@ type GQLClient interface { Query(name string, query interface{}, variables map[string]interface{}) error } -// GQLError contains GQLErrors from a GraphQ request. +// GQLError contains GQLErrors from a GraphQL request. type GQLError struct { Errors []GQLErrorItem } diff --git a/pkg/api/http.go b/pkg/api/http.go index 89546a9..e996f76 100644 --- a/pkg/api/http.go +++ b/pkg/api/http.go @@ -12,12 +12,12 @@ type HTTPError struct { RequestURL *url.URL Message string OAuthScopes string - Errors []HttpErrorItem + Errors []HTTPErrorItem } // HTTPErrorItem stores additional information about an error response // returned from the GitHub API. -type HttpErrorItem struct { +type HTTPErrorItem struct { Message string Resource string Field string From 6b5956f6453c609251d429dd536e9834106b5cce Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 9 May 2022 07:54:08 -0700 Subject: [PATCH 3/3] Use gock instead of httpmock --- gh_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/gh_test.go b/gh_test.go index a9e5b93..781c4c4 100644 --- a/gh_test.go +++ b/gh_test.go @@ -106,6 +106,7 @@ func TestGQLClient(t *testing.T) { } func TestGQLClientError(t *testing.T) { + t.Cleanup(gock.Off) tempDir := t.TempDir() orig_GH_CONFIG_DIR := os.Getenv("GH_CONFIG_DIR") orig_GH_TOKEN := os.Getenv("GH_TOKEN") @@ -116,19 +117,20 @@ func TestGQLClientError(t *testing.T) { os.Setenv("GH_CONFIG_DIR", tempDir) os.Setenv("GH_TOKEN", "GH_TOKEN") - http := httpmock.NewRegistry(t) - http.Register( - httpmock.GQL("QUERY"), - httpmock.StringResponse(`{"errors":[{"type":"NOT_FOUND","path":["organization"],"message":"Could not resolve to an Organization with the login of 'cli'."}]}`), - ) + gock.New("https://api.github.com"). + Post("/graphql"). + MatchHeader("Authorization", "token GH_TOKEN"). + BodyString(`{"query":"QUERY","variables":null}`). + Reply(200). + JSON(`{"errors":[{"type":"NOT_FOUND","path":["organization"],"message":"Could not resolve to an Organization with the login of 'cli'."}]}`) - client, err := GQLClient(&api.ClientOptions{Transport: http}) + client, err := GQLClient(nil) assert.NoError(t, err) - vars := map[string]interface{}{} res := struct{ Organization struct{ Name string } }{} - err = client.Do("QUERY", vars, &res) + err = client.Do("QUERY", nil, &res) assert.EqualError(t, err, "GQL error: Could not resolve to an Organization with the login of 'cli'.") + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } func TestHTTPClient(t *testing.T) {