Fix #1377 Add unit tests for #1367 changes

slack_sdk/audit_logs/v1/async_client.py

@@ -288,6 +288,10 @@ async def _perform_http_request(
                             )
                         except aiohttp.ContentTypeError:
                             self.logger.debug(f"No response data returned from the following API call: {url}.")
+                            retry_response = RetryHttpResponse(
+                                status_code=res.status,
+                                headers=res.headers,
+                            )
                         except json.decoder.JSONDecodeError as e:
                             message = f"Failed to parse the response body: {str(e)}"
                             raise SlackApiError(message, res)

slack_sdk/http_retry/builtin_async_handlers.py

@@ -54,8 +54,8 @@ async def _can_retry_async(
         *,
         state: RetryState,
         request: HttpRequest,
-        response: Optional[HttpResponse],
-        error: Optional[Exception],
+        response: Optional[HttpResponse] = None,
+        error: Optional[Exception] = None,
     ) -> bool:
         return response is not None and response.status_code == 429
 
@@ -101,8 +101,8 @@ async def _can_retry_async(
         *,
         state: RetryState,
         request: HttpRequest,
-        response: Optional[HttpResponse],
-        error: Optional[Exception],
+        response: Optional[HttpResponse] = None,
+        error: Optional[Exception] = None,
     ) -> bool:
         return response is not None and response.status_code in [500, 503]
 

slack_sdk/scim/v1/async_client.py

@@ -333,6 +333,10 @@ async def _perform_http_request(
                             )
                         except aiohttp.ContentTypeError:
                             self.logger.debug(f"No response data returned from the following API call: {url}.")
+                            retry_response = RetryHttpResponse(
+                                status_code=res.status,
+                                headers=res.headers,
+                            )
 
                         if res.status == 429:
                             for handler in self.retry_handlers:

slack_sdk/web/async_internal_utils.py

@@ -127,6 +127,10 @@ def convert_params(values: dict) -> dict:
                             )
                         except aiohttp.ContentTypeError:
                             logger.debug(f"No response data returned from the following API call: {api_url}.")
+                            retry_response = RetryHttpResponse(
+                                status_code=res.status,
+                                headers=res.headers,
+                            )
                         except json.decoder.JSONDecodeError:
                             try:
                                 body: str = await res.text()

slack_sdk/web/async_slack_response.py

@@ -199,5 +199,5 @@ def validate(self):
         """
         if self.status_code == 200 and self.data and (isinstance(self.data, bytes) or self.data.get("ok", False)):
             return self
-        msg = f"The request to the Slack API failed. (url: {self.api_url})"
+        msg = f"The request to the Slack API failed. (url: {self.api_url}, status: {self.status_code})"
         raise e.SlackApiError(message=msg, response=self)

slack_sdk/webhook/async_client.py

@@ -202,6 +202,10 @@ async def _perform_http_request(self, *, body: Dict[str, Any], headers: Dict[str
                             )
                         except aiohttp.ContentTypeError:
                             self.logger.debug(f"No response data returned from the following API call: {self.url}")
+                            retry_response = RetryHttpResponse(
+                                status_code=res.status,
+                                headers=res.headers,
+                            )
 
                         if res.status == 429:
                             for handler in self.retry_handlers:

tests/slack_sdk/web/mock_web_api_server.py

@@ -28,7 +28,7 @@ class MockHandler(SimpleHTTPRequestHandler):
 
     error_html_response_body = '<!DOCTYPE html>\n<html lang="en">\n<head>\n\t<meta charset="utf-8">\n\t<title>Server Error | Slack</title>\n\t<meta name="author" content="Slack">\n\t<style></style>\n</head>\n<body>\n\t<nav class="top persistent">\n\t\t<a href="https://status.slack.com/" class="logo" data-qa="logo"></a>\n\t</nav>\n\t<div id="page">\n\t\t<div id="page_contents">\n\t\t\t<h1>\n\t\t\t\t<svg width="30px" height="27px" viewBox="0 0 60 54" class="warning_icon"><path d="" fill="#D94827"/></svg>\n\t\t\t\tServer Error\n\t\t\t</h1>\n\t\t\t<div class="card">\n\t\t\t\t<p>It seems like there’s a problem connecting to our servers, and we’re investigating the issue.</p>\n\t\t\t\t<p>Please <a href="https://status.slack.com/">check our Status page for updates</a>.</p>\n\t\t\t</div>\n\t\t</div>\n\t</div>\n\t<script type="text/javascript">\n\t\tif (window.desktop) {\n\t\t\tdocument.documentElement.className = \'desktop\';\n\t\t}\n\n\t\tvar FIVE_MINS = 5 * 60 * 1000;\n\t\tvar TEN_MINS = 10 * 60 * 1000;\n\n\t\tfunction randomBetween(min, max) {\n\t\t\treturn Math.floor(Math.random() * (max - (min + 1))) + min;\n\t\t}\n\n\t\twindow.setTimeout(function () {\n\t\t\twindow.location.reload(true);\n\t\t}, randomBetween(FIVE_MINS, TEN_MINS));\n\t</script>\n</body>\n</html>'
 
-    state = {"ratelimited_count": 0, "fatal_error_count": 0}
+    state = {"ratelimited_count": 0, "fatal_error_count": 0, "server_error_count": 0}
 
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
@@ -184,6 +184,23 @@ def _handle(self):
                     self.wfile.write(self.error_html_response_body.encode("utf-8"))
                     self.wfile.close()
                     return
+                if pattern == "server_error_only_once":
+                    if self.state["server_error_count"] == 0:
+                        self.state["server_error_count"] += 1
+                        self.send_response(500)
+                        # no charset here is intentional for testing
+                        self.send_header("content-type", "text/html")
+                        self.send_header("connection", "close")
+                        self.end_headers()
+                        self.wfile.write(self.error_html_response_body.encode("utf-8"))
+                        self.wfile.close()
+                        return
+                    else:
+                        self.send_response(200)
+                        self.set_common_headers()
+                        self.wfile.write("""{"ok":true}""".encode("utf-8"))
+                        self.wfile.close()
+                        return
 
                 if pattern.startswith("user-agent"):
                     elements = pattern.split(" ")

tests/slack_sdk/web/test_web_client_http_retry_server_error.py

@@ -2,6 +2,7 @@
 
 import slack_sdk.errors as err
 from slack_sdk.http_retry import RetryHandler, RetryIntervalCalculator
+from slack_sdk.http_retry.builtin_handlers import ServerErrorRetryHandler
 from slack_sdk.http_retry.handler import default_interval_calculator
 from slack_sdk.web import WebClient
 from tests.slack_sdk.web.mock_web_api_server import (
@@ -10,7 +11,7 @@
 )
 
 
-class ServerErrorRetryHandler(RetryHandler):
+class MyServerErrorRetryHandler(RetryHandler):
     """RetryHandler that does retries for server-side errors."""
 
     def __init__(
@@ -41,7 +42,7 @@ def tearDown(self):
         cleanup_mock_web_api_server(self)
 
     def test_html_response_body_issue_829(self):
-        retry_handlers = [ServerErrorRetryHandler(max_retry_count=2)]
+        retry_handlers = [MyServerErrorRetryHandler(max_retry_count=2)]
         client = WebClient(
             base_url="http://localhost:8888",
             retry_handlers=retry_handlers,
@@ -53,3 +54,11 @@ def test_html_response_body_issue_829(self):
             self.assertTrue(str(e).startswith("Received a response in a non-JSON format: "), e)
 
         self.assertEqual(2, retry_handlers[0].call_count)
+
+    def test_server_error_using_built_in_handler(self):
+        retry_handlers = [ServerErrorRetryHandler()]
+        client = WebClient(
+            base_url="http://localhost:8888",
+            retry_handlers=retry_handlers,
+        )
+        client.users_list(token="xoxb-server_error_only_once")

tests/slack_sdk_async/web/test_async_web_client.py

@@ -136,7 +136,7 @@ async def test_html_response_body_issue_718_async(self):
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
             self.assertEqual(
-                "The request to the Slack API failed. (url: http://localhost:8888/users.list)\n"
+                "The request to the Slack API failed. (url: http://localhost:8888/users.list, status: 404)\n"
                 "The server responded with: {}",
                 str(e),
             )

tests/slack_sdk_async/web/test_async_web_client_http_retry.py

@@ -1,7 +1,7 @@
 import unittest
 
 import slack_sdk.errors as err
-from slack_sdk.http_retry.builtin_async_handlers import AsyncRateLimitErrorRetryHandler
+from slack_sdk.http_retry.builtin_async_handlers import AsyncRateLimitErrorRetryHandler, AsyncServerErrorRetryHandler
 from slack_sdk.web.async_client import AsyncWebClient
 from tests.slack_sdk_async.helpers import async_test
 from tests.slack_sdk.web.mock_web_api_server import (
@@ -19,10 +19,6 @@ def setUp(self):
     def tearDown(self):
         cleanup_mock_web_api_server(self)
 
-    @async_test
-    async def test_api_calls_return_a_future(self):
-        pass
-
     @async_test
     async def test_remote_disconnected(self):
         retry_handler = MyRetryHandler(max_retry_count=2)
@@ -88,3 +84,30 @@ async def test_fatal_error(self):
         client.retry_handlers.append(FatalErrorRetryHandler())
         # The auto-retry should work here
         await client.auth_test()
+
+    @async_test
+    async def test_retries(self):
+        retry_handler = MyRetryHandler(max_retry_count=2)
+        client = AsyncWebClient(
+            token="xoxp-remote_disconnected",
+            base_url="http://localhost:8888",
+            retry_handlers=[retry_handler],
+        )
+        try:
+            await client.auth_test()
+            self.fail("An exception is expected")
+        except Exception as _:
+            pass
+
+        self.assertEqual(2, retry_handler.call_count)
+
+    @async_test
+    async def test_server_error(self):
+        client = AsyncWebClient(
+            base_url="http://localhost:8888",
+            token="xoxb-server_error_only_once",
+            team_id="T111",
+        )
+        client.retry_handlers.append(AsyncServerErrorRetryHandler())
+        # The auto-retry should work here
+        await client.chat_postMessage(channel="C123", text="Hi there!")

tests/slack_sdk_async/web/test_web_client_issue_829.py

@@ -24,7 +24,7 @@ async def test_html_response_body_issue_829_async(self):
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
             self.assertEqual(
-                "The request to the Slack API failed. (url: http://localhost:8888/users.list)\n"
+                "The request to the Slack API failed. (url: http://localhost:8888/users.list, status: 503)\n"
                 "The server responded with: {}",
                 str(e),
             )

tests/web/test_async_web_client.py

@@ -131,7 +131,7 @@ async def test_html_response_body_issue_718_async(self):
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
             self.assertEqual(
-                "The request to the Slack API failed. (url: http://localhost:8888/users.list)\n"
+                "The request to the Slack API failed. (url: http://localhost:8888/users.list, status: 404)\n"
                 "The server responded with: {}",
                 str(e),
             )