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

Handling Rate Limiting With v2 Commands #436

Closed
2 tasks done
CyTheGuy opened this issue Jun 6, 2019 · 30 comments
Closed
2 tasks done

Handling Rate Limiting With v2 Commands #436

CyTheGuy opened this issue Jun 6, 2019 · 30 comments
Assignees
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented enhancement M-T: A feature request for new functionality Version: 2x

Comments

@CyTheGuy
Copy link

CyTheGuy commented Jun 6, 2019

Description

What is the best way to handle web API rate limiting with the new v2 client? The docs https://slack.dev/python-slackclient/basic_usage.html [slack.dev] don't seem to work anymore and the release notes and migration notes didn't suggest anything. I now get this error when hitting rate limiting but there appears no way to handle it like before since the response is never returned to me it is just output as an error.

slack.errors.SlackApiError: The request to the Slack API failed.
The server responded with: {'ok': False, 'error': 'ratelimited'}

What type of issue is this? (place an x in one of the [ ])

  • question
  • documentation related

Requirements (place an x in each of the [ ])

  • [X ] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [X ] I've read and agree to the Code of Conduct.
  • [X ] I've searched for any related issues and avoided creating a duplicate issue.

Reproducible in:

slackclient version: v2

python version: Python 3.7.3

OS version(s): Mac
Logs, screenshots, screencast, sample project, funny gif, etc.

@RodneyU215 RodneyU215 self-assigned this Jun 6, 2019
@RodneyU215 RodneyU215 added Priority: Medium Version: 2x enhancement M-T: A feature request for new functionality and removed Type: Examples labels Jun 6, 2019
@RodneyU215
Copy link
Contributor

Hi @CyTheGuy, thanks for opening this Github issue! Providing support for better handling of rate limits is something that we're definitely interested in exploring. This is a feature we want to add, but it's important that we do so in a way that doesn't make it easy to abuse the API. I'm planning on spending more time on this over the next couple of weeks.

If you'd like to fix it locally in the time being here's some pseudo code that should get you close:

max_attempts = 3
for i in range(max_attempts):
    while True:
        try:
            client.chat_postMessage() # Whatever API call you're making.
        except client_err.SlackApiError as exception

            wait_time = float(exception.response["headers"]["Retry-After"])
            time.sleep(wait_time) # or await asyncio.sleep(wait_time) for async methods.
            continue
        break

This has also helped me identify an existing bug where the amount of time you should wait is not provided in the exception. I'll likely work on this very soon.

@RodneyU215 RodneyU215 added Priority: High bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented and removed Priority: Medium labels Jun 6, 2019
@CyTheGuy
Copy link
Author

CyTheGuy commented Jun 7, 2019

Woohoo! Thank you for getting back so quickly. I will give this a try thank you so much. Didn't realize I could handle the error myself like that. Previously I would just see if the error returned was "ratelimited" and sleep if it wasn't "ok" but this looks like a solid path forward for now. Thanks again

@DarrenN
Copy link

DarrenN commented Jun 10, 2019

@RodneyU215 Howdy, just wanted to point out a small wrinkle in this strategy of throwing Exceptions for rate limiting. The new Response object is a generator / iterable, which is super useful for paging through conversations.list. When I use a for loop like:

        for response in sc.api_call("conversations.list", params=params):
            ...

I can't catch the SlackApiError with a try and use continue. I have to wrap the call in another generator, manually calling next and catching exceptions.

UPDATE I can't even do that successfully because in __iter__ you set self._iteration which will only be set if I use a for in loop, it won't be set if I call yield on the Response object. Example:

def gen_wrapper(response):
    while True:
        try:
            # Returns Response object
            yield next(response)
        except StopIteration:
            raise
        except SlackApiError as exn:
           # rate limiting code here

Will error object has no attribute '_iteration' because __iter__() was never called.

@lukwam
Copy link

lukwam commented Jul 29, 2019

I am also unable to handle any rate limit issues with the newest slackclient for python.

try:
    response = client.users_profile_get(user=uid)
except slack.errors.SlackApiError as e:
    if e.response['ok'] is False and e.response['headers']['Retry-After']:
        delay = int(e.response['headers']['Retry-After'])
        print('Rate limited. Retrying in ' + str(delay) + ' seconds')
        time.sleep(delay)
        response = client.users_profile_get(user=uid)

However, the SlackApiError response dict has no headers key.

Traceback (most recent call last):
  File "./test.py", line 71, in <module>
    main()
  File "./test.py", line 66, in main
    profiles = s.get_users_profiles()
  File "/usr/src/bits/slack/__init__.py", line 201, in get_users_profiles
    if e.response['ok'] is False and e.response['headers']['Retry-After']:
KeyError: 'headers'

I've tried getting the headers from the response instead, but that doesn't work because the client.users_profile_get(user=uid) call doesn't return a value, it raises an exception.

It seems like there is no way to gracefully handle rate limit errors using the python client?

How do I get the headers from the SlackApiError object?

@ddnomad
Copy link

ddnomad commented Aug 7, 2019

@RodneyU215 Howdy, just wanted to point out a small wrinkle in this strategy of throwing Exceptions for rate limiting. The new Response object is a generator / iterable, which is super useful for paging through conversations.list. When I use a for loop like:

        for response in sc.api_call("conversations.list", params=params):
            ...

I can't catch the SlackApiError with a try and use continue. I have to wrap the call in another generator, manually calling next and catching exceptions.

UPDATE I can't even do that successfully because in __iter__ you set self._iteration which will only be set if I use a for in loop, it won't be set if I call yield on the Response object. Example:

def gen_wrapper(response):
    while True:
        try:
            # Returns Response object
            yield next(response)
        except StopIteration:
            raise
        except SlackApiError as exn:
           # rate limiting code here

Will error object has no attribute '_iteration' because __iter__() was never called.

So I've just encountered that thing and now wondering how to get around. conversations_history() method will raise at a random point in time and there is no obvious way of how to resume.

@lukwam
Copy link

lukwam commented Aug 7, 2019

I really don't see any way to handle rate limiting in this new version. Can someone show a working example of a loop where you hit the rate limit and then you handle it? My goal is to do the following:

  • get all users in my workspace
  • for each user get user profile

I always hit the rate limit half way through the user list but I haven't figured out how to handle it proper and proceed. How do you retrieve the response headers after you've received the exception?

@ddnomad
Copy link

ddnomad commented Aug 7, 2019

@lukwam It is broken (no offence, just stating the fact). I think for any fairly long-term project it is worth to use Web API directly (aiohttp for performance).

I had to time.sleep(X) each Y requests to at least make it work (but it is very slow, very hax and very inefficient). But I just needed a some kind of one-off ad hoc tool, so that's sufficient.

@axi0m axi0m mentioned this issue Aug 14, 2019
6 tasks
@CyTheGuy
Copy link
Author

CyTheGuy commented Sep 5, 2019

Yeah I honestly just decided to write everything in Go now and use https://github.com/nlopes/slack instead

@ddnomad
Copy link

ddnomad commented Sep 5, 2019

Yeah I honestly just decided to write everything in Go now and use https://github.com/nlopes/slack instead

https://github.com/os/slacker might be worth a try as well, though not sure how full featured it is.

@RodneyU215
Copy link
Contributor

Thanks for all of the feedback! I appreciate it. I'm going to be working on an update to address the challenges mentioned here.

@zxul767
Copy link

zxul767 commented Sep 9, 2019

Yeah I honestly just decided to write everything in Go now and use https://github.com/nlopes/slack instead

https://github.com/os/slacker might be worth a try as well, though not sure how full featured it is.

It was working well for me, until I found that the users_list method didn't accept a cursor. There is an open PR to address that, but it seems abandoned now.

@Gerardwx
Copy link

This used to work in v1. Here's how I updated my prior loop to work with the broken v2. I add a second delay on every error and subtract one thousandth on every success. (In v1 I delayed the result of "Retry-After").
Deleting one message:

 try:
            r = self.sc.chat_delete(channel=self.channel_id, ts=msg_ts)
        except slack.errors.SlackApiError as e:
            err = e.response['error']
            if err == "ratelimited":
                self.api_throttle += 1.0
                self.logger.warning("Throttling to {:f}".format(self.api_throttle))
                time.sleep(self.api_throttle)
                self._delete(msg_ts)
                return True
            else:
                raise e
        return False

and the loop that calls the method above

    while True:
            response = self.sc.channels_history(channel=self.channel_id, count=1000, latest=int(latest_ts))
            allmsgs = [item['ts'] for item in response['messages']]
            for msg in allmsgs:
                time.sleep(self.api_throttle)
                if not self._delete(msg) and self.api_throttle > 0:
                    self.api_throttle = max(self.api_throttle - 0.001,0)
                self.deleted += 1
                self.logger.debug("Throttle {:f}".format(self.api_throttle))
            if not response['has_more']:
                break

@spumer
Copy link

spumer commented Sep 12, 2019

So, actually we need raw response in exception

I wrote workaround for that:

from slack.errors import SlackApiError

class SlackApiError2(SlackApiError):
    def __init__(self, message, response, raw_response):
        super().__init__(message, response)
        self.raw_response = raw_response

    @classmethod
    def from_original(cls, exc: SlackApiError):
        response_validate_frame = next(
            frame for frame, _ in traceback.walk_tb(exc.__traceback__)
            if (
                frame.f_code.co_name == 'validate' and
                frame.f_code.co_filename.endswith('slack_response.py')
            )
        )

        message, _ = exc.args[0].split('\n', maxsplit=1)
        response = exc.response
        raw_response = response_validate_frame.f_locals['self']

        return cls(message, response, raw_response)

Then in application code:

        try:
            response = self.client.api_call(method, json=json, data=data, params=params, files=files)
        except SlackApiError as e:
            # here you can reraise exception, but i need it below
            response = SlackApiError2.from_original(e).raw_response

      # handle response

@myasul
Copy link

myasul commented Sep 26, 2019

Can you at least return the headers and status code when raising the Exception @RodneyU215 ? 😭

    def validate(self):
        """Check if the response from Slack was successful.

        Returns:
            (SlackResponse)
                This method returns it's own object. e.g. 'self'

        Raises:
            SlackApiError: The request to the Slack API failed.
        """
        if self.status_code == 200 and self.data.get("ok", False):
            self._logger.debug("Received the following response: %s", self.data)
            return self
        msg = "The request to the Slack API failed."
        raise e.SlackApiError(message=msg, response=self.data)

@spumer
Copy link

spumer commented Sep 26, 2019

If raw response will be present in SlackApiError, then we can get any data: status_code, headers, data, anything

@RodneyU215
Copy link
Contributor

Thanks for everyone's help and patience on this issue! 🙇 I've just released 2.2.1 and which includes a fix for this.

As shown in the test below your exception will have a response variable. This is the SlackResponse object that contains all of the information about the response we received from Slack.

def test_slack_api_rate_limiting_exception_returns_retry_after(self, mock_request):
    mock_request.response.side_effect = [
        {"data": {"ok": False}, "status_code": 429, "headers": {"Retry-After": 30}}
    ]
    with self.assertRaises(err.SlackApiError) as context:
        self.client.api_test()
    slack_api_error = context.exception
    self.assertFalse(slack_api_error.response["ok"])
    self.assertEqual(429, slack_api_error.response.status_code)
    self.assertEqual(30, slack_api_error.response.headers["Retry-After"])

@myasul
Copy link

myasul commented Oct 9, 2019

Thanks for everyone's help and patience on this issue! I've just released 2.2.1 and which includes a fix for this.

As shown in the test below your exception will have a response variable. This is the SlackResponse object that contains all of the information about the response we received from Slack.

def test_slack_api_rate_limiting_exception_returns_retry_after(self, mock_request):
    mock_request.response.side_effect = [
        {"data": {"ok": False}, "status_code": 429, "headers": {"Retry-After": 30}}
    ]
    with self.assertRaises(err.SlackApiError) as context:
        self.client.api_test()
    slack_api_error = context.exception
    self.assertFalse(slack_api_error.response["ok"])
    self.assertEqual(429, slack_api_error.response.status_code)
    self.assertEqual(30, slack_api_error.response.headers["Retry-After"])

Thanks a lot man @RodneyU215 ! This saved me from my horrific workaround code!

@senpay
Copy link

senpay commented Feb 1, 2020

It was working well for me, until I found that the users_list method didn't accept a cursor. There is an open PR to address that, but it seems abandoned now.

@zxul767 Hello, just in case you're interested - I have forked slacker and merged the abandon PR there. New package is available in Pypi by name "Slacker2"

@abdulowork
Copy link

abdulowork commented Mar 18, 2020

@RodneyU215 thank you for the example! Perhaps it would be nice to update the example in the web api rate limits section of the documentation as it is somewhat misleading right now.

@seratch
Copy link
Member

seratch commented Mar 18, 2020

@Biboran Thanks for your feedback! I'll fix it soon. #640

@AlexisMundu-zz
Copy link

Hi,

I tried with the example code but I keep getting a response from the API without ["headers"]["Retry-After"].

slack.errors.SlackApiError: The request to the Slack API failed.
The server responded with: {'ok': False, 'error': 'ratelimited'}

I have slackclient==2.5.0

Am I missing something?

@seratch
Copy link
Member

seratch commented Apr 9, 2020

@AlexisMundu As discussed above, the document should be updated (I will work on it soon #640). You can access the header value the following way.

from slack import WebClient
from slack.errors import SlackApiError

token = os.environ['SLACK_BOT_TOKEN']
webclient = WebClient(token=token)

try:
    result = webclient.conversations_list()
except SlackApiError as e:
    print(e.response.headers['Retry-After'])

@AlexisMundu-zz
Copy link

AlexisMundu-zz commented Apr 10, 2020

Oh I see, thanks a lot @seratch 👍

@neelraman
Copy link

neelraman commented May 28, 2020

@seratch I've found that whenever I hit the rate limit, the response headers always have Retry-After = 1, which is clearly not enough time. Further, even if I wait for 30 seconds instead, I've found that it can sometimes take 6+ attempts (so 3 minutes of waiting) before returning successful responses again. The methods I'm using are all in Tiers 3 and 4, so I'm a bit confused how I'm even hitting the limit in the first place and how it takes so long to cool down. Any thoughts? Thanks!

@seratch
Copy link
Member

seratch commented May 28, 2020

The methods I'm using are all in Tiers 3 and 4,

@neelraman Can you share the list of methods (no need to be a complete list)? I can verify with those, just in case.

@neelraman
Copy link

@seratch thanks for the quick response! I did some further research, and it's actually only conversations.replies that is consistently giving me Retry-After=1 when I hit the rate limit.

@seratch
Copy link
Member

seratch commented Jun 2, 2020

@neelraman I've confirmed this with the server-side teams. The Retry-After looks a bit generous but the API is still Tier 3.

If retries after 1 second in the case don't work for you, could you submit an inquiry to our support team? They can investigate more deeply by accessing the data of your apps and workspaces.

@pestophagous
Copy link

@neelraman @seratch has there been any conclusive investigation regarding the issue with ratelimited specifically on conversations.replies always showing a headers['retry-after'] equal to 1 second 100% of the time?

I recently updated a python script I have used since 2017 so that I could replace the deprecated 'im.history' and 'groups.history' methods, and I am now running into this same problem that @neelraman reported.

@pestophagous
Copy link

Actually, in my case the retry-after 1 second is being honored.

As a quick test, I edited my script to sleep for 2 seconds whenever this happens, and now I see a pattern of four requests succeeding, then the fifth being rate limited, then 2 seconds sleep. Then repeat ad infinitum: 4 succeed, 5th one ratelimited, sleep for 2 seconds. This is holding for approximately 40 minutes straight just now.

So perhaps the 1 value is "sincere" and accurate? It would be good to hear official confirmation of that, still. Thanks :)

@cj81499
Copy link

cj81499 commented Jan 26, 2023

To anyone who finds this issue, check out RateLimitErrorRetryHandler (added by #1084)!

https://slack.dev/python-slack-sdk/web/index.html#retryhandler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented enhancement M-T: A feature request for new functionality Version: 2x
Projects
None yet
Development

No branches or pull requests