api: return structured error for failed API calls

pborzenkov · pborzenkov · commit c66eebc6fb24 · 2020-06-27T18:47:34.000+03:00
`fmt.Errorf` hides information and makes it hard to test for specific
conditions in returned error. Return a structured error instead.

Signed-off-by: Pavel Borzenkov &lt;pavel.borzenkov@gmail.com&gt;
diff --git a/api/client.go b/api/client.go
@@ -157,6 +157,17 @@ func (gr GraphQLErrorResponse) Error() string {
 	return fmt.Sprintf("graphql error: '%s'", strings.Join(errorMessages, ", "))
 }
 
+// HTTPError is an error returned by a failed API call
+type HTTPError struct {
+	Code    int
+	URL     string
+	Message string
+}
+
+func (e HTTPError) Error() string {
+	return fmt.Sprintf("http error, '%s' failed (%d): '%s'", e.URL, e.Code, e.Message)
+}
+
 // Returns whether or not scopes are present, appID, and error
 func (c Client) HasScopes(wantedScopes ...string) (bool, string, error) {
 	url := "https://api.github.com/user"
@@ -298,7 +309,11 @@ func handleHTTPError(resp *http.Response) error {
 		message = parsedBody.Message
 	}
 
-	return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message)
+	return HTTPError{
+		Code:    resp.StatusCode,
+		URL:     resp.Request.URL.String(),
+		Message: message,
+	}
 }
 
 var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
diff --git a/api/client_test.go b/api/client_test.go
@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"errors"
 	"io/ioutil"
 	"reflect"
 	"testing"
@@ -66,3 +67,16 @@ func TestRESTGetDelete(t *testing.T) {
 	err := client.REST("DELETE", "applications/CLIENTID/grant", r, nil)
 	eq(t, err, nil)
 }
+
+func TestRESTError(t *testing.T) {
+	http := &httpmock.Registry{}
+	client := NewClient(ReplaceTripper(http))
+
+	http.StubResponse(422, bytes.NewBufferString(`{"message": "OH NO"}`))
+
+	var httpErr HTTPError
+	err := client.REST("DELETE", "/repos/branch", nil, nil)
+	if err == nil || !errors.As(err, &httpErr) || httpErr.Code != 422 {
+		t.Fatalf("got %q", err.Error())
+	}
+}
