From b75bfcb7761ef5443e60f56536eb2f9ca739ddaf Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 12:57:42 -0400 Subject: [PATCH 1/9] respect rate limits during pagination by sleeping --- lib/slack/web/pagination/cursor.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index 5f4fedaf..ee4e0484 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -19,11 +19,15 @@ def each next_cursor = nil loop do query = { limit: client.default_page_size }.merge(params).merge(cursor: next_cursor) - response = client.send(verb, query) - yield response - break unless response.response_metadata - next_cursor = response.response_metadata.next_cursor - break if next_cursor.blank? + begin + response = client.send(verb, query) + yield response + break unless response.response_metadata + next_cursor = response.response_metadata.next_cursor + break if next_cursor.blank? + rescue Slack::Web::Api::Errors::TooManyRequestsError => e + sleep(e.retry_after.seconds) + end end end end From 8542d00df4b69f83c4641664f858d8f807e61e45 Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 13:26:47 -0400 Subject: [PATCH 2/9] only rescue from the call --- lib/slack/web/pagination/cursor.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index ee4e0484..234d3c6a 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -21,13 +21,14 @@ def each query = { limit: client.default_page_size }.merge(params).merge(cursor: next_cursor) begin response = client.send(verb, query) - yield response - break unless response.response_metadata - next_cursor = response.response_metadata.next_cursor - break if next_cursor.blank? rescue Slack::Web::Api::Errors::TooManyRequestsError => e sleep(e.retry_after.seconds) + next end + yield response + break unless response.response_metadata + next_cursor = response.response_metadata.next_cursor + break if next_cursor.blank? end end end From 791e00f1dc9b6218384ddd8ad87d137331629766 Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 15:09:29 -0400 Subject: [PATCH 3/9] add pause parameter for pagination --- lib/slack/web/pagination/cursor.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index 234d3c6a..b87c12dc 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -7,11 +7,13 @@ class Cursor attr_reader :client attr_reader :verb + attr_reader :pause attr_reader :params def initialize(client, verb, params = {}) @client = client @verb = verb + @pause = params.delete(:pause) @params = params end @@ -29,6 +31,7 @@ def each break unless response.response_metadata next_cursor = response.response_metadata.next_cursor break if next_cursor.blank? + sleep(pause) if pause end end end From e041d163b6b1d6e9ae5f26fc0e8e9b4d8d771fb9 Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 15:09:37 -0400 Subject: [PATCH 4/9] add tests --- spec/slack/web/api/pagination/cursor_spec.rb | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/slack/web/api/pagination/cursor_spec.rb b/spec/slack/web/api/pagination/cursor_spec.rb index 2d24f3c7..6c655d72 100644 --- a/spec/slack/web/api/pagination/cursor_spec.rb +++ b/spec/slack/web/api/pagination/cursor_spec.rb @@ -21,8 +21,20 @@ Slack::Messages::Message.new(response_metadata: { next_cursor: 'next' }), Slack::Messages::Message.new ) + expect(cursor).not_to receive(:sleep) cursor.to_a end + context 'with rate limiting' do + let(:error) { Slack::Web::Api::Errors::TooManyRequestsError.new(nil) } + it 'sleeps after a TooManyRequestsError' do + expect(client).to receive(:users_list).with(limit: 100, cursor: nil).ordered.and_return(Slack::Messages::Message.new(response_metadata: { next_cursor: 'next' })) + expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_raise(error) + expect(error).to receive(:retry_after).once.ordered.and_return(9) + expect(cursor).to receive(:sleep).once.ordered.with(9) + expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_return(Slack::Messages::Message.new) + cursor.to_a + end + end end context 'with a custom limit' do let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', limit: 42) } @@ -31,4 +43,16 @@ cursor.first end end + context 'with a custom pause' do + let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', pause: 3) } + it 'sleeps between requests' do + expect(client).to receive(:users_list).exactly(3).times.and_return( + Slack::Messages::Message.new(response_metadata: { next_cursor: 'next_a' }), + Slack::Messages::Message.new(response_metadata: { next_cursor: 'next_b' }), + Slack::Messages::Message.new + ) + expect(cursor).to receive(:sleep).with(3).twice + cursor.to_a + end + end end From 812e78f04780c9c5bd13a0471a4f8ebb92ef1b8e Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 15:11:24 -0400 Subject: [PATCH 5/9] add to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aec20f71..c92ada62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 0.9.2 (Next) +* [#167](https://github.com/slack-ruby/slack-ruby-client/pull/167): Respect rate limits during pagination by sleeping. Also add optional `pause` parameter in order to proactively sleep between each paginated request - [@jmanian](https://github.com/jmanian). * [#163](https://github.com/slack-ruby/slack-ruby-client/pull/164): Use `OpenSSL::X509::DEFAULT_CERT_DIR` and `OpenSSL::X509::DEFAULT_CERT_FILE` for default ca_cert and ca_file - [@leifcr](https://github.com/leifcr). * [#161](https://github.com/slack-ruby/slack-ruby-client/pull/161): Added support for cursor pagination - [@dblock](https://github.com/dblock). * Your contribution here. From 1e44ccc3c3018dd25bb6d5dc6785f1e01ae4416a Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 17:25:28 -0400 Subject: [PATCH 6/9] rename parameter to sleep_interval --- lib/slack/web/pagination/cursor.rb | 6 +++--- spec/slack/web/api/pagination/cursor_spec.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index b87c12dc..600b13a3 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -7,13 +7,13 @@ class Cursor attr_reader :client attr_reader :verb - attr_reader :pause + attr_reader :sleep_interval attr_reader :params def initialize(client, verb, params = {}) @client = client @verb = verb - @pause = params.delete(:pause) + @sleep_interval = params.delete(:sleep_interval) @params = params end @@ -31,7 +31,7 @@ def each break unless response.response_metadata next_cursor = response.response_metadata.next_cursor break if next_cursor.blank? - sleep(pause) if pause + sleep(sleep_interval) if sleep_interval end end end diff --git a/spec/slack/web/api/pagination/cursor_spec.rb b/spec/slack/web/api/pagination/cursor_spec.rb index 6c655d72..b0d11f9a 100644 --- a/spec/slack/web/api/pagination/cursor_spec.rb +++ b/spec/slack/web/api/pagination/cursor_spec.rb @@ -43,8 +43,8 @@ cursor.first end end - context 'with a custom pause' do - let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', pause: 3) } + context 'with a custom sleep_interval' do + let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', sleep_interval: 3) } it 'sleeps between requests' do expect(client).to receive(:users_list).exactly(3).times.and_return( Slack::Messages::Message.new(response_metadata: { next_cursor: 'next_a' }), From c8e11eb9b2f3b6e94433f436bac911d0cc562eb2 Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 18:19:56 -0400 Subject: [PATCH 7/9] don't retry after hitting max retries --- lib/slack/web/config.rb | 4 ++- lib/slack/web/pagination/cursor.rb | 7 ++++++ spec/slack/web/api/pagination/cursor_spec.rb | 26 ++++++++++++++------ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/lib/slack/web/config.rb b/lib/slack/web/config.rb index 0b5ca6d1..584e38ec 100644 --- a/lib/slack/web/config.rb +++ b/lib/slack/web/config.rb @@ -13,7 +13,8 @@ module Config :token, :timeout, :open_timeout, - :default_page_size + :default_page_size, + :default_max_retries ].freeze attr_accessor(*Config::ATTRIBUTES) @@ -29,6 +30,7 @@ def reset self.timeout = nil self.open_timeout = nil self.default_page_size = 100 + self.default_max_retries = 100 end end diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index 600b13a3..4631ebd6 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -8,22 +8,28 @@ class Cursor attr_reader :client attr_reader :verb attr_reader :sleep_interval + attr_reader :max_retries attr_reader :params def initialize(client, verb, params = {}) @client = client @verb = verb @sleep_interval = params.delete(:sleep_interval) + @max_retries = params.delete(:max_retries) || client.default_max_retries @params = params end def each next_cursor = nil + retry_count = 0 loop do query = { limit: client.default_page_size }.merge(params).merge(cursor: next_cursor) begin response = client.send(verb, query) rescue Slack::Web::Api::Errors::TooManyRequestsError => e + raise e if retry_count >= max_retries + client.logger.debug("#{self.class}##{__method__}") { e.to_s } + retry_count += 1 sleep(e.retry_after.seconds) next end @@ -31,6 +37,7 @@ def each break unless response.response_metadata next_cursor = response.response_metadata.next_cursor break if next_cursor.blank? + retry_count = 0 sleep(sleep_interval) if sleep_interval end end diff --git a/spec/slack/web/api/pagination/cursor_spec.rb b/spec/slack/web/api/pagination/cursor_spec.rb index b0d11f9a..efcc0595 100644 --- a/spec/slack/web/api/pagination/cursor_spec.rb +++ b/spec/slack/web/api/pagination/cursor_spec.rb @@ -26,13 +26,25 @@ end context 'with rate limiting' do let(:error) { Slack::Web::Api::Errors::TooManyRequestsError.new(nil) } - it 'sleeps after a TooManyRequestsError' do - expect(client).to receive(:users_list).with(limit: 100, cursor: nil).ordered.and_return(Slack::Messages::Message.new(response_metadata: { next_cursor: 'next' })) - expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_raise(error) - expect(error).to receive(:retry_after).once.ordered.and_return(9) - expect(cursor).to receive(:sleep).once.ordered.with(9) - expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_return(Slack::Messages::Message.new) - cursor.to_a + context 'with default max retries' do + it 'sleeps after a TooManyRequestsError' do + expect(client).to receive(:users_list).with(limit: 100, cursor: nil).ordered.and_return(Slack::Messages::Message.new(response_metadata: { next_cursor: 'next' })) + expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_raise(error) + expect(error).to receive(:retry_after).once.ordered.and_return(9) + expect(cursor).to receive(:sleep).once.ordered.with(9) + expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').ordered.and_return(Slack::Messages::Message.new) + cursor.to_a + end + end + context 'with a custom max_retries' do + let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', max_retries: 4) } + it 'raises the error after hitting the max retries' do + expect(client).to receive(:users_list).with(limit: 100, cursor: nil).and_return(Slack::Messages::Message.new(response_metadata: { next_cursor: 'next' })) + expect(client).to receive(:users_list).with(limit: 100, cursor: 'next').exactly(5).times.and_raise(error) + expect(error).to receive(:retry_after).exactly(4).times.and_return(9) + expect(cursor).to receive(:sleep).exactly(4).times.with(9) + expect { cursor.to_a }.to raise_error(error) + end end end end From 549563b022d453708c2b5691c0317005cc4efcf7 Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Mon, 18 Sep 2017 18:33:47 -0400 Subject: [PATCH 8/9] update readme and changelog --- CHANGELOG.md | 2 +- README.md | 39 +++++++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c92ada62..9db23175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ### 0.9.2 (Next) -* [#167](https://github.com/slack-ruby/slack-ruby-client/pull/167): Respect rate limits during pagination by sleeping. Also add optional `pause` parameter in order to proactively sleep between each paginated request - [@jmanian](https://github.com/jmanian). +* [#167](https://github.com/slack-ruby/slack-ruby-client/pull/167): Added support for pausing between paginated requests that can cause Slack rate limiting - [@jmanian](https://github.com/jmanian). * [#163](https://github.com/slack-ruby/slack-ruby-client/pull/164): Use `OpenSSL::X509::DEFAULT_CERT_DIR` and `OpenSSL::X509::DEFAULT_CERT_FILE` for default ca_cert and ca_file - [@leifcr](https://github.com/leifcr). * [#161](https://github.com/slack-ruby/slack-ruby-client/pull/161): Added support for cursor pagination - [@dblock](https://github.com/dblock). * Your contribution here. diff --git a/README.md b/README.md index 64ca935e..e8d38bdc 100644 --- a/README.md +++ b/README.md @@ -164,18 +164,19 @@ client = Slack::Web::Client.new(user_agent: 'Slack Ruby Client/1.0') The following settings are supported. -setting | description -------------------|------------------------------------------------------------------------------------------------- -token | Slack API token. -user_agent | User-agent, defaults to _Slack Ruby Client/version_. -proxy | Optional HTTP proxy. -ca_path | Optional SSL certificates path. -ca_file | Optional SSL certificates file. -endpoint | Slack endpoint, default is _https://slack.com/api_. -logger | Optional `Logger` instance that logs HTTP requests. -timeout | Optional open/read timeout in seconds. -open_timeout | Optional connection open timeout in seconds. -default_page_size | Optional page size for paginated requests, default is _100_. +setting | description +--------------------|------------------------------------------------------------------------------------------------- +token | Slack API token. +user_agent | User-agent, defaults to _Slack Ruby Client/version_. +proxy | Optional HTTP proxy. +ca_path | Optional SSL certificates path. +ca_file | Optional SSL certificates file. +endpoint | Slack endpoint, default is _https://slack.com/api_. +logger | Optional `Logger` instance that logs HTTP requests. +timeout | Optional open/read timeout in seconds. +open_timeout | Optional connection open timeout in seconds. +default_page_size | Optional page size for paginated requests, default is _100_. +default_max_retries | Optional number of retries for paginated requests, default is _100_. You can also pass request options, including `timeout` and `open_timeout` into individual calls. @@ -195,6 +196,20 @@ end all_members # many thousands of team members retrieved 10 at a time ``` +When using cursor pagination the client will automatically pause and then retry the request if it runs into Slack rate limiting. (It will pause according to the `Retry-After` header in the 429 response before retrying the request.) If it receives too many rate-limited responses in a row it will give up and raise an error. The default number of retries is 100 and can be adjusted via `Slack::Web::Client.config.default_max_retries` or by passing it directly into the method as `max_retries`. + +You can also proactively avoid rate limiting by adding a pause between every paginated request with the `sleep_interval` parameter, which is given in seconds. + +```ruby +all_members = [] +client.users_list(presence: true, limit: 10, sleep_interval: 5, max_retries: 20) do |response| + # pauses for 5 seconds between each request + # gives up after 20 consecutive rate-limited responses + all_members.concat(response.members) +end +all_members # many thousands of team members retrieved 10 at a time +``` + ### RealTime Client The Real Time Messaging API is a WebSocket-based API that allows you to receive events from Slack in real time and send messages as user. From f34811077f20a341bb7e8b20c5d0ce193c3023ed Mon Sep 17 00:00:00 2001 From: Jeff Manian Date: Tue, 19 Sep 2017 02:04:06 -0400 Subject: [PATCH 9/9] don't modify incoming params --- lib/slack/web/pagination/cursor.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/slack/web/pagination/cursor.rb b/lib/slack/web/pagination/cursor.rb index 4631ebd6..cbc4f350 100644 --- a/lib/slack/web/pagination/cursor.rb +++ b/lib/slack/web/pagination/cursor.rb @@ -14,9 +14,9 @@ class Cursor def initialize(client, verb, params = {}) @client = client @verb = verb - @sleep_interval = params.delete(:sleep_interval) - @max_retries = params.delete(:max_retries) || client.default_max_retries - @params = params + @sleep_interval = params[:sleep_interval] + @max_retries = params[:max_retries] || client.default_max_retries + @params = params.reject { |k, _| [:sleep_interval, :max_retries].include?(k) } end def each