From 46a2f52ca6d1746494e44e7e45bcbb660e3555a4 Mon Sep 17 00:00:00 2001 From: Owen Date: Wed, 24 Apr 2019 11:31:19 -0400 Subject: [PATCH 1/2] Return errors from JSON encode/decode I found two JSON errors being shadowed; the one in amazon/validator.go is being hidden by errors.New(responseError.Message) -- which should be an empty string if there's a JSON error. So, it wouldn't report success, but this gives the caller better information on what failed. The second is in appstore/validator.go, which was ignoring encode errors before POSTing a verify request. --- amazon/validator.go | 3 +++ appstore/validator.go | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/amazon/validator.go b/amazon/validator.go index 46a29c1..ea32c8f 100644 --- a/amazon/validator.go +++ b/amazon/validator.go @@ -102,6 +102,9 @@ func (c *Client) Verify(ctx context.Context, userID string, receiptID string) (I if resp.StatusCode < 200 || resp.StatusCode >= 300 { responseError := IAPResponseError{} err = json.NewDecoder(resp.Body).Decode(&responseError) + if err != nil { + return result, err + } return result, errors.New(responseError.Message) } diff --git a/appstore/validator.go b/appstore/validator.go index 1a0ff80..4786f01 100644 --- a/appstore/validator.go +++ b/appstore/validator.go @@ -98,7 +98,9 @@ func NewWithClient(client *http.Client) *Client { // Verify sends receipts and gets validation result func (c *Client) Verify(ctx context.Context, reqBody IAPRequest, result interface{}) error { b := new(bytes.Buffer) - json.NewEncoder(b).Encode(reqBody) + if err := json.NewEncoder(b).Encode(reqBody); err != nil { + return err + } req, err := http.NewRequest("POST", c.ProductionURL, b) if err != nil { From 92d56725f73ca25135dae914daaf17dabe074eeb Mon Sep 17 00:00:00 2001 From: Owen Date: Wed, 24 Apr 2019 11:36:47 -0400 Subject: [PATCH 2/2] Unwrap error in Amazon validator In amazon/validator.go, an error was being wrapped by fmt.Errorf("%v", err). That discards any type information about the returned error; returning err lets the callers inspect the exact error returned by the HTTP client. The test had to change to do similar inspection. --- amazon/validator.go | 2 +- amazon/validator_test.go | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/amazon/validator.go b/amazon/validator.go index ea32c8f..0a3eb68 100644 --- a/amazon/validator.go +++ b/amazon/validator.go @@ -95,7 +95,7 @@ func (c *Client) Verify(ctx context.Context, userID string, receiptID string) (I resp, err := c.httpCli.Do(req) if err != nil { - return result, fmt.Errorf("%v", err) + return result, err } defer resp.Body.Close() diff --git a/amazon/validator_test.go b/amazon/validator_test.go index 4bc9629..1cc80cb 100644 --- a/amazon/validator_test.go +++ b/amazon/validator_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "os" "reflect" "testing" @@ -143,11 +144,17 @@ func TestVerifyTimeout(t *testing.T) { server, client := testTools(100, "timeout response") defer server.Close() - expected := errors.New("") ctx := context.Background() _, actual := client.Verify(ctx, "timeout", "timeout") - if !reflect.DeepEqual(reflect.TypeOf(actual), reflect.TypeOf(expected)) { - t.Errorf("got %v\nwant %v", actual, expected) + + // Actual should be a "request canceled" *url.Error + urlErr, ok := actual.(*url.Error) + if !ok { + t.Errorf("Expected *url.Error, got %T", actual) + } + + if !urlErr.Timeout() { + t.Errorf("got %v\nwant timeout", actual) } }