Merge pull request #716 from seratch/issue-712

Fix #712 by adding timeout arg support in sync WebClient/WebhookClient

Author: seratch

slack/web/base_client.py

@@ -558,7 +558,9 @@ def _perform_urllib_http_request(
                             f"Invalid proxy detected: {self.proxy} must be a str value"
                         )
 
-                resp: HTTPResponse = urlopen(req, context=self.ssl)
+                resp: HTTPResponse = urlopen(
+                    req, context=self.ssl, timeout=self.timeout
+                )
                 charset = resp.headers.get_content_charset()
                 body: str = resp.read().decode(charset)  # read the response body here
                 return {"status": resp.code, "headers": resp.headers, "body": body}

slack/webhook/client.py

@@ -1,6 +1,8 @@
 import json
 import logging
+import re
 from http.client import HTTPResponse
+from ssl import SSLContext
 from typing import Dict, Union, List, Optional
 from urllib.error import HTTPError
 from urllib.request import Request, urlopen
@@ -16,13 +18,24 @@ class WebhookClient:
     logger = logging.getLogger(__name__)
 
     def __init__(
-        self, url: str, default_headers: Dict[str, str] = {},
+        self,
+        url: str,
+        timeout: int = 30,
+        ssl: Optional[SSLContext] = None,
+        proxy: Optional[str] = None,
+        default_headers: Dict[str, str] = {},
     ):
         """API client for Incoming Webhooks and response_url
         :param url: a complete URL to send data (e.g., https://hooks.slack.com/XXX)
+        :param timeout: request timeout (in seconds)
+        :param ssl: ssl.SSLContext to use for requests
+        :param proxy: proxy URL (e.g., localhost:9000, http://localhost:9000)
         :param default_headers: request headers to add to all requests
         """
         self.url = url
+        self.timeout = timeout
+        self.ssl = ssl
+        self.proxy = proxy
         self.default_headers = default_headers
 
     def send(
@@ -65,11 +78,11 @@ def send_dict(
         body = convert_bool_to_0_or_1(body)
         self._parse_web_class_objects(body)
         return self._perform_http_request(
-            url=self.url, body=body, headers=self._build_request_headers(headers),
+            body=body, headers=self._build_request_headers(headers)
         )
 
     def _perform_http_request(
-        self, *, url: str, body: Dict[str, any], headers: Dict[str, str]
+        self, *, body: Dict[str, any], headers: Dict[str, str]
     ) -> WebhookResponse:
         """Performs an HTTP request and parses the response.
         :param url: a complete URL to send data (e.g., https://hooks.slack.com/XXX)
@@ -85,19 +98,24 @@ def _perform_http_request(
                 f"Sending a request - url: {self.url}, body: {body}, headers: {headers}"
             )
         try:
+            url = self.url
             # for security
             if url.lower().startswith("http"):
                 req = Request(
                     method="POST", url=url, data=body.encode("utf-8"), headers=headers
                 )
+                if self.proxy is not None:
+                    host = re.sub("^https?://", "", self.proxy)
+                    req.set_proxy(host, "http")
+                    req.set_proxy(host, "https")
             else:
                 raise SlackRequestError(f"Invalid URL detected: {url}")
 
-            resp: HTTPResponse = urlopen(req)
+            resp: HTTPResponse = urlopen(req, context=self.ssl, timeout=self.timeout)
             charset: str = resp.headers.get_content_charset() or "utf-8"
             response_body: str = resp.read().decode(charset)
             resp = WebhookResponse(
-                url=self.url,
+                url=url,
                 status_code=resp.status,
                 body=response_body,
                 headers=resp.headers,
@@ -109,7 +127,7 @@ def _perform_http_request(
             charset: str = e.headers.get_content_charset() or "utf-8"
             response_body: str = resp.read().decode(charset)
             resp = WebhookResponse(
-                url=self.url, status_code=e.code, body=response_body, headers=e.headers,
+                url=url, status_code=e.code, body=response_body, headers=e.headers,
             )
             if e.code == 429:
                 # for backward-compatibility with WebClient (v.2.5.0 or older)

tests/web/mock_web_api_server.py

@@ -2,6 +2,7 @@
 import logging
 import re
 import threading
+import time
 from http import HTTPStatus
 from http.server import HTTPServer, SimpleHTTPRequestHandler
 from typing import Type
@@ -90,6 +91,13 @@ def _handle(self):
                     self.wfile.close()
                     return
 
+                if pattern == "timeout":
+                    time.sleep(2)
+                    self.send_response(200)
+                    self.wfile.write("""{"ok":true}""".encode("utf-8"))
+                    self.wfile.close()
+                    return
+
                 if request_body and "cursor" in request_body:
                     page = request_body["cursor"]
                     pattern = f"{pattern}_{page}"

tests/web/test_web_client.py

@@ -1,5 +1,6 @@
 import asyncio
 import re
+import socket
 import unittest
 
 import slack.errors as err
@@ -211,3 +212,14 @@ async def test_token_param_async(self):
         self.assertIsNone(resp["error"])
         with self.assertRaises(err.SlackApiError):
             await client.users_list()
+
+    def test_timeout_issue_712(self):
+        client = WebClient(base_url="http://localhost:8888", timeout=1)
+        with self.assertRaises(socket.timeout):
+            client.users_list(token="xoxb-timeout")
+
+    @async_test
+    async def test_timeout_issue_712_async(self):
+        client = WebClient(base_url="http://localhost:8888", timeout=1, run_async=True)
+        with self.assertRaises(asyncio.TimeoutError):
+            await client.users_list(token="xoxb-timeout")

tests/webhook/mock_web_api_server.py

@@ -1,7 +1,7 @@
-import json
 import logging
 import re
 import threading
+import time
 from http import HTTPStatus
 from http.server import HTTPServer, SimpleHTTPRequestHandler
 from typing import Type
@@ -28,6 +28,9 @@ def set_common_headers(self):
 
     def do_POST(self):
         try:
+            if self.path == "/timeout":
+                time.sleep(2)
+
             body = "ok"
 
             self.send_response(HTTPStatus.OK)

tests/webhook/test_webhook.py

@@ -1,4 +1,6 @@
 import unittest
+import socket
+import urllib
 
 from slack.web.classes.attachments import Attachment, AttachmentField
 from slack.web.classes.blocks import SectionBlock, ImageBlock
@@ -154,3 +156,13 @@ def test_send_dict(self):
         resp: WebhookResponse = client.send_dict({"text": "hello!"})
         self.assertEqual(200, resp.status_code)
         self.assertEqual("ok", resp.body)
+
+    def test_timeout_issue_712(self):
+        client = WebhookClient(url="http://localhost:8888/timeout", timeout=1)
+        with self.assertRaises(socket.timeout):
+            client.send_dict({"text": "hello!"})
+
+    def test_proxy_issue_714(self):
+        client = WebhookClient(url="http://localhost:8888", proxy="http://invalid-host:9999")
+        with self.assertRaises(urllib.error.URLError):
+            client.send_dict({"text": "hello!"})