From 08b3b61993d9374746721a2c037d51d6474f0945 Mon Sep 17 00:00:00 2001 From: Anton Tolchanov Date: Mon, 24 Sep 2018 10:33:06 +0100 Subject: [PATCH 1/2] Use json.Number in screen_widgets_test --- integration/screen_widgets_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integration/screen_widgets_test.go b/integration/screen_widgets_test.go index d68d371..d6f0273 100644 --- a/integration/screen_widgets_test.go +++ b/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{{ From 5282e9fe42d850176e8e2ac342cee5ef92a409e2 Mon Sep 17 00:00:00 2001 From: Anton Tolchanov Date: Wed, 19 Sep 2018 11:36:13 +0100 Subject: [PATCH 2/2] Propagate errors from JSON response This fixes #170. --- request.go | 35 ++++++++++++++++++++++++------- request_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/request.go b/request.go index f65ac5a..a89ed42 100644 --- a/request.go +++ b/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) } diff --git a/request_test.go b/request_test.go index 9b45a64..ee582bd 100644 --- a/request_test.go +++ b/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) + } + }) +}