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

Handle Slack API rate limiting #151

Merged
merged 4 commits into from
Sep 25, 2019
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
36 changes: 35 additions & 1 deletion response/slack/client.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import re
import time

import slackclient
from django.conf import settings
Expand All @@ -15,15 +16,48 @@ def __init__(self, message, slack_error=None):


class SlackClient(object):
def __init__(self, api_token):
def __init__(
self,
api_token,
max_retry_attempts=10,
retry_base_backoff_seconds=0.2,
retryable_errors=None,
):
self.api_token = api_token
self.client = slackclient.SlackClient(self.api_token)
self.max_retry_attempts = max_retry_attempts
self.retry_base_backoff_seconds = retry_base_backoff_seconds
self.retryable_errors = retryable_errors or ["ratelimited"]

def api_call(self, api_endpoint, *args, **kwargs):
logger.info(f"Calling Slack API {api_endpoint}")
response = self.client.api_call(api_endpoint, *args, **kwargs)
if not response.get("ok", False):
error = response.get("error", "<no error given>")

# Check if we want to back off and retry (but only if this isn't a
# retry attempt itself)
if error in self.retryable_errors and not kwargs.get("is_retrying", False):
# Iterate over retry attempts. We could implement this
# recursively, but there's a danger of overflowing the stack

# Start index at 1 so we have a multiplier for backoff
for i in range(1, self.max_retry_attempts + 1):
try:
# Increase backoff with every attempt
backoff_seconds = self.retry_base_backoff_seconds * i
logging.warn(
f"Retrying request to {api_endpoint} after error {error}. Backing off {backoff_seconds:.2f}s (attempt {i} of {self.max_retry_attempts})"
)

time.sleep(backoff_seconds)
return self.api_call(
api_endpoint, is_retrying=True, *args, **kwargs
)
except SlackError as exc:
if exc.slack_error not in self.retryable_errors:
raise exc

raise SlackError(
f"Error calling Slack API endpoint '{api_endpoint}': {error}",
slack_error=error,
Expand Down
66 changes: 66 additions & 0 deletions tests/slack/test_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
from unittest import mock

import pytest
import slackclient
from django.conf import settings

from response.slack import client


@pytest.fixture
def slack_api_mock(monkeypatch):
client_mock = mock.Mock(spec=slackclient.SlackClient)
monkeypatch.setattr(settings.SLACK_CLIENT, "client", client_mock)
return client_mock


@pytest.fixture
def slack_client(slack_api_mock):
# We set the backoff ridiculously low so that we're not hanging around
# waiting for retries in tests
c = client.SlackClient("test-token", retry_base_backoff_seconds=0.0000001)
c.client = slack_api_mock
return c


def test_slack_api_call_success(slack_client, slack_api_mock):
expected_resp = {"ok": True, "response": "bar"}
slack_api_mock.api_call.return_value = expected_resp

resp = slack_client.api_call("test_endpoint", "arg1", kwarg2="foo")
assert resp == expected_resp
slack_api_mock.api_call.assert_called_with("test_endpoint", "arg1", kwarg2="foo")


def test_slack_api_call_error(slack_client, slack_api_mock):
slack_api_mock.api_call.return_value = {"ok": False, "error": "test_error"}

with pytest.raises(client.SlackError) as e:
slack_client.api_call("test_endpoint", "arg1", kwarg2="foo")
assert e.slack_error == "test_error"

slack_api_mock.api_call.assert_called_with("test_endpoint", "arg1", kwarg2="foo")


def test_slack_backoff_rate_limit_succeeded(slack_client, slack_api_mock):
expected_resp = {"ok": True, "response": "bar"}
slack_api_mock.api_call.side_effect = [
{"ok": False, "error": "ratelimited"},
{"ok": False, "error": "ratelimited"},
{"ok": False, "error": "ratelimited"},
{"ok": False, "error": "ratelimited"},
{"ok": False, "error": "ratelimited"},
expected_resp,
]

resp = slack_client.api_call("test_endpoint", "arg1", kwarg2="foo")
assert resp == expected_resp
assert slack_api_mock.api_call.call_count == 6


def test_slack_backoff_rate_limit_max_retry_attempts(slack_client, slack_api_mock):
slack_api_mock.api_call.return_value = {"ok": False, "error": "ratelimited"}

with pytest.raises(client.SlackError) as e:
slack_client.api_call("test_endpoint", "arg1", kwarg2="foo")
assert e.slack_error == "test_error"