RSC13 - Parameters for connection retry, similar to Ruby lib

ably/http/http.py

@@ -2,6 +2,7 @@
 
 import functools
 import logging
+import time
 
 from six.moves import range
 from six.moves.urllib.parse import urljoin
@@ -124,6 +125,13 @@ def links(self):
 
 
 class Http(object):
+    CONNECTION_RETRY = {
+        'single_request_connect_timeout': 4,
+        'single_request_read_timeout': 15,
+        'max_retry_attempts': 3,
+        'cumulative_timeout': 10,
+    }
+
     def __init__(self, ably, options):
         options = options or {}
         self.__ably = ably
@@ -148,21 +156,35 @@ def make_request(self, method, url, headers=None, body=None, skip_auth=False, ti
         if not skip_auth:
             headers.update(self.auth._get_auth_headers())
 
-        request = requests.Request(method, url, data=body, headers=headers)
-        prepped = self.__session.prepare_request(request)
-
-        # log.debug("Method: %s" % method)
-        # log.debug("Url: %s" % url)
-        # log.debug("Headers: %s" % headers)
-        # log.debug("Body: %s" % body)
-        # log.debug("Prepped: %s" % prepped)
-
-        # TODO add timeouts from options here
-        response = self.__session.send(prepped)
-
-        AblyException.raise_for_response(response)
-
-        return Response(response)
+        single_request_connect_timeout = self.CONNECTION_RETRY['single_request_connect_timeout']
+        single_request_read_timeout = self.CONNECTION_RETRY['single_request_read_timeout']
+        max_retry_attempts = self.CONNECTION_RETRY['max_retry_attempts']
+        cumulative_timeout = self.CONNECTION_RETRY['cumulative_timeout']
+        requested_at = time.time()
+        for retry_count in range(max_retry_attempts):
+            try:
+                request = requests.Request(method, url, data=body, headers=headers)
+                prepped = self.__session.prepare_request(request)
+                response = self.__session.send(
+                    prepped,
+                    timeout=(single_request_connect_timeout,
+                             single_request_read_timeout))
+
+                AblyException.raise_for_response(response)
+                return Response(response)
+            except Exception as e:
+                # Need to catch `Exception`, see:
+                # https://github.com/kennethreitz/requests/issues/1236#issuecomment-133312626
+
+                # if not server error, throw exception up
+                if isinstance(e, AblyException) and not e.is_server_error:
+                    raise e
+
+                # if last try or cumulative timeout is done, throw exception up
+                time_passed = time.time() - requested_at
+                if retry_count == max_retry_attempts - 1 or \
+                   time_passed > cumulative_timeout:
+                    raise e
 
     def request(self, request):
         return self.make_request(request.method, request.url, headers=request.headers, body=request.body)

ably/util/exceptions.py

@@ -19,6 +19,10 @@ def __init__(self, reason, status_code, code):
     def __unicode__(self):
         return six.u('%s %s %s') % (self.code, self.status_code, self.reason)
 
+    @property
+    def is_server_error(self):
+        return self.status_code == 500
+
     @staticmethod
     def raise_for_response(response):
         if response.status_code >= 200 and response.status_code < 300:

test/ably/resthttp_test.py

@@ -0,0 +1,53 @@
+from __future__ import absolute_import
+
+import unittest
+import time
+
+import mock
+import requests
+
+from ably import AblyRest
+
+
+class TestRestHttp(unittest.TestCase):
+    def test_max_retry_attempts_and_timeouts(self):
+        ably = AblyRest(token="foo")
+        self.assertIn('single_request_connect_timeout', ably.http.CONNECTION_RETRY)
+        self.assertIn('single_request_read_timeout', ably.http.CONNECTION_RETRY)
+        self.assertIn('max_retry_attempts', ably.http.CONNECTION_RETRY)
+
+        with mock.patch('requests.sessions.Session.send',
+                        side_effect=requests.exceptions.RequestException) as send_mock:
+            try:
+                ably.http.make_request('GET', '/', skip_auth=True)
+            except requests.exceptions.RequestException:
+                pass
+
+            self.assertEqual(
+                send_mock.call_count,
+                ably.http.CONNECTION_RETRY['max_retry_attempts'])
+            self.assertEqual(
+                send_mock.call_args,
+                mock.call(mock.ANY, timeout=(ably.http.CONNECTION_RETRY['single_request_connect_timeout'],
+                                             ably.http.CONNECTION_RETRY['single_request_read_timeout'])))
+
+    def test_cumulative_timeout(self):
+        ably = AblyRest(token="foo")
+        self.assertIn('cumulative_timeout', ably.http.CONNECTION_RETRY)
+
+        cumulative_timeout_original_value = ably.http.CONNECTION_RETRY['cumulative_timeout']
+        ably.http.CONNECTION_RETRY['cumulative_timeout'] = 0.5
+
+        def sleep_and_raise(*args, **kwargs):
+            time.sleep(0.51)
+            raise requests.exceptions.RequestException
+        with mock.patch('requests.sessions.Session.send',
+                        side_effect=sleep_and_raise) as send_mock:
+            try:
+                ably.http.make_request('GET', '/', skip_auth=True)
+            except requests.exceptions.RequestException:
+                pass
+
+            self.assertEqual(send_mock.call_count, 1)
+
+        ably.http.CONNECTION_RETRY['cumulative_timeout'] = cumulative_timeout_original_value