Skip to content

Commit

Permalink
Retry requests on a 429 that's a lock timeout (#841)
Browse files Browse the repository at this point in the history
Tweaks the retry logic so that 429s which have the special status code
lock_timeout are retried automatically.

Similar to what was recently implemented in Go: stripe/stripe-go#935
  • Loading branch information
brandur authored and brandur-stripe committed Aug 27, 2019
1 parent 82b5e51 commit 3e21fa9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
10 changes: 10 additions & 0 deletions lib/stripe/stripe_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ def self.should_retry?(error, method:, num_retries:)
# 409 Conflict
return true if error.http_status == 409

# 429 Too Many Requests
#
# There are a few different problems that can lead to a 429. The most
# common is rate limiting, on which we *don't* want to retry because
# that'd likely contribute to more contention problems. However, some
# 429s are lock timeouts, which is when a request conflicted with
# another request or an internal process on some particular object.
# These 429s are safe to retry.
return true if error.http_status == 429 && error.code == "lock_timeout"

# 500 Internal Server Error
#
# We only bother retrying these for non-POST requests. POSTs end up
Expand Down
12 changes: 12 additions & 0 deletions test/stripe/stripe_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ class StripeClientTest < Test::Unit::TestCase
method: :post, num_retries: 0)
end

should "retry on a 429 Too Many Requests when lock timeout" do
assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 429,
code: "lock_timeout"),
method: :post, num_retries: 0)
end

should "retry on a 500 Internal Server Error when non-POST" do
assert StripeClient.should_retry?(Stripe::StripeError.new(http_status: 500),
method: :get, num_retries: 0)
Expand All @@ -142,6 +148,12 @@ class StripeClientTest < Test::Unit::TestCase
method: :post, num_retries: 0)
end

should "not retry on a 429 Too Many Requests when not lock timeout" do
refute StripeClient.should_retry?(Stripe::StripeError.new(http_status: 429,
code: "rate_limited"),
method: :post, num_retries: 0)
end

should "not retry on a 500 Internal Server Error when POST" do
refute StripeClient.should_retry?(Stripe::StripeError.new(http_status: 500),
method: :post, num_retries: 0)
Expand Down

0 comments on commit 3e21fa9

Please sign in to comment.