Revert "[ECO-5305] fix: rest retry logic"

This reverts commit b09264d.

Author: ttypic

ably/http/http.py

@@ -188,11 +188,6 @@ async def make_request(self, method, path, version=None, headers=None, body=None
 
         hosts = self.get_rest_hosts()
         for retry_count, host in enumerate(hosts):
-            def should_stop_retrying():
-                time_passed = time.time() - requested_at
-                # if it's the last try or cumulative timeout is done, we stop retrying
-                return retry_count == len(hosts) - 1 or time_passed > http_max_retry_duration
-
             base_url = "%s://%s:%d" % (self.preferred_scheme,
                                        host,
                                        self.preferred_port)
@@ -209,22 +204,12 @@ def should_stop_retrying():
             try:
                 response = await self.__client.send(request)
             except Exception as e:
-                if should_stop_retrying():
+                # if last try or cumulative timeout is done, throw exception up
+                time_passed = time.time() - requested_at
+                if retry_count == len(hosts) - 1 or time_passed > http_max_retry_duration:
                     raise e
             else:
-                # RSC15l4
-                cloud_front_error = (response.headers.get('Server', '').lower() == 'cloudfront'
-                                     and response.status_code >= 400)
-                # RSC15l3
-                retryable_server_error = response.status_code >= 500 and response.status_code <= 504
-                # Resending requests that have failed for other failure conditions will not fix the problem
-                # and will simply increase the load on other datacenters unnecessarily
-                response_is_retryable = cloud_front_error or retryable_server_error
-
                 try:
-                    if response_is_retryable and not should_stop_retrying():
-                        continue
-
                     if raise_on_error:
                         AblyException.raise_for_response(response)
 
@@ -235,7 +220,12 @@ def should_stop_retrying():
 
                     return Response(response)
                 except AblyException as e:
-                    if should_stop_retrying() or not response_is_retryable:
+                    if 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 == len(hosts) - 1 or time_passed > http_max_retry_duration:
                         raise e
 
     async def delete(self, url, headers=None, skip_auth=False, timeout=None):

test/ably/rest/resthttp_test.py

@@ -151,7 +151,6 @@ async def test_no_retry_if_not_500_to_599_http_code(self):
 
         await ably.close()
 
-    @respx.mock
     async def test_500_errors(self):
         """
         Raise error if all the servers reply with a 5xx error.
@@ -160,13 +159,16 @@ async def test_500_errors(self):
 
         ably = AblyRest(token="foo")
 
-        mock_request = respx.route().mock(return_value=httpx.Response(500, text="Internal Server Error"))
+        def raise_ably_exception(*args, **kwargs):
+            raise AblyException(message="", status_code=500, code=50000)
 
-        with pytest.raises(AblyException):
-            await ably.http.make_request('GET', '/', skip_auth=True)
-
-        assert mock_request.call_count == 3
+        with mock.patch('httpx.Request', wraps=httpx.Request):
+            with mock.patch('ably.util.exceptions.AblyException.raise_for_response',
+                            side_effect=raise_ably_exception) as send_mock:
+                with pytest.raises(AblyException):
+                    await ably.http.make_request('GET', '/', skip_auth=True)
 
+                assert send_mock.call_count == 3
         await ably.close()
 
     def test_custom_http_timeouts(self):

test/ably/rest/restrequest_test.py

@@ -126,101 +126,6 @@ async def test_timeout(self):
             await ably.request('GET', '/time', version=Defaults.protocol_version)
         await ably.close()
 
-    # RSC15l3
-    @dont_vary_protocol
-    async def test_503_status_fallback(self):
-        default_endpoint = 'https://sandbox-rest.ably.io/time'
-        fallback_host = 'sandbox-a-fallback.ably-realtime.com'
-        fallback_endpoint = f'https://{fallback_host}/time'
-        ably = await TestApp.get_ably_rest(fallback_hosts=[fallback_host])
-        with respx.mock:
-            default_route = respx.get(default_endpoint)
-            fallback_route = respx.get(fallback_endpoint)
-            headers = {
-                "Content-Type": "application/json"
-            }
-            default_route.return_value = httpx.Response(503, headers=headers)
-            fallback_route.return_value = httpx.Response(200, headers=headers, text='[123]')
-            result = await ably.request('GET', '/time', version=Defaults.protocol_version)
-            assert default_route.called
-            assert result.status_code == 200
-            assert result.items[0] == 123
-        await ably.close()
-
-    # RSC15l2
-    @dont_vary_protocol
-    async def test_httpx_timeout_fallback(self):
-        default_endpoint = 'https://sandbox-rest.ably.io/time'
-        fallback_host = 'sandbox-a-fallback.ably-realtime.com'
-        fallback_endpoint = f'https://{fallback_host}/time'
-        ably = await TestApp.get_ably_rest(fallback_hosts=[fallback_host])
-        with respx.mock:
-            default_route = respx.get(default_endpoint)
-            fallback_route = respx.get(fallback_endpoint)
-            headers = {
-                "Content-Type": "application/json"
-            }
-            default_route.side_effect = httpx.ReadTimeout
-            fallback_route.return_value = httpx.Response(200, headers=headers, text='[123]')
-            result = await ably.request('GET', '/time', version=Defaults.protocol_version)
-            assert default_route.called
-            assert result.status_code == 200
-            assert result.items[0] == 123
-        await ably.close()
-
-    # RSC15l3
-    @dont_vary_protocol
-    async def test_503_status_fallback_on_publish(self):
-        default_endpoint = 'https://sandbox-rest.ably.io/channels/test/messages'
-        fallback_host = 'sandbox-a-fallback.ably-realtime.com'
-        fallback_endpoint = f'https://{fallback_host}/channels/test/messages'
-
-        fallback_response_text = (
-            '{"id": "unique_id:0", "channel": "test", "name": "test", "data": "data", '
-            '"clientId": null, "connectionId": "connection_id", "timestamp": 1696944145000, '
-            '"encoding": null}'
-        )
-
-        ably = await TestApp.get_ably_rest(fallback_hosts=[fallback_host])
-        with respx.mock:
-            default_route = respx.post(default_endpoint)
-            fallback_route = respx.post(fallback_endpoint)
-            headers = {
-                "Content-Type": "application/json"
-            }
-            default_route.return_value = httpx.Response(503, headers=headers)
-            fallback_route.return_value = httpx.Response(
-                200,
-                headers=headers,
-                text=fallback_response_text,
-            )
-            message_response = await ably.channels['test'].publish('test', 'data')
-            assert default_route.called
-            assert message_response.to_native()['data'] == 'data'
-        await ably.close()
-
-    # RSC15l4
-    @dont_vary_protocol
-    async def test_400_cloudfront_fallback(self):
-        default_endpoint = 'https://sandbox-rest.ably.io/time'
-        fallback_host = 'sandbox-a-fallback.ably-realtime.com'
-        fallback_endpoint = f'https://{fallback_host}/time'
-        ably = await TestApp.get_ably_rest(fallback_hosts=[fallback_host])
-        with respx.mock:
-            default_route = respx.get(default_endpoint)
-            fallback_route = respx.get(fallback_endpoint)
-            headers = {
-                "Server": "CloudFront",
-                "Content-Type": "application/json",
-            }
-            default_route.return_value = httpx.Response(400, headers=headers, text='[456]')
-            fallback_route.return_value = httpx.Response(200, headers=headers, text='[123]')
-            result = await ably.request('GET', '/time', version=Defaults.protocol_version)
-            assert default_route.called
-            assert result.status_code == 200
-            assert result.items[0] == 123
-        await ably.close()
-
     async def test_version(self):
         version = "150"  # chosen arbitrarily
         result = await self.ably.request('GET', '/time', "150")