diff --git a/integration_tests/web/test_admin_rate_limit_retries.py b/integration_tests/web/test_admin_rate_limit_retries.py new file mode 100644 index 000000000..9f10a4686 --- /dev/null +++ b/integration_tests/web/test_admin_rate_limit_retries.py @@ -0,0 +1,44 @@ +import logging +import os +import unittest + +import pytest + +from integration_tests.env_variable_names import ( + SLACK_SDK_TEST_GRID_ORG_ADMIN_USER_TOKEN, +) +from integration_tests.helpers import async_test, is_not_specified +from slack_sdk.http_retry import RateLimitErrorRetryHandler +from slack_sdk.http_retry.builtin_async_handlers import AsyncRateLimitErrorRetryHandler +from slack_sdk.web import WebClient +from slack_sdk.web.async_client import AsyncWebClient + + +class TestWebClient(unittest.TestCase): + """Runs integration tests with real Slack API""" + + def setUp(self): + self.logger = logging.getLogger(__name__) + self.org_admin_token = os.environ[SLACK_SDK_TEST_GRID_ORG_ADMIN_USER_TOKEN] + self.sync_client: WebClient = WebClient(token=self.org_admin_token) + self.sync_client.retry_handlers.append(RateLimitErrorRetryHandler(max_retry_count=2)) + self.async_client: AsyncWebClient = AsyncWebClient(token=self.org_admin_token) + self.async_client.retry_handlers.append( + AsyncRateLimitErrorRetryHandler(max_retry_count=2) + ) + + def tearDown(self): + pass + + @pytest.mark.skipif(condition=is_not_specified(), reason="execution can take long") + def test_sync(self): + client = self.sync_client + for response in client.admin_users_session_list(limit=1): + self.assertIsNotNone(response.get("active_sessions")) + + @pytest.mark.skipif(condition=is_not_specified(), reason="execution can take long") + @async_test + async def test_async(self): + client = self.async_client + async for response in await client.admin_users_session_list(limit=1): + self.assertIsNotNone(response.get("active_sessions")) diff --git a/slack_sdk/http_retry/builtin_async_handlers.py b/slack_sdk/http_retry/builtin_async_handlers.py index 9d381de47..8a9943877 100644 --- a/slack_sdk/http_retry/builtin_async_handlers.py +++ b/slack_sdk/http_retry/builtin_async_handlers.py @@ -70,7 +70,7 @@ async def prepare_for_next_attempt_async( if response is None: raise error - state.increment_current_attempt() + state.next_attempt_requested = True retry_after_header_name: Optional[str] = None for k in response.headers.keys(): if k.lower() == "retry-after": @@ -85,6 +85,7 @@ async def prepare_for_next_attempt_async( int(response.headers.get(retry_after_header_name)[0]) + random.random() ) await asyncio.sleep(duration) + state.increment_current_attempt() def async_default_handlers() -> List[AsyncRetryHandler]: diff --git a/slack_sdk/http_retry/builtin_handlers.py b/slack_sdk/http_retry/builtin_handlers.py index dd38c6397..287e5bf4a 100644 --- a/slack_sdk/http_retry/builtin_handlers.py +++ b/slack_sdk/http_retry/builtin_handlers.py @@ -39,6 +39,10 @@ def _can_retry( if error is None: return False + if isinstance(error, URLError): + if response is not None: + return False # status 40x + for error_type in self.error_types_to_do_retries: if isinstance(error, error_type): return True @@ -69,7 +73,7 @@ def prepare_for_next_attempt( if response is None: raise error - state.increment_current_attempt() + state.next_attempt_requested = True retry_after_header_name: Optional[str] = None for k in response.headers.keys(): if k.lower() == "retry-after": @@ -84,3 +88,4 @@ def prepare_for_next_attempt( int(response.headers.get(retry_after_header_name)[0]) + random.random() ) time.sleep(duration) + state.increment_current_attempt() diff --git a/slack_sdk/http_retry/response.py b/slack_sdk/http_retry/response.py index 34f5673cd..028f8230a 100644 --- a/slack_sdk/http_retry/response.py +++ b/slack_sdk/http_retry/response.py @@ -12,12 +12,12 @@ class HttpResponse: def __init__( self, *, - status_code: int, + status_code: Union[int, str], headers: Dict[str, Union[str, List[str]]], body: Optional[Dict[str, Any]] = None, data: Optional[bytes] = None, ): - self.status_code = status_code + self.status_code = int(status_code) self.headers = { k: v if isinstance(v, list) else [v] for k, v in headers.items() } diff --git a/slack_sdk/web/async_internal_utils.py b/slack_sdk/web/async_internal_utils.py index ba423531f..0f36bbd0d 100644 --- a/slack_sdk/web/async_internal_utils.py +++ b/slack_sdk/web/async_internal_utils.py @@ -143,6 +143,15 @@ def convert_params(values: dict) -> dict: res, ) + if logger.level <= logging.DEBUG: + body = data if isinstance(data, dict) else "(binary)" + logger.debug( + "Received the following response - " + f"status: {res.status}, " + f"headers: {dict(res.headers)}, " + f"body: {body}" + ) + if res.status == 429: for handler in retry_handlers: if await handler.can_retry_async( diff --git a/slack_sdk/web/base_client.py b/slack_sdk/web/base_client.py index c142b0599..f8edddaff 100644 --- a/slack_sdk/web/base_client.py +++ b/slack_sdk/web/base_client.py @@ -522,10 +522,24 @@ def _perform_urllib_http_request_internal( if resp.headers.get_content_type() == "application/gzip": # admin.analytics.getFile body: bytes = resp.read() + if self._logger.level <= logging.DEBUG: + self._logger.debug( + "Received the following response - " + f"status: {resp.code}, " + f"headers: {dict(resp.headers)}, " + f"body: (binary)" + ) return {"status": resp.code, "headers": resp.headers, "body": body} charset = resp.headers.get_content_charset() or "utf-8" body: str = resp.read().decode(charset) # read the response body here + if self._logger.level <= logging.DEBUG: + self._logger.debug( + "Received the following response - " + f"status: {resp.code}, " + f"headers: {dict(resp.headers)}, " + f"body: {body}" + ) return {"status": resp.code, "headers": resp.headers, "body": body} raise SlackRequestError(f"Invalid URL detected: {url}") diff --git a/slack_sdk/web/slack_response.py b/slack_sdk/web/slack_response.py index 7d47f145c..31d3d8daa 100644 --- a/slack_sdk/web/slack_response.py +++ b/slack_sdk/web/slack_response.py @@ -186,14 +186,6 @@ def validate(self): Raises: SlackApiError: The request to the Slack API failed. """ - if self._logger.level <= logging.DEBUG: - body = self.data if isinstance(self.data, dict) else "(binary)" - self._logger.debug( - "Received the following response - " - f"status: {self.status_code}, " - f"headers: {dict(self.headers)}, " - f"body: {body}" - ) if ( self.status_code == 200 and self.data