Set timeout on HTTP requests

James E. Blair · James E. Blair · commit 324713779a1d · 2018-11-29T09:26:05.000-08:00
The requests documentation[0] recommends always setting a timeout for requests in order to avoid indefinite hangs. This can happen if the remote side is unresponsive, or if a network error causes a FIN or RST packet never to arrive. This sets a default connect timeout of 4 seconds and a read timeout of 1 second for all HTTP requests. The default can be overridden in the GitHubSession constructor. The GitHubSession class is also added to the documentation on the GitHub Object page since it is closely associated with it. [0] http://docs.python-requests.org/en/master/user/quickstart/#timeouts
diff --git a/AUTHORS.rst b/AUTHORS.rst
@@ -174,3 +174,5 @@ Contributors
 - Raphael Vogel (@RaphaelVogel)
 
 - Andreas Burger (@AndreasBurger)
+
+- James E. Blair (@jeblair)
diff --git a/docs/source/api-reference/github.rst b/docs/source/api-reference/github.rst
@@ -34,3 +34,9 @@ GitHubStatus Object
 
 .. autoclass:: github3.github.GitHubStatus
     :members:
+
+
+GitHubSession Object
+====================
+
+.. autoclass:: github3.session.GitHubSession
diff --git a/src/github3/session.py b/src/github3/session.py
@@ -64,17 +64,40 @@ def __call__(self, request):
 
 
 class GitHubSession(requests.Session):
-    """Our slightly specialized Session object."""
+    """Our slightly specialized Session object.
+
+    Normally this is created automatically by
+    :class:`~github3.github.GitHub`.  To use alternate values for
+    network timeouts, this class can be instantiated directly and
+    passed to the GitHub object.  For example:
+
+    .. code-block:: python
+
+       gh = github.GitHub(session=session.GitHubSession(
+           default_connect_timeout=T, default_read_timeout=N))
+
+    :param default_connect_timeout:
+       the number of seconds to wait when establishing a connection to
+       GitHub
+    :type default_connect_timeout:
+       float
+    :param default_read_timeout:
+       the number of seconds to wait for a response from GitHub
+    :type default_read_timeout:
+       float
+    """
 
     auth = None
     __attrs__ = requests.Session.__attrs__ + [
         "base_url",
         "two_factor_auth_cb",
     ]
 
-    def __init__(self):
+    def __init__(self, default_connect_timeout=4, default_read_timeout=1):
         """Slightly modify how we initialize our session."""
         super(GitHubSession, self).__init__()
+        self.default_connect_timeout = default_connect_timeout
+        self.default_read_timeout = default_read_timeout
         self.headers.update(
             {
                 # Only accept JSON responses
@@ -91,6 +114,11 @@ def __init__(self):
         self.two_factor_auth_cb = None
         self.request_counter = 0
 
+    @property
+    def timeout(self):
+        """Return the timeout tuple as expected by Requests"""
+        return (self.default_connect_timeout, self.default_read_timeout)
+
     def basic_auth(self, username, password):
         """Set the Basic Auth credentials on this Session.
 
@@ -137,6 +165,7 @@ def oauth2_auth(self, client_id, client_secret):
 
     def request(self, *args, **kwargs):
         """Make a request, count it, and handle 2FA if necessary."""
+        kwargs.setdefault('timeout', self.timeout)
         response = super(GitHubSession, self).request(*args, **kwargs)
         self.request_counter += 1
         if requires_2fa(response) and self.two_factor_auth_cb:
diff --git a/tests/unit/test_github_session.py b/tests/unit/test_github_session.py
@@ -30,6 +30,37 @@ def test_has_default_headers(self):
         assert "User-Agent" in s.headers
         assert s.headers["User-Agent"].startswith("github3.py/")
 
+    @mock.patch.object(requests.Session, 'request')
+    def test_default_timeout(self, request_mock):
+        """Test that default timeout values are used"""
+        response = mock.Mock()
+        response.configure_mock(status_code=200, headers={})
+        request_mock.return_value = response
+        s = self.build_session()
+        r = s.get("http://example.com")
+        assert r is response
+        request_mock.assert_called_once_with(
+            "GET", "http://example.com", allow_redirects=True,
+            timeout=(4, 1)
+        )
+
+    @mock.patch.object(requests.Session, 'request')
+    def test_custom_timeout(self, request_mock):
+        """Test that custom timeout values are used"""
+        response = mock.Mock()
+        response.configure_mock(status_code=200, headers={})
+        request_mock.return_value = response
+        s = session.GitHubSession(
+            default_connect_timeout=300,
+            default_read_timeout=400
+        )
+        r = s.get("http://example.com")
+        assert r is response
+        request_mock.assert_called_once_with(
+            "GET", "http://example.com", allow_redirects=True,
+            timeout=(300, 400)
+        )
+
     def test_build_url(self):
         """Test that GitHubSessions build basic URLs"""
         s = self.build_session()
@@ -117,7 +148,8 @@ def test_request_ignores_responses_that_do_not_require_2fa(
         r = s.get("http://example.com")
         assert r is response
         request_mock.assert_called_once_with(
-            "GET", "http://example.com", allow_redirects=True
+            "GET", "http://example.com", allow_redirects=True,
+            timeout=(4, 1)
         )
 
     @mock.patch.object(requests.Session, "request")
