Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Occasional invalid JSON returned by Stripe #830

Closed
craigandrews opened this issue Apr 11, 2019 · 6 comments · Fixed by #833
Closed

Occasional invalid JSON returned by Stripe #830

craigandrews opened this issue Apr 11, 2019 · 6 comments · Fixed by #833
Labels

Comments

@craigandrews
Copy link

Very occasionally (once every few hundred thousands requests) I will receive an apparently nonsensical error from charge.New(). Errors such as:

invalid character 'o' in literal null (expecting 'u')

and

invalid character 'u' looking for beginning of value

After a bit of head-scratching I realised that these are actually JSON unmarshaling errors so I had a look in stripe.go and found this at line 437:

	if res.StatusCode >= 400 {
		return s.ResponseToError(res, resBody)
	}

	s.LeveledLogger.Debugf("Response: %s\n", string(resBody))

	if v != nil {
		return json.Unmarshal(resBody, v)
	}

	return nil

and in ResponseToError at line 451:

func (s *BackendImplementation) ResponseToError(res *http.Response, resBody []byte) error {
	var raw rawError
	if err := json.Unmarshal(resBody, &raw); err != nil {
		return err
	}

The problem here is that invalid JSON is being returned from the remote API but the client code is making no effort at exposing it. And if the response is a 400 or worse, it doesn't even log the actual body to the debug log.

It would be useful to be able to get the actual raw response from the request so it can be logged and we can understand why we're seeing failures, rather than just cryptic notes about unexpected vowels.

@remi-stripe
Copy link
Contributor

@candrewsmtn Thank you for the report! In theory, our API should always return a JSON string for the error which is why the library expects this explicitly. In some really rare cases, it's possible for the request to fail in a way that a non-json error is returned. That error is unlikely to be helpful since it would certainly be some default HTML page error or similar or a truncated response but I do agree that logging it could be useful.

Tagging as future for @brandur-stripe to maybe investigate.

@brandur-stripe
Copy link
Contributor

It would be useful to be able to get the actual raw response from the request so it can be logged and we can understand why we're seeing failures, rather than just cryptic notes about unexpected vowels.

That error message originates directly from Go core, but yeah, we should do a better job of at least logging more detail about the problem. I'll write a patch for that.

@craigandrews
Copy link
Author

It would be good if the error message was replaced with something more useful. Just an indication of the HTTP status code and the fact it could not be parsed would help enormously to work out if the issue is with us, stripe, or the network in between.

brandur-stripe pushed a commit that referenced this issue Apr 12, 2019
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.
brandur-stripe pushed a commit that referenced this issue Apr 15, 2019
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.
@brandur-stripe
Copy link
Contributor

We just released 60.4.0, which logs some more flavor in case of a failure like this.

@candrewsmtn Would you mind trying that and checking back in if you discover something concerning is revealed?

@craigandrews
Copy link
Author

craigandrews commented Apr 15, 2019 via email

@layby42
Copy link
Contributor

layby42 commented May 20, 2019

Hi Everybody

attempt to get customer resulted in Stripe error:

Couldn't deserialize JSON (response status: 503, body sample: 'upstream connect error or disconnect/reset before headers'): invalid character 'u' looking for beginning of value

happened only once so far, cannot reproduce.

stripe-go: 60.13.1

nadaismail-stripe pushed a commit that referenced this issue Oct 18, 2024
Bumps [sorbet](https://github.com/sorbet/sorbet) from 0.5.10455 to 0.5.10477.
- [Release notes](https://github.com/sorbet/sorbet/releases)
- [Commits](https://github.com/sorbet/sorbet/commits)

---
updated-dependencies:
- dependency-name: sorbet
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants