Repository: ably/ably-python
Issue: Channel.publish sometimes returns None after exhausting retries

When trying to figure out the failure cases for `Channel.publish`, I found that 
if the method receives a series of 500 requests greater than the retry count, 
the method won't raise an error and will instead return `None`.

This means that to check for all errors with `Channel.publish` we need to write:

```python
channel = ably.channels.get(channel)
res = channel.publish(...)
if res is None:
    raise requests.ConnectionError
```

Instead of:

```python
channel = ably.channels.get(channel)
channel.publish(...)
```

I think the method should be updated to raise an exception after 
exhausting all retry attempts.

Something like:

```diff
diff --git a/ably/http/http.py b/ably/http/http.py
index 6e7bbbb..ea8067b 100644
--- a/ably/http/http.py
+++ b/ably/http/http.py
@@ -217,6 +217,7 @@ class Http:
                 except AblyException as e:
                     if not e.is_server_error:
                         raise e
+        raise AblyException(message="", status_code=0, code=0)
 
     def delete(self, url, headers=None, skip_auth=False, timeout=None):
         return self.make_request('DELETE', url, headers=headers,
```



┆Issue is synchronized with this [Jira Bug](https://ably.atlassian.net/browse/SDK-609) by [Unito](https://www.unito.io)


Changed files:
test/ably/resthttp_test.py, ably/http/http.py

diff --git a/ably/http/http.py b/ably/http/http.py
index 6e7bbbb..8cccd47 100644
--- a/ably/http/http.py
+++ b/ably/http/http.py
@@ -200,8 +200,7 @@ class Http:
 
                 # 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:
+                if retry_count == len(hosts) - 1 or time_passed > http_max_retry_duration:
                     raise e
             else:
                 try:
@@ -218,6 +217,11 @@ class Http:
                     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
+
     def delete(self, url, headers=None, skip_auth=False, timeout=None):
         return self.make_request('DELETE', url, headers=headers,
                                  skip_auth=skip_auth, timeout=timeout)
diff --git a/test/ably/resthttp_test.py b/test/ably/resthttp_test.py
index 370a64e..73c5559 100644
--- a/test/ably/resthttp_test.py
+++ b/test/ably/resthttp_test.py
@@ -132,9 +132,7 @@ class TestRestHttp(BaseTestCase):
             ably.http.preferred_port)
 
         def raise_ably_exception(*args, **kwagrs):
-            raise AblyException(message="",
-                                status_code=600,
-                                code=50500)
+            raise AblyException(message="", status_code=600, code=50500)
 
         with mock.patch('requests.Request', wraps=requests.Request) as request_mock:
             with mock.patch('ably.util.exceptions.AblyException.raise_for_response',
@@ -145,6 +143,30 @@ class TestRestHttp(BaseTestCase):
                 assert send_mock.call_count == 1
                 assert request_mock.call_args == mock.call(mock.ANY, default_url, data=mock.ANY, headers=mock.ANY)
 
+    def test_500_errors(self):
+        """
+        Raise error if all the servers reply with a 5xx error.
+        https://github.com/ably/ably-python/issues/160
+        """
+        default_host = Options().get_rest_host()
+        ably = AblyRest(token="foo")
+
+        default_url = "%s://%s:%d/" % (
+            ably.http.preferred_scheme,
+            default_host,
+            ably.http.preferred_port)
+
+        def raise_ably_exception(*args, **kwagrs):
+            raise AblyException(message="", status_code=500, code=50000)
+
+        with mock.patch('requests.Request', wraps=requests.Request) as request_mock:
+            with mock.patch('ably.util.exceptions.AblyException.raise_for_response',
+                            side_effect=raise_ably_exception) as send_mock:
+                with pytest.raises(AblyException):
+                    ably.http.make_request('GET', '/', skip_auth=True)
+
+                assert send_mock.call_count == 3
+
     def test_custom_http_timeouts(self):
         ably = AblyRest(
             token="foo", http_request_timeout=30, http_open_timeout=8,