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

Bring retry code in-line with current best practices #928

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Aug 22, 2019

Brings stripe-go's retry code in-line with what we currently understand
to be the best practices as already implemented in stripe-node and
stripe-ruby. Basically has the effect of adding some 500s and 503s to
results that will be retried.

r? @ob-stripe
cc @stripe/api-libraries

Brings stripe-go's retry code in-line with what we currently understand
to be the best practices as already implemented in stripe-node and
stripe-ruby. Basically has the effect of adding some 500s and 503s to
results that will be retried.
if numRetries >= s.MaxNetworkRetries {
return false
}

// TODO: There are many errors that should not be retried. Try to make this
// more granular by including only connection errors, timeout errors, etc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this TODO here because honestly what we're doing right now isn't great.

Unfortunately, routing around Go source looking for errors that we can compare against is slightly non-trivial, so deferring this to a separate improvement project.

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brandur-stripe
Copy link
Contributor

Thanks!

@brandur-stripe brandur-stripe merged commit 76454ec into master Aug 22, 2019
@brandur-stripe brandur-stripe deleted the brandur-should-retry branch August 22, 2019 23:17
@brandur-stripe
Copy link
Contributor

Released as 62.3.0.

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 this pull request may close these issues.

None yet

4 participants