pubsub: breaking changes to exceptions

UPDATING.md

@@ -124,6 +124,104 @@ await pubsub_realtime_client.channels.get('my-channel').detach()
 pubsub_realtime_client.channels.release('my-channel')
 ```
 
+### Some errors had their `code` and `status_code` the wrong way round
+
+Ten errors the library raised itself carried the Ably error code in `status_code` and the
+HTTP status in `code`. They now carry them the right way round:
+
+| Error | 3.x `status_code`, `code` | 4.0.0 `status_code`, `code` |
+|---|---|---|
+| Detaching a `FAILED` realtime channel | 90001, 400 | 400, 90001 |
+| A detach superseded by an attach | 90000, 409 | 409, 90000 |
+| A queued message failed with no reason given | 80000, 500 | 500, 80000 |
+| A NACK with no error | 50001, 500 | 500, 50001 |
+| A vcdiff delta with no decoder configured | 40019, 40019 | 400, 40019 |
+| A vcdiff delta that cannot be decoded | 40018, 40018 | 400, 40018 |
+| `ChannelOptions` given params or options that are not a dict | 40000, 400 | 400, 40000 |
+
+If you branch on `code` or `status_code` for any of these, compare against the 4.0.0 values.
+
+### Network errors and timeouts are raised as `AblyException`
+
+In 3.x a request that got no response, because the host could not be reached or did not
+answer in time, raised the `httpx` exception itself from most methods, such as `request`,
+`publish`, `history` and `auth.authorize`. In 4.0.0 every method raises an `AblyException`:
+code 50003 and status 504 for a timeout, and code 80000 and status 500 for any other
+network error. The `httpx` exception is its `cause`. A timeout or network error reaching
+an `auth_url` is an `AblyException` with code 40170 and status 401, as for any other
+`auth_url` failure.
+
+Example 3.x code:
+```python
+try:
+    await rest.channels.get('my-channel').publish('event', 'data')
+except httpx.TimeoutException:
+    ...
+```
+
+Example 4.0.0 code:
+```python
+try:
+    await pubsub_http_client.channels.get('my-channel').publish('event', 'data')
+except AblyException as e:
+    if e.code == 50003:
+        ...
+```
+
+### An `AblyException` raised by an `auth_callback` is no longer rewritten
+
+In 3.x any exception an `auth_callback` raised reached the caller as an `AblyException` with
+code 40170 and status 401, the original only in its `cause`. In 4.0.0 an `AblyException` the
+callback raises reaches the caller unchanged; any other exception is still wrapped as 40170.
+On a realtime client, a callback error with status 403 now fails the connection, with an
+80019 whose status is 403 and whose `cause` is the callback's error, whether it happens while
+connecting, on a reauth or in an explicit `authorize()` call. 3.x went `DISCONNECTED` and
+retried. Every 80019 the connection reports now carries the auth provider's error as its
+`cause`.
+
+If you check for 40170 after `request_token` or `authorize` to find a failure in your own
+callback, check the code your callback raised instead. Raise a status 403 only for a client
+that should stop connecting:
+
+Example 4.0.0 code:
+```python
+async def auth_callback(token_params):
+    if not await user_may_connect():
+        raise AblyException('Access revoked', 403, 40300)
+    return await fetch_token(token_params)
+```
+
+### Invalid arguments raise `AblyException`
+
+The factories raised a bare `ValueError` in 3.x when given no means of authenticating, a
+`key` together with `key_name` or `key_secret`, a `token_details` that is not a
+`TokenDetails`, or `use_token_auth=False` without a key. In 4.0.0 each raises an
+`AblyException` with code 40106 and status 401. Combining `endpoint` with `environment`,
+`rest_host` or `realtime_host` still raises an `AblyException` with code 40106 and status
+400. `push.admin.publish` raised `TypeError` for a `recipient` or `data` that is not a dict
+and `ValueError` for an empty one, and now raises an `AblyException` with code 40000 and
+status 400 for all four.
+
+These exceptions are also a `ValueError`, so `except ValueError` still catches them, but
+`except TypeError` no longer catches the `push.admin.publish` ones:
+
+Example 3.x code:
+```python
+try:
+    await rest.push.admin.publish(recipient, data)
+except TypeError:
+    ...
+```
+
+Example 4.0.0 code:
+```python
+try:
+    await pubsub_http_client.push.admin.publish(recipient, data)
+except AblyException as e:
+    if e.code == 40000:
+        ...
+```
+
 ## Version 2.x to 3.0.0
 
 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include:

ably/pubsub/http/auth.py

@@ -6,6 +6,8 @@
 from datetime import timedelta
 from typing import TYPE_CHECKING
 
+import httpx
+
 from ably.pubsub.types.options import Options
 
 if TYPE_CHECKING:
@@ -16,7 +18,12 @@
 from ably.pubsub.types.tokendetails import TokenDetails
 from ably.pubsub.types.tokenrequest import TokenRequest
 from ably.pubsub.util.clock import select_clock
-from ably.pubsub.util.exceptions import AblyAuthException, AblyException, IncompatibleClientIdException
+from ably.pubsub.util.exceptions import (
+    AblyAuthException,
+    AblyException,
+    IncompatibleClientIdException,
+    InvalidArgumentException,
+)
 from ably.pubsub.util.helper import extract_url_params
 
 __all__ = ["Auth"]
@@ -60,7 +67,7 @@ def __init__(self, ably: DefaultPubSubHttpClient | DefaultPubSubRealtimeClient,
             self.__basic_credentials = basic_key.decode('ascii')
             return
         elif must_not_use_token_auth and not can_use_basic_auth:
-            raise ValueError('If use_token_auth is False you must provide a key')
+            raise InvalidArgumentException('If use_token_auth is False you must provide a key', 401, 40106)
 
         # Using token auth
         self.__auth_mechanism = Auth.Method.TOKEN
@@ -83,8 +90,9 @@ def __init__(self, ably: DefaultPubSubHttpClient | DefaultPubSubRealtimeClient,
         elif options.token_details:
             log.debug("using token auth with supplied token_details")
         else:
-            raise ValueError("Can't authenticate via token, must provide "
-                             "auth_callback, auth_url, key, token or a TokenDetail")
+            # RSC1b
+            raise InvalidArgumentException("Can't authenticate via token, must provide "
+                                           "auth_callback, auth_url, key, token or a TokenDetail", 401, 40106)
 
     async def get_auth_transport_param(self):
         auth_credentials = {}
@@ -151,7 +159,14 @@ def token_details_has_expired(self):
         return expires < timestamp + token_details.TOKEN_EXPIRY_BUFFER
 
     async def authorize(self, token_params: dict | None = None, auth_options=None):
-        return await self.__authorize_when_necessary(token_params, auth_options, force=True)
+        try:
+            return await self.__authorize_when_necessary(token_params, auth_options, force=True)
+        except AblyException as e:
+            # RSA4d, RSA4d1: a 403 from the auth provider fails the connection, even
+            # while the token it holds is still valid
+            if self.ably._is_realtime and e.status_code == 403:
+                self.ably.connection.connection_manager.on_error_from_authorize(e)
+            raise
 
     async def request_token(self, token_params: dict | None = None,
                             # auth_options
@@ -184,6 +199,9 @@ async def request_token(self, token_params: dict | None = None,
             log.debug("using token auth with authCallback")
             try:
                 token_request = await auth_callback(token_params)
+            except AblyException:
+                # RSA4d, RSA4e: an error the callback gives as an AblyException is kept as it is
+                raise
             except Exception as e:
                 raise AblyException("auth_callback raised an exception", 401, 40170, cause=e) from e
         elif auth_url:
@@ -212,7 +230,7 @@ async def request_token(self, token_params: dict | None = None,
                 raise AblyAuthException(msg, 401, 40170, cause=e) from e
         elif isinstance(token_request, str):
             if len(token_request) == 0:
-                raise AblyAuthException("Token string is empty", 401, 4017)
+                raise AblyAuthException("Token string is empty", 401, 40170)
             return TokenDetails(token=token_request)
         elif token_request is None:
             raise AblyAuthException("Token string was None", 401, 40170)
@@ -410,8 +428,12 @@ async def token_request_from_auth_url(self, method: str, url: str, token_params,
         url = clean_url
 
         from ably.pubsub.request.http import Response
-        resp = await self.ably.http.request_external(
-            method=method, url=url, headers=headers, params=params, body=body)
+        try:
+            resp = await self.ably.http.request_external(
+                method=method, url=url, headers=headers, params=params, body=body)
+        except httpx.RequestError as e:
+            # RSA4e: a timeout or network error reaching the auth_url is a 40170
+            raise AblyAuthException("auth_url request failed", 401, 40170, cause=e) from e
         response = Response(resp)
 
         AblyException.raise_for_response(response)

ably/pubsub/http/http.py

@@ -22,7 +22,7 @@
 from ably.pubsub.types.stats import stats_response_processor
 from ably.pubsub.types.tokendetails import TokenDetails
 from ably.pubsub.util.construction import reject_direct_construction
-from ably.pubsub.util.exceptions import AblyException, catch_all
+from ably.pubsub.util.exceptions import AblyException, InvalidArgumentException, catch_all
 
 log = logging.getLogger(__name__)
 
@@ -67,20 +67,22 @@ def __init__(self, key: Optional[str] = None, token: Optional[str] = None,
         reject_direct_construction(type(self), 'ably.pubsub.server.create_http_client')
 
         if key is not None and ('key_name' in kwargs or 'key_secret' in kwargs):
-            raise ValueError("key and key_name or key_secret are mutually exclusive. "
-                             "Provider either a key or key_name & key_secret")
+            raise InvalidArgumentException("key and key_name or key_secret are mutually exclusive. "
+                                           "Provider either a key or key_name & key_secret", 401, 40106)
         if key is not None:
             options = Options(key=key, **kwargs)
         elif token is not None:
             options = Options(auth_token=token, **kwargs)
         elif token_details is not None:
             if not isinstance(token_details, TokenDetails):
-                raise ValueError("token_details must be an instance of TokenDetails")
+                raise InvalidArgumentException("token_details must be an instance of TokenDetails", 401, 40106)
             options = Options(token_details=token_details, **kwargs)
         elif not ('auth_callback' in kwargs or 'auth_url' in kwargs or
                   # and don't have both key_name and key_secret
                   ('key_name' in kwargs and 'key_secret' in kwargs)):
-            raise ValueError("key is missing. Either an API key, token, or token auth method must be provided")
+            # RSC1b
+            raise InvalidArgumentException(
+                "key is missing. Either an API key, token, or token auth method must be provided", 401, 40106)
         else:
             options = Options(**kwargs)
 

ably/pubsub/http/push.py

@@ -7,6 +7,7 @@
     channels_response_processor,
 )
 from ably.pubsub.types.device import DeviceDetails, device_details_response_processor
+from ably.pubsub.util.exceptions import InvalidArgumentException
 
 
 class Push:
@@ -47,16 +48,16 @@ async def publish(self, recipient: dict, data: dict, timeout: Optional[float] =
         - `data`: the data of the notification
         """
         if not isinstance(recipient, dict):
-            raise TypeError(f'Unexpected {type(recipient)} recipient, expected a dict')
+            raise InvalidArgumentException(f'Unexpected {type(recipient)} recipient, expected a dict', 400, 40000)
 
         if not isinstance(data, dict):
-            raise TypeError(f'Unexpected {type(data)} data, expected a dict')
+            raise InvalidArgumentException(f'Unexpected {type(data)} data, expected a dict', 400, 40000)
 
         if not recipient:
-            raise ValueError('recipient is empty')
+            raise InvalidArgumentException('recipient is empty', 400, 40000)
 
         if not data:
-            raise ValueError('data is empty')
+            raise InvalidArgumentException('data is empty', 400, 40000)
 
         body = data.copy()
         body.update({'recipient': recipient})

ably/pubsub/realtime/channel.py

@@ -244,7 +244,7 @@ async def detach(self) -> None:
             self._notify_state(ChannelState.DETACHED)
             return
         elif self.state == ChannelState.FAILED:
-            raise AblyException("Unable to detach; channel state = failed", 90001, 400)
+            raise AblyException("Unable to detach; channel state = failed", 400, 90001)
         else:
             self._request_state(ChannelState.DETACHING)
 
@@ -258,7 +258,7 @@ async def detach(self) -> None:
         if new_state == ChannelState.DETACHED:
             return
         elif new_state == ChannelState.ATTACHING:
-            raise AblyException("Detach request superseded by a subsequent attach request", 90000, 409)
+            raise AblyException("Detach request superseded by a subsequent attach request", 409, 90000)
         else:
             raise state_change.reason
 

ably/pubsub/realtime/connectionmanager.py

@@ -343,7 +343,7 @@ def fail_queued_messages(self, err) -> None:
             f"ConnectionManager.fail_queued_messages(): discarding {len(self.queued_messages)} messages;" +
             f" reason = {err}"
         )
-        error = err or AblyException("Connection failed", 80000, 500)
+        error = err or AblyException("Connection failed", 500, 80000)
         while len(self.queued_messages) > 0:
             pending_msg = self.queued_messages.pop()
             log.exception(
@@ -487,11 +487,11 @@ def on_error_from_authorize(self, exception: AblyException) -> None:
         elif exception.status_code == 403:
             msg = 'Client configured authentication provider returned 403; failing the connection'
             log.error(f'ConnectionManager.on_error_from_authorize(): {msg}')
-            self.notify_state(ConnectionState.FAILED, AblyException(msg, 403, 80019))
+            self.notify_state(ConnectionState.FAILED, AblyException(msg, 403, 80019, cause=exception))
         else:
             msg = 'Client configured authentication provider request failed'
             log.warning(f'ConnectionManager.on_error_from_authorize: {msg}')
-            self.notify_state(self.__fail_state, AblyException(msg, 401, 80019))
+            self.notify_state(self.__fail_state, AblyException(msg, 401, 80019, cause=exception))
 
     async def on_closed(self) -> None:
         if self.transport:
@@ -538,7 +538,7 @@ def on_nack(self, serial: int, count: int, err: AblyException | None) -> None:
             err: Error information from the server
         """
         if not err:
-            err = AblyException('Unable to send message; channel not responding', 50001, 500)
+            err = AblyException('Unable to send message; channel not responding', 500, 50001)
 
         log.error(f'ConnectionManager.on_nack(): serial={serial}, count={count}, err={err}')
         self.pending_message_queue.complete_messages(serial, count, None, err)

ably/pubsub/request/http.py

@@ -43,6 +43,17 @@ async def wrapper(rest, *args, **kwargs):
     return wrapper
 
 
+def wrap_request_error(host: str, e: httpx.RequestError) -> AblyException:
+    """Wraps the error httpx raised for a request that got no response (RSC19e).
+
+    A timeout is 50003 and any other failure to complete the request is 80000, with
+    the httpx error kept as the cause.
+    """
+    if isinstance(e, httpx.TimeoutException):
+        return AblyException(f'Request to {host} timed out', 504, 50003, cause=e)
+    return AblyException(f'Request to {host} failed with a network error', 500, 80000, cause=e)
+
+
 class Request:
     def __init__(self, method='GET', url='/', version=None, headers=None, body=None,
                  skip_auth=False, raise_on_error=True):
@@ -219,6 +230,8 @@ def should_stop_retrying(retry_count=retry_count):
                 response = await self.__client.send(request)
             except Exception as e:
                 if should_stop_retrying():
+                    if isinstance(e, httpx.RequestError):
+                        raise wrap_request_error(host, e) from e
                     raise e
             else:
                 # RSC15l4

ably/pubsub/server/__init__.py

@@ -163,11 +163,10 @@ def create_http_client(**kwargs) -> PubSubHttpClient:
 
     Raises
     ------
-    ValueError
-        If `key` is combined with `key_name` or `key_secret`.
     AblyException
-        If no means of authenticating is given, or if `endpoint` is combined with
-        `environment`, `rest_host` or `realtime_host`.
+        With code 40106, if no means of authenticating is given, if `key` is combined
+        with `key_name` or `key_secret`, or if `endpoint` is combined with
+        `environment`, `rest_host` or `realtime_host`. It is also a ValueError.
     """
     with factory_construction():
         return _DefaultPubSubHttpClient(**kwargs)
@@ -243,11 +242,10 @@ def create_realtime_client(**kwargs) -> PubSubRealtimeClient:
 
     Raises
     ------
-    ValueError
-        If `key` is combined with `key_name` or `key_secret`.
     AblyException
-        If no means of authenticating is given, or if `endpoint` is combined with
-        `environment`, `rest_host` or `realtime_host`.
+        With code 40106, if no means of authenticating is given, if `key` is combined
+        with `key_name` or `key_secret`, or if `endpoint` is combined with
+        `environment`, `rest_host` or `realtime_host`. It is also a ValueError.
     """
     with factory_construction():
         return _DefaultPubSubRealtimeClient(**kwargs)

ably/pubsub/server/sync.py

@@ -52,11 +52,10 @@ def create_http_client(**kwargs) -> PubSubHttpClient:
 
     Raises
     ------
-    ValueError
-        If `key` is combined with `key_name` or `key_secret`.
     AblyException
-        If no means of authenticating is given, or if `endpoint` is combined with
-        `environment`, `rest_host` or `realtime_host`.
+        With code 40106, if no means of authenticating is given, if `key` is combined
+        with `key_name` or `key_secret`, or if `endpoint` is combined with
+        `environment`, `rest_host` or `realtime_host`. It is also a ValueError.
     """
     with factory_construction():
         return _DefaultPubSubHttpClientSync(**kwargs)