From fb179a86f67ac2c574bfa656226490b9c718a94d Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Thu, 10 Aug 2023 20:16:55 -0400 Subject: [PATCH 1/5] retry requests when rate limit reached Adds support for retrying requests that fail with 429 using the `Retry-After` header, similar to what we do in godo. This is still WIP, since the README hasn't yet been update to describe the change in behaviour. I also still need to run some manual tests. --- droplet_kit.gemspec | 1 + lib/droplet_kit/client.rb | 29 ++++++++++++---- lib/droplet_kit/error_handling_resourcable.rb | 6 ---- spec/lib/droplet_kit/client_spec.rb | 20 +++++++++++ .../resources/account_resource_spec.rb | 6 ++++ .../resources/droplet_resource_spec.rb | 7 ++++ spec/spec_helper.rb | 1 + spec/support/shared_examples/common_errors.rb | 16 --------- .../shared_examples/rate_limit_retry.rb | 34 +++++++++++++++++++ 9 files changed, 92 insertions(+), 28 deletions(-) create mode 100644 spec/support/shared_examples/rate_limit_retry.rb diff --git a/droplet_kit.gemspec b/droplet_kit.gemspec index 54c25188..26ba9c36 100644 --- a/droplet_kit.gemspec +++ b/droplet_kit.gemspec @@ -20,6 +20,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.5.0' spec.add_dependency 'faraday', '>= 0.15' + spec.add_dependency 'faraday-retry', '~> 2.2.0' spec.add_dependency 'kartograph', '~> 0.2.8' spec.add_dependency 'resource_kit', '~> 0.1.5' spec.add_dependency 'virtus', '>= 1.0.3', '<= 3' diff --git a/lib/droplet_kit/client.rb b/lib/droplet_kit/client.rb index 882aa981..69ae97a4 100644 --- a/lib/droplet_kit/client.rb +++ b/lib/droplet_kit/client.rb @@ -1,23 +1,29 @@ # frozen_string_literal: true require 'faraday' +require 'faraday/retry' require 'droplet_kit/utils' module DropletKit class Client DEFAULT_OPEN_TIMEOUT = 60 DEFAULT_TIMEOUT = 120 + DEFAULT_RETRY_MAX = 3 + DEFAULT_RETRY_WAIT_MIN = 0 + DIGITALOCEAN_API = 'https://api.digitalocean.com' - attr_reader :access_token, :api_url, :open_timeout, :timeout, :user_agent + attr_reader :access_token, :api_url, :open_timeout, :timeout, :user_agent, :retry_max, :retry_wait_min def initialize(options = {}) options = DropletKit::Utils.transform_keys(options, &:to_sym) - @access_token = options[:access_token] - @api_url = options[:api_url] || DIGITALOCEAN_API - @open_timeout = options[:open_timeout] || DEFAULT_OPEN_TIMEOUT - @timeout = options[:timeout] || DEFAULT_TIMEOUT - @user_agent = options[:user_agent] + @access_token = options[:access_token] + @api_url = options[:api_url] || DIGITALOCEAN_API + @open_timeout = options[:open_timeout] || DEFAULT_OPEN_TIMEOUT + @timeout = options[:timeout] || DEFAULT_TIMEOUT + @user_agent = options[:user_agent] + @retry_max = options[:retry_max] || DEFAULT_RETRY_MAX + @retry_wait_min = options[:retry_wait_min] || DEFAULT_RETRY_WAIT_MIN end def connection @@ -25,6 +31,17 @@ def connection req.adapter :net_http req.options.open_timeout = open_timeout req.options.timeout = timeout + req.request :retry, { + max: @retry_max, + interval: @retry_wait_min, + retry_statuses: [429], + # faraday-retry supports both the Retry-After and RateLimit-Reset + # headers, however, it favours the RateLimit-Reset one. To force it + # to use the Retry-After header, we override the header that it + # expects for the RateLimit-Reset header to something that we know + # we don't set. + rate_limit_reset_header: 'undefined' + } end end diff --git a/lib/droplet_kit/error_handling_resourcable.rb b/lib/droplet_kit/error_handling_resourcable.rb index 598d08fc..f8ee1069 100644 --- a/lib/droplet_kit/error_handling_resourcable.rb +++ b/lib/droplet_kit/error_handling_resourcable.rb @@ -7,12 +7,6 @@ def self.included(base) case response.status when 200...299 next - when 429 - error = DropletKit::RateLimitReached.new("#{response.status}: #{response.body}") - error.limit = response.headers['RateLimit-Limit'] - error.remaining = response.headers['RateLimit-Remaining'] - error.reset_at = response.headers['RateLimit-Reset'] - raise error else raise DropletKit::Error, "#{response.status}: #{response.body}" end diff --git a/spec/lib/droplet_kit/client_spec.rb b/spec/lib/droplet_kit/client_spec.rb index 17f11170..5feed054 100644 --- a/spec/lib/droplet_kit/client_spec.rb +++ b/spec/lib/droplet_kit/client_spec.rb @@ -51,6 +51,26 @@ expect(client.timeout).to eq(timeout) end + + it 'allows retry max to be set' do + retry_max = 10 + client = described_class.new( + 'access_token' => 'my-token', + 'retry_max' => retry_max + ) + + expect(client.retry_max).to eq(retry_max) + end + + it 'allows retry wait min to be set' do + retry_wait_min = 3 + client = described_class.new( + 'access_token' => 'my-token', + 'retry_wait_min' => retry_wait_min + ) + + expect(client.retry_wait_min).to eq(retry_wait_min) + end end describe '#method_missing' do diff --git a/spec/lib/droplet_kit/resources/account_resource_spec.rb b/spec/lib/droplet_kit/resources/account_resource_spec.rb index 8bb4e258..993be108 100644 --- a/spec/lib/droplet_kit/resources/account_resource_spec.rb +++ b/spec/lib/droplet_kit/resources/account_resource_spec.rb @@ -31,5 +31,11 @@ let(:method) { :get } let(:action) { :info } end + + it_behaves_like 'resource that handles rate limit retries' do + let(:path) { '/v2/account' } + let(:method) { :get } + let(:action) { :info } + end end end diff --git a/spec/lib/droplet_kit/resources/droplet_resource_spec.rb b/spec/lib/droplet_kit/resources/droplet_resource_spec.rb index 88d4a2c9..3c6a756a 100644 --- a/spec/lib/droplet_kit/resources/droplet_resource_spec.rb +++ b/spec/lib/droplet_kit/resources/droplet_resource_spec.rb @@ -133,6 +133,13 @@ def check_droplet(droplet, tags = [], overrides = {}) let(:action) { :find } let(:arguments) { { id: 123 } } end + + it_behaves_like 'resource that handles rate limit retries' do + let(:path) { '/v2/droplets/123' } + let(:method) { :get } + let(:action) { :find } + let(:arguments) { { id: 123 } } + end end describe '#create' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b92ae4f4..0a2495c0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,7 @@ SimpleCov.start require 'faraday' +require 'faraday/retry' require 'addressable/uri' require 'droplet_kit' require 'webmock/rspec' diff --git a/spec/support/shared_examples/common_errors.rb b/spec/support/shared_examples/common_errors.rb index c36b6e1d..c9039324 100644 --- a/spec/support/shared_examples/common_errors.rb +++ b/spec/support/shared_examples/common_errors.rb @@ -3,22 +3,6 @@ shared_examples_for 'resource that handles common errors' do let(:arguments) { {} } - it 'handles rate limit' do - response_body = { id: :rate_limit, message: 'Too much!!!' } - stub_do_api(path, method).to_return(body: response_body.to_json, status: 429, headers: { - 'RateLimit-Limit' => 1200, - 'RateLimit-Remaining' => 1193, - 'RateLimit-Reset' => 1_402_425_459 - }) - - expect { resource.send(action, arguments).to_a }.to raise_exception(DropletKit::Error) do |exception| - expect(exception.message).to match(/#{response_body[:message]}/) - expect(exception.limit).to eq 1200 - expect(exception.remaining).to eq 1193 - expect(exception.reset_at).to eq '1402425459' - end - end - it 'handles unauthorized' do response_body = { id: :unauthorized, message: 'Nuh uh.' } diff --git a/spec/support/shared_examples/rate_limit_retry.rb b/spec/support/shared_examples/rate_limit_retry.rb new file mode 100644 index 00000000..8f93fe10 --- /dev/null +++ b/spec/support/shared_examples/rate_limit_retry.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +shared_examples_for 'resource that handles rate limit retries' do + let(:arguments) { {} } + + it 'handles rate limit' do + response_body = { id: :rate_limit, message: 'example' } + stub_do_api(path, method).to_return( + [ + { + body: nil, + status: 429, + headers: { + 'RateLimit-Limit' => 1200, + 'RateLimit-Remaining' => 1193, + 'RateLimit-Reset' => 1_402_425_459, + 'Retry-After' => 0 # Retry immediately in tests. + } + }, + { + body: response_body.to_json, + status: 200, + headers: { + 'RateLimit-Limit' => 1200, + 'RateLimit-Remaining' => 1192, + 'RateLimit-Reset' => 1_402_425_459 + } + } + ] + ) + + expect { resource.send(action, arguments) }.not_to raise_error + end +end From 01032e23f942548206f75e53d5a530634fd585f5 Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Wed, 16 Aug 2023 13:01:15 -0400 Subject: [PATCH 2/5] restore rate limit err when retry-after missing --- lib/droplet_kit/error_handling_resourcable.rb | 8 ++++++++ spec/support/shared_examples/common_errors.rb | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/droplet_kit/error_handling_resourcable.rb b/lib/droplet_kit/error_handling_resourcable.rb index f8ee1069..38d8e6e5 100644 --- a/lib/droplet_kit/error_handling_resourcable.rb +++ b/lib/droplet_kit/error_handling_resourcable.rb @@ -7,6 +7,14 @@ def self.included(base) case response.status when 200...299 next + when 429 + unless response.headers.key?('Retry-After') + error = DropletKit::RateLimitReached.new("#{response.status}: #{response.body}") + error.limit = response.headers['RateLimit-Limit'] + error.remaining = response.headers['RateLimit-Remaining'] + error.reset_at = response.headers['RateLimit-Reset'] + raise error + end else raise DropletKit::Error, "#{response.status}: #{response.body}" end diff --git a/spec/support/shared_examples/common_errors.rb b/spec/support/shared_examples/common_errors.rb index c9039324..e5c03c12 100644 --- a/spec/support/shared_examples/common_errors.rb +++ b/spec/support/shared_examples/common_errors.rb @@ -3,6 +3,22 @@ shared_examples_for 'resource that handles common errors' do let(:arguments) { {} } + it 'handles rate limit when retry-after is not present' do + response_body = { id: :rate_limit, message: 'Too much!!!' } + stub_do_api(path, method).to_return(body: response_body.to_json, status: 429, headers: { + 'RateLimit-Limit' => 1200, + 'RateLimit-Remaining' => 1193, + 'RateLimit-Reset' => 1_402_425_459 + }) + + expect { resource.send(action, arguments).to_a }.to raise_exception(DropletKit::Error) do |exception| + expect(exception.message).to match(/#{response_body[:message]}/) + expect(exception.limit).to eq 1200 + expect(exception.remaining).to eq 1193 + expect(exception.reset_at).to eq '1402425459' + end + end + it 'handles unauthorized' do response_body = { id: :unauthorized, message: 'Nuh uh.' } From 7c855849c498782479db40863b12d52222633ad8 Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Wed, 16 Aug 2023 13:06:16 -0400 Subject: [PATCH 3/5] remove support for ruby 2.5 (eol 31/03/2021) --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8bd0e46d..e4e09e81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: fail-fast: true matrix: os: [ ubuntu-latest, macos-latest ] - ruby: [ 2.5, 2.6, 2.7, "3.0", 3.1, 3.2 ] # "x.0" to workaround: https://github.com/ruby/setup-ruby/issues/252 + ruby: [ 2.6, 2.7, "3.0", 3.1, 3.2 ] # "x.0" to workaround: https://github.com/ruby/setup-ruby/issues/252 runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 From 5aff9fa8ff526d18d95bbc680e411a859c18546d Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Fri, 18 Aug 2023 16:56:56 -0400 Subject: [PATCH 4/5] only retry rate limits when enabled --- lib/droplet_kit/client.rb | 29 ++++++++++++------- lib/droplet_kit/error_handling_resourcable.rb | 2 +- spec/lib/droplet_kit/client_spec.rb | 13 +++++++++ 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/droplet_kit/client.rb b/lib/droplet_kit/client.rb index 69ae97a4..9756218f 100644 --- a/lib/droplet_kit/client.rb +++ b/lib/droplet_kit/client.rb @@ -31,17 +31,19 @@ def connection req.adapter :net_http req.options.open_timeout = open_timeout req.options.timeout = timeout - req.request :retry, { - max: @retry_max, - interval: @retry_wait_min, - retry_statuses: [429], - # faraday-retry supports both the Retry-After and RateLimit-Reset - # headers, however, it favours the RateLimit-Reset one. To force it - # to use the Retry-After header, we override the header that it - # expects for the RateLimit-Reset header to something that we know - # we don't set. - rate_limit_reset_header: 'undefined' - } + unless retry_max.zero? + req.request :retry, { + max: @retry_max, + interval: @retry_wait_min, + retry_statuses: [429], + # faraday-retry supports both the Retry-After and RateLimit-Reset + # headers, however, it favours the RateLimit-Reset one. To force it + # to use the Retry-After header, we override the header that it + # expects for the RateLimit-Reset header to something that we know + # we don't set. + rate_limit_reset_header: 'undefined' + } + end end end @@ -108,6 +110,11 @@ def connection_options content_type: 'application/json', authorization: "Bearer #{access_token}", user_agent: "#{user_agent} #{default_user_agent}".strip + }, + request: { + context: { + retry_max: @retry_max + } } } end diff --git a/lib/droplet_kit/error_handling_resourcable.rb b/lib/droplet_kit/error_handling_resourcable.rb index 38d8e6e5..5d84d96d 100644 --- a/lib/droplet_kit/error_handling_resourcable.rb +++ b/lib/droplet_kit/error_handling_resourcable.rb @@ -8,7 +8,7 @@ def self.included(base) when 200...299 next when 429 - unless response.headers.key?('Retry-After') + unless response.headers.key?('Retry-After') && !connection.options.context.key?(:retry_max) error = DropletKit::RateLimitReached.new("#{response.status}: #{response.body}") error.limit = response.headers['RateLimit-Limit'] error.remaining = response.headers['RateLimit-Remaining'] diff --git a/spec/lib/droplet_kit/client_spec.rb b/spec/lib/droplet_kit/client_spec.rb index 5feed054..2ce673d7 100644 --- a/spec/lib/droplet_kit/client_spec.rb +++ b/spec/lib/droplet_kit/client_spec.rb @@ -71,6 +71,19 @@ expect(client.retry_wait_min).to eq(retry_wait_min) end + + it 'does not handle rate limited requests when retry max is zero' do + client = described_class.new(retry_max: 0) + + stub_do_api('/v2/account', :get).to_return(body: { id: :rate_limit, message: '429' }.to_json, status: 429, headers: { + 'RateLimit-Limit' => 1200, + 'RateLimit-Remaining' => 1193, + 'RateLimit-Reset' => 1_402_425_459, + 'Retry-After' => 0 + }) + + expect { client.account.send(:info).to_a }.to raise_exception(DropletKit::RateLimitReached) + end end describe '#method_missing' do From 426432796a69f6846b33eb377910524110327d61 Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Fri, 18 Aug 2023 17:11:54 -0400 Subject: [PATCH 5/5] document rate limit handling and config --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 73879d36..689c0167 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,19 @@ require 'droplet_kit' client = DropletKit::Client.new(access_token: 'YOUR_TOKEN', user_agent: 'custom') ``` +### Automatically Retry Rate Limited Requests + +By default, DropletKit will handle requests that are rate limited by the DigitalOcean API's [burst limit](https://docs.digitalocean.com/reference/api/api-reference/#section/Introduction/Rate-Limit). When the burst rate limit is reached, DropletKit will wait according to the value of the API response's `Retry-After` header. Typically the wait time is less than one minute. When the hourly rate limit is hit, an error is raised. + +By default, DropletKit will retry a rate limited request three times before returning an error. If you would like to disable the retry behavior altogether, and instead raise an error when any rate limit is reached, you can set the `retry_max` config value to zero. + +DropletKit will also wait zero seconds until retrying a request after the `Retry-After` time has elapsed by default. To change this, set the `retry_wait_min` to a different value. + +```ruby +require 'droplet_kit' +client = DropletKit::Client.new(access_token: 'YOUR_TOKEN', retry_max: 3, retry_wait_min: 1) +``` + ## Design DropletKit follows a strict design of resources as methods on your client. For examples, for droplets, you will call your client like this: