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

respect rate limits during pagination by sleeping #167

Merged
merged 9 commits into from
Sep 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### 0.9.2 (Next)

* [#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.
Expand Down
39 changes: 27 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion lib/slack/web/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ module Config
:token,
:timeout,
:open_timeout,
:default_page_size
:default_page_size,
:default_max_retries
].freeze

attr_accessor(*Config::ATTRIBUTES)
Expand All @@ -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

Expand Down
19 changes: 17 additions & 2 deletions lib/slack/web/pagination/cursor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,38 @@ 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
@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
next_cursor = nil
retry_count = 0
loop do
query = { limit: client.default_page_size }.merge(params).merge(cursor: next_cursor)
response = client.send(verb, query)
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
yield response
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can move this inside the rescue block and avoid next, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also did I tell you that I was a nitpicky code reviewer? Hang on tight :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand, I'm the same way 😎.

Which part are you suggesting go in the rescue block? I started out with everything inside the begin block (see first commit b75bfcb) which avoids the next. Is this what you mean?

I changed it (8542d00) because I thought it was bad practice to have more lines than necessary inside the begin block — even if just for clarity (so that it's clear which line is expected to raise the error). But in this case it's obvious where the error is coming from, so I guess it's fine either way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Exactly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually now think yours is better. Someone can do something inside the yield that causes that exception and we're going to have bad side effects and catching an error we shouldn't.

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
end
Expand Down
36 changes: 36 additions & 0 deletions spec/slack/web/api/pagination/cursor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,32 @@
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) }
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
context 'with a custom limit' do
let(:cursor) { Slack::Web::Api::Pagination::Cursor.new(client, 'users_list', limit: 42) }
Expand All @@ -31,4 +55,16 @@
cursor.first
end
end
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' }),
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