Merge pull request zorkian#171 from knyar/errors

Propagate errors from JSON response

integration/screen_widgets_test.go

@@ -364,12 +364,12 @@ func TestWidgets(t *testing.T) {
 			},
 			Rules: map[string]*datadog.Rule{
 				"0": {
-					Threshold: datadog.Int(95),
+					Threshold: datadog.JsonNumber("95"),
 					Timeframe: datadog.String("Month-to-date"),
 					Color:     datadog.String("green"),
 				},
 				"1": {
-					Threshold: datadog.Int(98),
+					Threshold: datadog.JsonNumber("98"),
 					Timeframe: datadog.String("7 days"),
 					Color:     datadog.String("red"),
 				},
@@ -384,6 +384,7 @@ func TestWidgets(t *testing.T) {
 			Y:      datadog.Int(1),
 			Width:  datadog.Int(5),
 			Height: datadog.Int(5),
+			Time:   &datadog.Time{},
 			TileDef: &datadog.TileDef{
 				Viz: datadog.String("process"),
 				Requests: []datadog.TileDefRequest{{

request.go

@@ -22,6 +22,12 @@ import (
 	"github.com/cenkalti/backoff"
 )
 
+// Response contains common fields that might be present in any API response.
+type Response struct {
+	Status string `json:"status"`
+	Error  string `json:"error"`
+}
+
 // uriForAPI is to be called with something like "/v1/events" and it will give
 // the proper request URI to be posted to.
 func (client *Client) uriForAPI(api string) (string, error) {
@@ -99,14 +105,6 @@ func (client *Client) doJsonRequestUnredacted(method, api string,
 		return fmt.Errorf("API error %s: %s", resp.Status, body)
 	}
 
-	// If they don't care about the body, then we don't care to give them one,
-	// so bail out because we're done.
-	if out == nil {
-		// read the response body so http conn can be reused immediately
-		io.Copy(ioutil.Discard, resp.Body)
-		return nil
-	}
-
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return err
@@ -118,6 +116,27 @@ func (client *Client) doJsonRequestUnredacted(method, api string,
 		body = []byte{'{', '}'}
 	}
 
+	// Try to parse common response fields to check whether there's an error reported in a response.
+	var common *Response
+	err = json.Unmarshal(body, &common)
+	if err != nil {
+		// UnmarshalTypeError errors are ignored, because in some cases API response is an array that cannot be
+		// unmarshalled into a struct.
+		_, ok := err.(*json.UnmarshalTypeError)
+		if !ok {
+			return err
+		}
+	}
+	if common != nil && common.Status == "error" {
+		return fmt.Errorf("API returned error: %s", common.Error)
+	}
+
+	// If they don't care about the body, then we don't care to give them one,
+	// so bail out because we're done.
+	if out == nil {
+		return nil
+	}
+
 	return json.Unmarshal(body, &out)
 }
 

request_test.go

@@ -3,6 +3,7 @@ package datadog
 import (
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -60,3 +61,57 @@ func TestRedactError(t *testing.T) {
 		assert.Nil(t, redactedErr)
 	})
 }
+
+func makeTestServer(code int, response string) *httptest.Server {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/api/v1/something", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(code)
+		w.Write([]byte(response))
+	})
+	server := httptest.NewServer(mux)
+	return server
+}
+
+func TestErrorHandling(t *testing.T) {
+	c := Client{
+		apiKey:       "sample_api_key",
+		appKey:       "sample_app_key",
+		HttpClient:   &http.Client{},
+		RetryTimeout: 1000,
+	}
+	for _, code := range []int{401, 403, 500, 502} {
+		t.Run(fmt.Sprintf("Returns error on http code %d", code), func(t *testing.T) {
+			s := makeTestServer(code, "")
+			defer s.Close()
+			c.SetBaseUrl(s.URL)
+
+			for _, method := range []string{"GET", "POST", "PUT"} {
+				err := c.doJsonRequest(method, "/v1/something", nil, nil)
+				assert.NotNil(t, err)
+			}
+		})
+	}
+	t.Run("Returns error if status is error", func(t *testing.T) {
+		s := makeTestServer(200, `{"status": "error", "error": "something wrong"}`)
+		defer s.Close()
+		c.SetBaseUrl(s.URL)
+
+		for _, method := range []string{"GET", "POST", "PUT"} {
+			err := c.doJsonRequest(method, "/v1/something", nil, nil)
+			if assert.NotNil(t, err) {
+				assert.Contains(t, err.Error(), "something wrong")
+			}
+		}
+	})
+	t.Run("Does not return error if status is ok", func(t *testing.T) {
+		s := makeTestServer(200, `{"status": "ok"}`)
+		defer s.Close()
+		c.SetBaseUrl(s.URL)
+
+		for _, method := range []string{"GET", "POST", "PUT"} {
+			err := c.doJsonRequest(method, "/v1/something", nil, nil)
+			assert.Nil(t, err)
+		}
+	})
+}