Repository: slackapi/python-slackclient (python)

Issue:
TypeError - decode() argument 'encoding' must be str, not None
It seems slack just had a blip and returned some kind of error responses that did not feature the charset header. This broke in the SDK with:

```
TypeError - decode() argument 'encoding' must be str, not None
```

On this line:

https://github.com/slackapi/python-slackclient/blob/5a2c80eb78fa82c2805251a60bc3e0eccb0b1269/slack/web/base_client.py#L489

From the Sentry local-caputring I can see `charset` was `None` - in typing terms it's `Optional[str]` being used as a `str`.

### Reproducible in:

Requires slack returning these unexpected responses, or simulating them

#### The Slack SDK version

2.9.1

#### Python runtime version

3.8.5

#### OS info

Linux

#### Steps to reproduce:

As above

### Expected result:

Some kind of fallback decoding?

### Actual result:

`TypeError` raised

### Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. :bow:

Please read the [Contributing guidelines](https://github.com/slackapi/python-slackclient/blob/main/.github/contributing.md) and [Code of Conduct](https://slackhq.github.io/code-of-conduct) before creating this issue or pull request. By submitting, you are agreeing to those rules.


Pull request description:
Fix #829 by adding the default encoding
## Summary

This pull request fixes #829 by adding default values for the charset that is supposed to be used for parsing response body from Slack. The situation may arise either when the `content-type` header is missing or `content-type` header doesn't contain charset information in it.

We can safely assume "utf-8" is the one it can fallback in the case where the server-side cannot return a valid response.

### Category (place an `x` in each of the `[ ]`)

- [x] **slack.web.WebClient** (Web API client)
- [x] **slack.webhook.WebhookClient** (Incoming Webhook, response_url sender)
- [ ] **slack.web.classes** (UI component builders)
- [ ] **slack.rtm.RTMClient** (RTM client)
- [ ] Documents
- [ ] Others

## Requirements (place an `x` in each `[ ]`)

- [x] I've read and understood the [Contributing Guidelines](https://github.com/slackapi/python-slackclient/blob/main/.github/contributing.md) and have done my best effort to follow them.
- [x] I've read and agree to the [Code of Conduct](https://slackhq.github.io/code-of-conduct).
- [x] I've run `python setup.py validate` after making the changes.


Patch:
diff --git a/slack/web/base_client.py b/slack/web/base_client.py
index 9f38dca..26778e6 100644
--- a/slack/web/base_client.py
+++ b/slack/web/base_client.py
@@ -475,7 +475,7 @@ class BaseClient:
                     resp = urlopen(  # skipcq: BAN-B310
                         req, context=self.ssl, timeout=self.timeout
                     )
-                charset = resp.headers.get_content_charset()
+                charset = resp.headers.get_content_charset() or "utf-8"
                 body: str = resp.read().decode(charset)  # read the response body here
                 return {"status": resp.code, "headers": resp.headers, "body": body}
             raise SlackRequestError(f"Invalid URL detected: {url}")
@@ -485,7 +485,7 @@ class BaseClient:
                 # for compatibility with aiohttp
                 resp["headers"]["Retry-After"] = resp["headers"]["retry-after"]
 
-            charset = e.headers.get_content_charset()
+            charset = e.headers.get_content_charset() or "utf-8"
             body: str = e.read().decode(charset)  # read the response body here
             resp["body"] = body
             return resp
diff --git a/slack/webhook/client.py b/slack/webhook/client.py
index cf2e62f..ac376cc 100644
--- a/slack/webhook/client.py
+++ b/slack/webhook/client.py
@@ -144,7 +144,7 @@ class WebhookClient:
             return resp
 
         except HTTPError as e:
-            charset = e.headers.get_content_charset()
+            charset = e.headers.get_content_charset() or "utf-8"
             body: str = e.read().decode(charset)  # read the response body here
             resp = WebhookResponse(
                 url=url, status_code=e.code, body=body, headers=e.headers,


Test patch:
diff --git a/tests/web/mock_web_api_server.py b/tests/web/mock_web_api_server.py
index 4700930..94968f7 100644
--- a/tests/web/mock_web_api_server.py
+++ b/tests/web/mock_web_api_server.py
@@ -20,6 +20,8 @@ class MockHandler(SimpleHTTPRequestHandler):
 
     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'
 
+    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>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(user_agent) \
@@ -102,11 +104,23 @@ class MockHandler(SimpleHTTPRequestHandler):
 
                 if pattern == "html_response":
                     self.send_response(404)
-                    self.set_common_headers()
+                    self.send_header("content-type", "text/html;charset=utf-8")
+                    self.send_header("connection", "close")
+                    self.end_headers()
                     self.wfile.write(self.html_response_body.encode("utf-8"))
                     self.wfile.close()
                     return
 
+                if pattern == "error_html_response":
+                    self.send_response(503)
+                    # 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
+
                 if pattern.startswith("user-agent"):
                     elements = pattern.split(" ")
                     prefix, suffix = elements[1], elements[-1]
diff --git a/tests/web/test_async_web_client.py b/tests/web/test_async_web_client.py
index c3dd282..91fa65f 100644
--- a/tests/web/test_async_web_client.py
+++ b/tests/web/test_async_web_client.py
@@ -127,8 +127,11 @@ class TestAsyncWebClient(unittest.TestCase):
             await self.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)
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
+            )
 
     @async_test
     async def test_user_agent_customization_issue_769_async(self):
diff --git a/tests/web/test_web_client.py b/tests/web/test_web_client.py
index 2406015..8862296 100644
--- a/tests/web/test_web_client.py
+++ b/tests/web/test_web_client.py
@@ -262,8 +262,11 @@ class TestWebClient(unittest.TestCase):
             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)
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
+            )
 
     def test_user_agent_customization_issue_769(self):
         client = WebClient(
diff --git a/tests/web/test_web_client_issue_829.py b/tests/web/test_web_client_issue_829.py
new file mode 100644
index 0000000..4f70866
--- /dev/null
+++ b/tests/web/test_web_client_issue_829.py
@@ -0,0 +1,45 @@
+import unittest
+
+import slack.errors as err
+from slack import WebClient
+from tests.helpers import async_test
+from tests.web.mock_web_api_server import (
+    setup_mock_web_api_server,
+    cleanup_mock_web_api_server,
+)
+
+
+class TestWebClient_Issue_829(unittest.TestCase):
+    def setUp(self):
+        setup_mock_web_api_server(self)
+        self.client = WebClient(token="xoxp-1234", base_url="http://localhost:8888", )
+        self.async_client = WebClient(
+            token="xoxp-1234", run_async=True, base_url="http://localhost:8888",
+        )
+
+    def tearDown(self):
+        cleanup_mock_web_api_server(self)
+
+    def test_html_response_body_issue_829(self):
+        client = WebClient(base_url="http://localhost:8888")
+        try:
+            client.users_list(token="xoxb-error_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: "),
+                e,
+            )
+
+    @async_test
+    async def test_html_response_body_issue_829_async(self):
+        client = WebClient(base_url="http://localhost:8888", run_async=True)
+        try:
+            await client.users_list(token="xoxb-error_html_response")
+            self.fail("SlackApiError expected here")
+        except err.SlackApiError as e:
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
+            )
diff --git a/tests/webhook/mock_web_api_server.py b/tests/webhook/mock_web_api_server.py
index 3284a04..85b44ed 100644
--- a/tests/webhook/mock_web_api_server.py
+++ b/tests/webhook/mock_web_api_server.py
@@ -16,6 +16,8 @@ class MockHandler(SimpleHTTPRequestHandler):
     pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE)
     pattern_for_package_identifier = re.compile("slackclient/(\\S+)")
 
+    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>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(user_agent) \
@@ -51,8 +53,10 @@ class MockHandler(SimpleHTTPRequestHandler):
 
             if self.path == "/error":
                 self.send_response(HTTPStatus.INTERNAL_SERVER_ERROR)
-                self.set_common_headers()
-                self.wfile.write("error".encode("utf-8"))
+                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
 
diff --git a/tests/webhook/test_webhook.py b/tests/webhook/test_webhook.py
index 2f9e3bd..5830c45 100644
--- a/tests/webhook/test_webhook.py
+++ b/tests/webhook/test_webhook.py
@@ -166,7 +166,7 @@ class TestWebhook(unittest.TestCase):
         client = WebhookClient(url="http://localhost:8888/error")
         resp: WebhookResponse = client.send_dict({"text": "hello!"})
         self.assertEqual(500, resp.status_code)
-        self.assertEqual("error", resp.body)
+        self.assertTrue(resp.body.startswith("<!DOCTYPE html>"))
 
     def test_proxy_issue_714(self):
         client = WebhookClient(url="http://localhost:8888", proxy="http://invalid-host:9999")