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

Don't retry on client errors. #7

Merged
merged 1 commit into from
Jul 26, 2017

Conversation

dreid
Copy link
Contributor

@dreid dreid commented Jul 19, 2017

Many error classes aren't likely to succeed when you try again and so they probably shouldn't be retried by default.

Errors I don't think will typically work if retried:

  • 401 - BadCredentialsError
  • 402 - PaymentRequiredError
  • 413 - RequestEntityTooLargeError
  • 400 - BadRequestError
  • 422 - UnprocessableEntityError
  • 429 - TooManyRequestsError

Errors which may work if retried:

  • 500 - InternalServerError
  • 503 - ServiceUnavailableError

This is based on the errors handled by StatusCodeSender, as you can see it groups neatly into 400 errors (client errors) and 500 errors (server errors).

This PR only retries anything less than 500.

Depending on your API compatibility stance this could be a big backwards compatibility change but it's in line with typical usage of HTTP.

@MouaYing MouaYing merged commit 5590626 into smartystreets:master Jul 26, 2017
@MouaYing
Copy link
Contributor

Thanks for helping us make this better, @dreid !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants