Skip to content

Commit

Permalink
Propagate errors from JSON response
Browse files Browse the repository at this point in the history
This fixes zorkian#170.
  • Loading branch information
knyar committed Sep 25, 2018
1 parent 08b3b61 commit 5282e9f
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 8 deletions.
35 changes: 27 additions & 8 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
55 changes: 55 additions & 0 deletions request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package datadog
import (
"fmt"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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)
}
})
}

0 comments on commit 5282e9f

Please sign in to comment.