os/slacker

Add support for Slack 429 response

pleasantone opened this issue · 8 comments

A slack client that sends too many responses at once can elicit a 429 status code, followed by a json error object saying how many seconds we need to pause.

https://api.slack.com/docs/rate-limits

There are two ways to skin this -- mange the 429 response ourself in BaseAPI._request
which gives us the advantage of being able to pause for exactly the right delay time, -or-
or ask the requests module to simply implement an incremental backoff algorithm to respect 429.

Either solution seems relatively reasonable.

For plan A, by default, requests will -not- retry a 429, so we'll get back the slack error response and can (if enabled by the user) sleep and then retry the request in BaseAPI._request with something vaguely like this (totally untested, and it's late at night when I wrote this):

success = False
while not success:
    response = method ...
    if response.status_code == 429 and self.backoff:
        # get the value of the Retry-after header in the response packet
        sleep(response.<headers>.Retry_after)
        continue
    response.raise_for_status()
    success = True    

For plan B, one would do something like this:

in init, create a session with a retry object on the HTTP adapter:

retries = Retry(status_forcelist={429})
adapter = requests.adapters.HTTPAdapter(retries)
self.session = requests.Session()
self.session.mount('https://', adapter)

and use self.session.get and self.session.post et all instead of requests.get / requests.post as the methods.

reference:

http://docs.python-requests.org/en/master/api/?highlight=retries
http://urllib3.readthedocs.io/en/latest/user-guide.html#retrying-requests
http://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#module-urllib3.util.retry

Slack docs are very vague:

Slack will start returning a HTTP 429 Too Many Requests error, a JSON object containing the number of calls you have been making, and a Retry-After header containing the number of seconds until you can retry.

@pleasantone , do you have an example of HTTP response body when status code is 429?

I'm sorry, I don't anymore.

Got one...
It sets 'Retry-After' (seconds) in the header.

                try:
                    slack.chat.delete(chan_id, msg['ts'])
                except requests.exceptions.HTTPError as err:
                    if err.response.status_code == 429:
                        time.sleep(int(err.response.headers['Retry-After']))
                        slack.chat.delete(chan_id, msg['ts'])
                    else:
                        raise

Any news on that subject ? Trying to get the headers in the Response(object), without success ..

Pull request to add this feature sent: #123

Oh, I didn't see pull request 114. Please do accept one of them!!

#123 fixes this.

os commented

Thanks guys for looking at it.