Print HTTP errors on stderr in api command

mislav · mislav · commit d57b5171cfa0 · 2020-06-05T18:24:24.000+02:00
Most API errors are present in the response body itself, which will be
sent to stdout normally, but if stdout is redirected somewhere (as it's
common with scripts), failed HTTP requests will likely sabotage the rest
of the script, but no useful info will be shown on stderr.

This makes it so all REST and GraphQL errors are always shown on stderr.
Additionally, this makes sure that the command exits with a nonzero
status on any GraphQL errors.
diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go
@@ -1,6 +1,8 @@
 package api
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -109,24 +111,57 @@ func apiRun(opts *ApiOptions) error {
 	if resp.StatusCode == 204 {
 		return nil
 	}
+	var responseBody io.Reader = resp.Body
 	defer resp.Body.Close()
 
 	isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type"))
 
+	var serverError string
+	if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) {
+		bodyCopy := &bytes.Buffer{}
+		b, err := ioutil.ReadAll(io.TeeReader(responseBody, bodyCopy))
+		if err != nil {
+			return err
+		}
+		var respData struct {
+			Message string
+			Errors  []struct {
+				Message string
+			}
+		}
+		err = json.Unmarshal(b, &respData)
+		if err != nil {
+			return err
+		}
+		if respData.Message != "" {
+			serverError = fmt.Sprintf("%s (HTTP %d)", respData.Message, resp.StatusCode)
+		} else if len(respData.Errors) > 0 {
+			msgs := make([]string, len(respData.Errors))
+			for i, e := range respData.Errors {
+				msgs[i] = e.Message
+			}
+			serverError = strings.Join(msgs, "\n")
+		}
+		responseBody = bodyCopy
+	}
+
 	if isJSON && opts.IO.ColorEnabled() {
-		err = jsoncolor.Write(opts.IO.Out, resp.Body, "  ")
+		err = jsoncolor.Write(opts.IO.Out, responseBody, "  ")
 		if err != nil {
 			return err
 		}
 	} else {
-		_, err = io.Copy(opts.IO.Out, resp.Body)
+		_, err = io.Copy(opts.IO.Out, responseBody)
 		if err != nil {
 			return err
 		}
 	}
 
-	// TODO: detect GraphQL errors
-	if resp.StatusCode > 299 {
+	if serverError != "" {
+		fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError)
+		return cmdutil.SilentError
+	} else if resp.StatusCode > 299 {
+		fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode)
 		return cmdutil.SilentError
 	}
 
diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go
@@ -158,6 +158,31 @@ func Test_apiRun(t *testing.T) {
 			stdout: ``,
 			stderr: ``,
 		},
+		{
+			name: "REST error",
+			httpResponse: &http.Response{
+				StatusCode: 400,
+				Body:       ioutil.NopCloser(bytes.NewBufferString(`{"message": "THIS IS FINE"}`)),
+				Header:     http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
+			},
+			err:    cmdutil.SilentError,
+			stdout: `{"message": "THIS IS FINE"}`,
+			stderr: "gh: THIS IS FINE (HTTP 400)\n",
+		},
+		{
+			name: "GraphQL error",
+			options: ApiOptions{
+				RequestPath: "graphql",
+			},
+			httpResponse: &http.Response{
+				StatusCode: 200,
+				Body:       ioutil.NopCloser(bytes.NewBufferString(`{"errors": [{"message":"AGAIN"}, {"message":"FINE"}]}`)),
+				Header:     http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
+			},
+			err:    cmdutil.SilentError,
+			stdout: `{"errors": [{"message":"AGAIN"}, {"message":"FINE"}]}`,
+			stderr: "gh: AGAIN\nFINE\n",
+		},
 		{
 			name: "failure",
 			httpResponse: &http.Response{
@@ -166,7 +191,7 @@ func Test_apiRun(t *testing.T) {
 			},
 			err:    cmdutil.SilentError,
 			stdout: `gateway timeout`,
-			stderr: ``,
+			stderr: "gh: HTTP 502\n",
 		},
 	}
 
