Fix #718 by handling unexpected response body format (#751)

seratch · web-flow · commit 93a4731bfda2 · 2020-07-16T16:49:45.000+09:00
diff --git a/slack/web/base_client.py b/slack/web/base_client.py
@@ -303,6 +303,10 @@ async def _request(self, *, http_verb, api_url, req_args):
                     self._logger.debug(
                         f"No response data returned from the following API call: {api_url}."
                     )
+                except json.decoder.JSONDecodeError as e:
+                    message = f"Failed to parse the response body: {str(e)}"
+                    raise err.SlackApiError(message, res)
+
                 response = {
                     "data": data,
                     "headers": res.headers,
@@ -456,7 +460,11 @@ def convert_params(values: dict) -> dict:
 
             response = self._perform_urllib_http_request(url=url, args=request_args)
             if response.get("body", None):
-                response_body_data: dict = json.loads(response["body"])
+                try:
+                    response_body_data: dict = json.loads(response["body"])
+                except json.decoder.JSONDecodeError as e:
+                    message = f"Failed to parse the response body: {str(e)}"
+                    raise err.SlackApiError(message, response)
             else:
                 response_body_data: dict = None
 
diff --git a/tests/web/mock_web_api_server.py b/tests/web/mock_web_api_server.py
@@ -18,6 +18,8 @@ class MockHandler(SimpleHTTPRequestHandler):
     pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE)
     pattern_for_package_identifier = re.compile("slackclient/(\\S+)")
 
+    html_response_body = '<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML 2.0//EN\">\n<html><head>\n<title>404 Not Found</title>\n</head><body>\n<h1>Not Found</h1>\n<p>The requested URL /api/team.info was not found on this server.</p>\n</body></html>\n'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(user_agent) \
@@ -98,6 +100,13 @@ def _handle(self):
                     self.wfile.close()
                     return
 
+                if pattern == "html_response":
+                    self.send_response(404)
+                    self.set_common_headers()
+                    self.wfile.write(self.html_response_body.encode("utf-8"))
+                    self.wfile.close()
+                    return
+
                 if request_body and "cursor" in request_body:
                     page = request_body["cursor"]
                     pattern = f"{pattern}_{page}"
diff --git a/tests/web/test_web_client.py b/tests/web/test_web_client.py
@@ -243,4 +243,23 @@ async def issue_645():
         loop.set_exception_handler(exception_handler)
         loop.run_until_complete(issue_645())
         gc.collect()  # force Python to gc unclosed client session
-        self.assertFalse(session_unclosed, "Unclosed client session")
+        self.assertFalse(session_unclosed, "Unclosed client session")
+
+    def test_html_response_body_issue_718(self):
+        client = WebClient(base_url="http://localhost:8888")
+        try:
+            client.users_list(token="xoxb-html_response")
+            self.fail("SlackApiError expected here")
+        except err.SlackApiError as e:
+            self.assertTrue(
+                str(e).startswith("Failed to parse the response body: Expecting value: line 1 column 1 (char 0)"), e)
+
+    @async_test
+    async def test_html_response_body_issue_718_async(self):
+        client = WebClient(base_url="http://localhost:8888", run_async=True)
+        try:
+            await client.users_list(token="xoxb-html_response")
+            self.fail("SlackApiError expected here")
+        except err.SlackApiError as e:
+            self.assertTrue(
+                str(e).startswith("Failed to parse the response body: Expecting value: line 1 column 1 (char 0)"), e)
