From 559767c0a50daa75dcac0c0208b1cda44d4417a7 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 12 Apr 2019 16:00:10 -0700 Subject: [PATCH] Add more context when failing to unmarshal JSON As mentioned in #830, we occasionally have a situation where non-JSON data comes back to a client even though this is not supposed to happen. Unfortunately, when it does, the error message produced by Go isn't very good and doesn't help much with debugging. Here we add a new helper function for unmarshaling JSON in standard or error responses that behaves normally in the normal cases, and in case of a failure, adds more context to the standard library's error and logs. --- stripe.go | 31 +++++++++++++++++++++++++++++-- stripe_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/stripe.go b/stripe.go index bea55ac0e1..cab0dcdcec 100644 --- a/stripe.go +++ b/stripe.go @@ -441,7 +441,7 @@ func (s *BackendImplementation) Do(req *http.Request, body *bytes.Buffer, v inte s.LeveledLogger.Debugf("Response: %s\n", string(resBody)) if v != nil { - return json.Unmarshal(resBody, v) + return s.UnmarshalJSONVerbose(res.StatusCode, resBody, v) } return nil @@ -450,9 +450,10 @@ func (s *BackendImplementation) Do(req *http.Request, body *bytes.Buffer, v inte // ResponseToError converts a stripe response to an Error. func (s *BackendImplementation) ResponseToError(res *http.Response, resBody []byte) error { var raw rawError - if err := json.Unmarshal(resBody, &raw); err != nil { + if err := s.UnmarshalJSONVerbose(res.StatusCode, resBody, &raw); err != nil { return err } + // no error in resBody if raw.E == nil { err := errors.New(string(resBody)) @@ -505,6 +506,32 @@ func (s *BackendImplementation) SetNetworkRetriesSleep(sleep bool) { s.networkRetriesSleep = sleep } +// UnmarshalJSONVerbose unmarshals JSON, but in case of a failure logs and +// produces a more descriptive error. +func (s *BackendImplementation) UnmarshalJSONVerbose(statusCode int, body []byte, v interface{}) error { + err := json.Unmarshal(body, v) + if err != nil { + // If we got invalid JSON back then something totally unexpected is + // happening (caused by a bug on the server side). Put a sample of the + // response body into the error message so we can get a better feel for + // what the problem was. + bodySample := string(body) + if len(bodySample) > 50 { + bodySample = bodySample[0:50] + } + + // Make sure a multi-line response ends up all on one line + bodySample = strings.Replace(bodySample, "\n", "\\n", -1) + + newErr := fmt.Errorf("Couldn't deserialize JSON (response status: %v, body sample: '%s'): %v", + statusCode, bodySample, err) + s.LeveledLogger.Errorf("%s", newErr.Error()) + return newErr + } + + return nil +} + // Checks if an error is a problem that we should retry on. This includes both // socket errors that may represent an intermittent problem and some special // HTTP statuses. diff --git a/stripe_test.go b/stripe_test.go index 87a4cb9fcd..1d6760c76b 100644 --- a/stripe_test.go +++ b/stripe_test.go @@ -527,6 +527,48 @@ func TestStripeAccount(t *testing.T) { assert.Equal(t, TestMerchantID, req.Header.Get("Stripe-Account")) } +func TestUnmarshalJSONVerbose(t *testing.T) { + type testServerResponse struct { + Message string `json:"message"` + } + + backend := stripe.GetBackend(stripe.APIBackend).(*stripe.BackendImplementation) + + // Valid JSON + { + type testServerResponse struct { + Message string `json:"message"` + } + + var sample testServerResponse + err := backend.UnmarshalJSONVerbose(200, []byte(`{"message":"hello"}`), &sample) + assert.NoError(t, err) + assert.Equal(t, "hello", sample.Message) + } + + // Invalid JSON (short) + { + body := `server error` + + var sample testServerResponse + err := backend.UnmarshalJSONVerbose(200, []byte(body), &sample) + assert.Regexp(t, + fmt.Sprintf(`^Couldn't deserialize JSON \(response status: 200, body sample: '%s'\): invalid character`, body), + err) + } + + // Invalid JSON (long, and therefore truncated) + { + body := `this is a really long body that will be truncated when added to the error message` + + var sample testServerResponse + err := backend.UnmarshalJSONVerbose(200, []byte(body), &sample) + assert.Regexp(t, + fmt.Sprintf(`^Couldn't deserialize JSON \(response status: 200, body sample: '%s'\): invalid character`, body[0:50]), + err) + } +} + func TestUserAgent(t *testing.T) { c := stripe.GetBackend(stripe.APIBackend).(*stripe.BackendImplementation)