diff --git a/UPDATING.md b/UPDATING.md index 64f69d56..9865dc14 100644 --- a/UPDATING.md +++ b/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: diff --git a/ably/pubsub/http/auth.py b/ably/pubsub/http/auth.py index aa71d517..f4a2a713 100644 --- a/ably/pubsub/http/auth.py +++ b/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) diff --git a/ably/pubsub/http/http.py b/ably/pubsub/http/http.py index 7d912d0b..ca9486cf 100644 --- a/ably/pubsub/http/http.py +++ b/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) diff --git a/ably/pubsub/http/push.py b/ably/pubsub/http/push.py index afcea029..a127de1f 100644 --- a/ably/pubsub/http/push.py +++ b/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}) diff --git a/ably/pubsub/realtime/channel.py b/ably/pubsub/realtime/channel.py index e484f656..17796cf2 100644 --- a/ably/pubsub/realtime/channel.py +++ b/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 diff --git a/ably/pubsub/realtime/connectionmanager.py b/ably/pubsub/realtime/connectionmanager.py index 407692d2..bd9a7dcd 100644 --- a/ably/pubsub/realtime/connectionmanager.py +++ b/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) diff --git a/ably/pubsub/request/http.py b/ably/pubsub/request/http.py index c664f1ab..cd8b54d4 100644 --- a/ably/pubsub/request/http.py +++ b/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 diff --git a/ably/pubsub/server/__init__.py b/ably/pubsub/server/__init__.py index 15c20641..10526d5f 100644 --- a/ably/pubsub/server/__init__.py +++ b/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) diff --git a/ably/pubsub/server/sync.py b/ably/pubsub/server/sync.py index 3fcc0eec..56357a6e 100644 --- a/ably/pubsub/server/sync.py +++ b/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) diff --git a/ably/pubsub/types/channeloptions.py b/ably/pubsub/types/channeloptions.py index 9f7a99e1..71226e95 100644 --- a/ably/pubsub/types/channeloptions.py +++ b/ably/pubsub/types/channeloptions.py @@ -29,7 +29,7 @@ def __init__( self.__modes = modes # Validate params if self.__params and not isinstance(self.__params, dict): - raise AblyException("params must be a dictionary", 40000, 400) + raise AblyException("params must be a dictionary", 400, 40000) @property def cipher(self) -> CipherParams | None: @@ -77,7 +77,7 @@ def to_dict(self) -> dict[str, Any]: def from_dict(cls, options_dict: dict[str, Any]) -> ChannelOptions: """Create ChannelOptions from dictionary""" if not isinstance(options_dict, dict): - raise AblyException("options must be a dictionary", 40000, 400) + raise AblyException("options must be a dictionary", 400, 40000) return cls( cipher=options_dict.get('cipher'), diff --git a/ably/pubsub/types/mixins.py b/ably/pubsub/types/mixins.py index 976da3b1..0a352bee 100644 --- a/ably/pubsub/types/mixins.py +++ b/ably/pubsub/types/mixins.py @@ -81,11 +81,11 @@ def decode(data, encoding='', cipher=None, context=None): elif encoding == ENC_VCDIFF: if not context or not context.vcdiff_decoder: log.error('Message cannot be decoded as no VCDiff decoder available') - raise AblyException('VCDiff decoder not available', 40019, 40019) + raise AblyException('VCDiff decoder not available', 400, 40019) if not context.base_payload: log.error('VCDiff decoding requires base payload') - raise AblyException('VCDiff decode failure', 40018, 40018) + raise AblyException('VCDiff decode failure', 400, 40018) try: # Convert base payload to bytes if it's a string @@ -108,7 +108,7 @@ def decode(data, encoding='', cipher=None, context=None): except Exception as e: log.error(f'VCDiff decode failed: {e}') - raise AblyException('VCDiff decode failure', 40018, 40018) from e + raise AblyException('VCDiff decode failure', 400, 40018) from e elif encoding.startswith(f'{CipherData.ENCODING_ID}+'): if not cipher: diff --git a/ably/pubsub/types/options.py b/ably/pubsub/types/options.py index 0a4fc932..6518744d 100644 --- a/ably/pubsub/types/options.py +++ b/ably/pubsub/types/options.py @@ -4,7 +4,7 @@ from ably.pubsub.transport.defaults import Defaults from ably.pubsub.types.authoptions import AuthOptions -from ably.pubsub.util.exceptions import AblyException +from ably.pubsub.util.exceptions import InvalidArgumentException log = logging.getLogger(__name__) @@ -42,7 +42,7 @@ def __init__(self, client_id=None, log_level=0, tls=True, rest_host=None, realti # REC1b1: endpoint is incompatible with deprecated options if endpoint is not None: if environment is not None or rest_host is not None or realtime_host is not None: - raise AblyException( + raise InvalidArgumentException( message='endpoint is incompatible with any of environment, rest_host or realtime_host', status_code=400, code=40106, @@ -67,14 +67,14 @@ def __init__(self, client_id=None, log_level=0, tls=True, rest_host=None, realti suspended_retry_timeout = Defaults.suspended_retry_timeout if environment is not None and rest_host is not None: - raise AblyException( + raise InvalidArgumentException( message='specify rest_host or environment, not both', status_code=400, code=40106, ) if environment is not None and realtime_host is not None: - raise AblyException( + raise InvalidArgumentException( message='specify realtime_host or environment, not both', status_code=400, code=40106, diff --git a/ably/pubsub/util/exceptions.py b/ably/pubsub/util/exceptions.py index 245d4a43..a1572d58 100644 --- a/ably/pubsub/util/exceptions.py +++ b/ably/pubsub/util/exceptions.py @@ -97,6 +97,9 @@ async def wrapper(*args, **kwargs): return await func(*args, **kwargs) except Exception as e: log.exception(e) + if isinstance(e, AblyException): + # Raised as it is, so that its own __cause__ is kept rather than replaced by itself + raise raise AblyException.from_exception(e) from e return wrapper @@ -108,3 +111,11 @@ class AblyAuthException(AblyException): class IncompatibleClientIdException(AblyException): pass + + +class InvalidArgumentException(AblyException, ValueError): + """Arguments the library rejects before making any request (RSC1b, RSH1a). + + Also a ValueError, which is what 3.x raised for them, so `except ValueError` still + catches it. + """ diff --git a/ably/pubsub/vcdiff/defaultvcdiffdecoder.py b/ably/pubsub/vcdiff/defaultvcdiffdecoder.py index 07af3b9e..b5f4981c 100644 --- a/ably/pubsub/vcdiff/defaultvcdiffdecoder.py +++ b/ably/pubsub/vcdiff/defaultvcdiffdecoder.py @@ -75,7 +75,7 @@ def decode(self, delta: bytes, base: bytes) -> bytes: return result except Exception as e: log.error(f"VCDiff decode failed: {e}") - raise AblyException(f"VCDiff decode failure: {e}", 40018, 40018) from e + raise AblyException(f"VCDiff decode failure: {e}", 400, 40018) from e # Export for easy importing diff --git a/test/ably/http/httphttp_test.py b/test/ably/http/httphttp_test.py index 7f0bfa04..91a39146 100644 --- a/test/ably/http/httphttp_test.py +++ b/test/ably/http/httphttp_test.py @@ -34,7 +34,7 @@ async def test_max_retry_attempts_and_timeouts_defaults(self): assert 'http_request_timeout' in ably.http.CONNECTION_RETRY_DEFAULTS with mock.patch('httpx.AsyncClient.send', side_effect=httpx.RequestError('')) as send_mock: - with pytest.raises(httpx.RequestError): + with pytest.raises(AblyException, match="network error"): await ably.http.make_request('GET', '/', version=Defaults.protocol_version, skip_auth=True) assert send_mock.call_count == Defaults.http_max_retry_count @@ -52,7 +52,7 @@ def sleep_and_raise(*args, **kwargs): raise httpx.TimeoutException('timeout') with mock.patch('httpx.AsyncClient.send', side_effect=sleep_and_raise) as send_mock: - with pytest.raises(httpx.TimeoutException): + with pytest.raises(AblyException, match="timed out"): await ably.http.make_request('GET', '/', skip_auth=True) assert send_mock.call_count == 1 @@ -67,7 +67,7 @@ def make_url(host): with mock.patch('httpx.Request', wraps=httpx.Request) as request_mock: with mock.patch('httpx.AsyncClient.send', side_effect=httpx.RequestError('')) as send_mock: - with pytest.raises(httpx.RequestError): + with pytest.raises(AblyException, match="network error"): await ably.http.make_request('GET', '/', skip_auth=True) assert send_mock.call_count == Defaults.http_max_retry_count @@ -93,7 +93,7 @@ async def test_no_host_fallback_nor_retries_if_custom_host(self): mock_route = respx.get("https://example.org").mock(side_effect=httpx.RequestError('')) - with pytest.raises(httpx.RequestError): + with pytest.raises(AblyException, match="network error"): await ably.http.make_request('GET', '/', skip_auth=True) assert mock_route.call_count == 1 diff --git a/test/ably/http/httppush_test.py b/test/ably/http/httppush_test.py index 9c430cf7..1e17ab18 100644 --- a/test/ably/http/httppush_test.py +++ b/test/ably/http/httppush_test.py @@ -137,9 +137,9 @@ async def test_admin_publish(self): } publish = self.ably.push.admin.publish - with pytest.raises(TypeError): + with pytest.raises(AblyException): await publish('ablyChannel', data) - with pytest.raises(TypeError): + with pytest.raises(AblyException): await publish(recipient, 25) with pytest.raises(ValueError): await publish({}, data) diff --git a/test/ably/http/httprequest_test.py b/test/ably/http/httprequest_test.py index ec41c5ff..bc378e92 100644 --- a/test/ably/http/httprequest_test.py +++ b/test/ably/http/httprequest_test.py @@ -5,6 +5,7 @@ from ably.pubsub.request.paginatedresult import HttpPaginatedResponse from ably.pubsub.server import create_http_client from ably.pubsub.transport.defaults import Defaults +from ably.pubsub.util.exceptions import AblyException from test.ably.testapp import TestApp from test.ably.utils import BaseAsyncTestCase, VaryByProtocolTestsMetaclass, dont_vary_protocol @@ -102,8 +103,10 @@ async def test_timeout(self): assert ably.http.http_request_timeout == timeout # `http_open_timeout` keeps its four-second default, so the request carries a # connect budget as well as a read budget and either can be the one to expire - with pytest.raises(httpx.TimeoutException): + with pytest.raises(AblyException) as excinfo: await ably.request('GET', '/time', version=Defaults.protocol_version) + assert excinfo.value.code == 50003 + assert isinstance(excinfo.value.cause, httpx.TimeoutException) await ably.close() default_endpoint = 'https://sandbox.realtime.ably-nonprod.net/time' @@ -127,8 +130,10 @@ async def test_timeout(self): port=self.test_vars["port"], tls_port=self.test_vars["tls_port"], tls=self.test_vars["tls"]) - with pytest.raises(httpx.ConnectError): + with pytest.raises(AblyException) as excinfo: await ably.request('GET', '/time', version=Defaults.protocol_version) + assert excinfo.value.code == 80000 + assert isinstance(excinfo.value.cause, httpx.ConnectError) await ably.close() # RSC15l3 diff --git a/test/ably/realtime/realtimechannel_publish_test.py b/test/ably/realtime/realtimechannel_publish_test.py index a5af09f5..5afa1d27 100644 --- a/test/ably/realtime/realtimechannel_publish_test.py +++ b/test/ably/realtime/realtimechannel_publish_test.py @@ -186,7 +186,7 @@ async def check_pending(): # Force FAILED state connection_manager.notify_state( ConnectionState.FAILED, - AblyException('Test failure', 80000, 500) + AblyException('Test failure', 500, 80000) ) # Should raise exception diff --git a/test/unit/http_test.py b/test/unit/http_test.py index ebae07b2..c5be5803 100644 --- a/test/unit/http_test.py +++ b/test/unit/http_test.py @@ -1,9 +1,13 @@ import httpx import pytest +from ably.pubsub.realtime.connection import ConnectionState from ably.pubsub.server import create_http_client from ably.pubsub.types.testoptions import TestOptions from ably.pubsub.util.exceptions import AblyException +from test.uts.helpers.client import await_connection_state, close_open_clients, realtime_client +from test.uts.helpers.mock_http import MockHttpClient +from test.uts.helpers.mock_websocket import MockWebSocket def test_http_get_rest_hosts_works_when_fallback_realtime_host_is_set(): @@ -54,14 +58,35 @@ def refuse(request): transport = RecordingTransport(refuse) ably = create_http_client(token="foo", _test_options=TestOptions(http_transport=transport)) - with pytest.raises(AblyException): + with pytest.raises(AblyException) as excinfo: await ably.time() + # RSC19e: the httpx error is wrapped, and kept as the cause + assert excinfo.value.code == 80000 + assert excinfo.value.status_code == 500 + assert isinstance(excinfo.value.cause, httpx.ConnectError) + assert excinfo.value.__cause__ is excinfo.value.cause # Every host is tried before the error is surfaced (RSC15l) assert len(transport.requests) == len(ably.http.get_hosts()) await ably.close() +async def test_http_surfaces_transport_timeouts_as_50003(): + def time_out(request): + raise httpx.ReadTimeout("timed out", request=request) + + transport = RecordingTransport(time_out) + ably = create_http_client(key="appId.keyId:keySecret", _test_options=TestOptions(http_transport=transport)) + + with pytest.raises(AblyException) as excinfo: + await ably.channels.get('test').publish('event', 'data') + + assert excinfo.value.code == 50003 + assert excinfo.value.status_code == 504 + assert isinstance(excinfo.value.cause, httpx.TimeoutException) + await ably.close() + + def test_http_uses_a_network_transport_without_test_options(): ably = create_http_client(token="foo") assert isinstance(ably.http._Http__client._transport, httpx.AsyncHTTPTransport) @@ -86,3 +111,57 @@ def respond(request): assert 'Authorization' not in auth_requests[0].headers assert 'X-Ably-Version' not in auth_requests[0].headers await ably.close() + + +async def test_auth_url_network_errors_are_40170(): + def respond(request): + if request.url.host == 'auth.example.com': + raise httpx.ConnectError("connection refused", request=request) + return httpx.Response(200, json=[1500000000000]) + + transport = RecordingTransport(respond) + ably = create_http_client(auth_url='https://auth.example.com/token', + _test_options=TestOptions(http_transport=transport)) + + # RSA4e: a network error reaching the auth_url is a 40170, with the httpx error as the cause + with pytest.raises(AblyException) as excinfo: + await ably.auth.authorize() + + assert excinfo.value.code == 40170 + assert excinfo.value.status_code == 401 + assert isinstance(excinfo.value.cause, httpx.ConnectError) + await ably.close() + + +async def test_realtime_token_request_network_errors_reach_the_connection_wrapped(): + mock_http = MockHttpClient(on_connection_attempt=lambda conn: conn.respond_with_refused()) + mock_ws = MockWebSocket() + client = realtime_client(mock_ws, mock_http, key="appId.keyId:keySecret", use_token_auth=True, + disconnected_retry_timeout=600000) + try: + client.connect() + await await_connection_state(client, ConnectionState.DISCONNECTED) + + # RSA4c: a failed TokenRequest exchange is an auth error, so the connection is not + # left holding the httpx exception + assert isinstance(client.connection.error_reason, AblyException) + assert client.connection.error_reason.code == 80019 + assert mock_ws.connection_attempts == [] + finally: + await close_open_clients() + + +async def test_auth_callback_ably_exceptions_are_passed_through(): + error = AblyException('Account disabled', 403, 40300) + + async def auth_callback(params): + raise error + + ably = create_http_client(auth_callback=auth_callback) + + # RSA4e: only an error that is not already an AblyException becomes a 40170 + with pytest.raises(AblyException) as excinfo: + await ably.auth.authorize() + + assert excinfo.value is error + await ably.close() diff --git a/test/unit/options_test.py b/test/unit/options_test.py index 84cc963b..b58e4ce1 100644 --- a/test/unit/options_test.py +++ b/test/unit/options_test.py @@ -1,5 +1,6 @@ import pytest +from ably.pubsub.server import create_http_client from ably.pubsub.types.options import Options from ably.pubsub.util.exceptions import AblyException @@ -197,3 +198,19 @@ def test_options_localhost_no_fallback_hosts(): opts = Options(endpoint="localhost") # REC2c2: explicit hostname has empty fallback assert opts.get_fallback_hosts() == [] + + +# RSC1b: invalid arguments are a 40106, and still a ValueError as they were in 3.x +@pytest.mark.parametrize('kwargs, status_code', [ + ({}, 401), + ({'key': 'appId.keyId:keySecret', 'key_name': 'appId.keyId'}, 401), + ({'token_details': 'not-token-details'}, 401), + ({'token': 'a-token', 'use_token_auth': False}, 401), + ({'key': 'appId.keyId:keySecret', 'endpoint': 'foo', 'environment': 'foo'}, 400), +]) +def test_invalid_client_arguments_raise_40106(kwargs, status_code): + with pytest.raises(AblyException) as exinfo: + create_http_client(**kwargs) + assert exinfo.value.code == 40106 + assert exinfo.value.status_code == status_code + assert isinstance(exinfo.value, ValueError) diff --git a/test/uts/deviations.md b/test/uts/deviations.md index 28535f9f..96654a0d 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -33,8 +33,8 @@ parametrized over a table of fixtures the specification gives inline; and five `objects/unit` tests are parametrized, three over a table and two over the scenarios the specification loops through. That turns 1481 derived tests into 1608 pytest cases. -Of **1472 Test IDs, derived as 1481 tests and run as 1608 pytest cases**: 1289 Test IDs -(1298 tests, 1423 cases) pass, 168 (168 tests, 170 cases) are gated behind +Of **1472 Test IDs, derived as 1481 tests and run as 1608 pytest cases**: 1293 Test IDs +(1302 tests, 1427 cases) pass, 164 (164 tests, 166 cases) are gated behind `RUN_DEVIATIONS`, and 15 (15 tests, 15 cases) cannot be run at all. The three groups are disjoint: two Test IDs have a gated part and a passing part — `TI1`, as two functions, and `TO/endpoint-affects-host`, as one parametrized test — and are counted with the gated, @@ -45,19 +45,19 @@ the Test IDs come from `uts/rest/unit` (503 tests, 536 cases), 482 from `uts/rest/integration` (84, 122), 73 from `uts/realtime/integration` (73, 95) and 17 from `uts/objects/integration` (17, 29); 8 of the REST integration ids (8, 8), 30 of the realtime ones (30, 30) and 5 of the objects ones (5, 5) come from the `proxy` package -within each. Of the gated Test IDs 79 are REST and 89 realtime, which is 80 REST cases -and 90 realtime. None of the 339 objects Test IDs is gated or unrunnable. +within each. Of the gated Test IDs 79 are REST and 85 realtime, which is 80 REST cases +and 86 realtime. None of the 339 objects Test IDs is gated or unrunnable. A further 148 pytest cases cover the harness itself and are not derived from a specification: 130 under `helpers/`, for the mocks, and 18 under `objects/helpers/`, for the objects builders and the synced-channel mock. -The 157 gated Test IDs that record SDK non-compliance — 157 tests, 159 cases — reduce to -**69 distinct root causes**, 26 on the REST side and 43 on the realtime side. Three further +The 153 gated Test IDs that record SDK non-compliance — 153 tests, 155 cases — reduce to +**68 distinct root causes**, 26 on the REST side and 42 on the realtime side. Three further defects are recorded below with no test of their own, because the specification's test cannot discriminate (RTP18a), has nothing to assert against (the timezone split on synthesized LEAVE timestamps), or is worked around in the setup of every test that would otherwise trip over it (`enterClient` on an anonymous connection), so the file -carries **72 SDK root causes** in all. The remaining 11 gated Test IDs are +carries **71 SDK root causes** in all. The remaining 11 gated Test IDs are specification faults, and reduce to 8. The objects tier records no SDK non-compliance; its specification faults are all corrected or worked around in tests that pass, and reduce to the seven candidate issues O.1–O.7, none of them filed. @@ -1735,37 +1735,14 @@ seconds rather than the default ten. token string, or leaves `_configure_client_id` alone where the fetched token carries no clientId of its own. -#### An authCallback error is always rewritten as 401/40170, so RSA4d is unreachable — 4 tests - -**Spec points:** RSA4d, RSA4d1. - -`ably/rest/auth.py:182-187` wraps **every** exception an authCallback raises as -`AblyException("auth_callback raised an exception", 401, 40170, cause=e)`, discarding the -original `statusCode`. `ConnectionManager.on_error_from_authorize` (`connectionmanager.py:479-491`) -then branches on `exception.status_code == 403` to reach FAILED, and that branch can never be -taken for an authCallback: the status is always 401, so a 403 goes to the `__fail_state` -(DISCONNECTED) with an 80019/401 instead. RSA4d requires FAILED with 80019/**403** and -`cause` set to the 403, both during the connect sequence and during an RTN22 reauth. - -| Test | Observed | -|---|---| -| `connection_auth_test.py::test_rsa4d_callback_403_causes_failed` | `Timed out waiting for connection state failed; it was disconnected` | -| `connection_auth_test.py::test_rsa4d_callback_403_reauth_causes_failed` | `Timed out waiting for connection state failed; it was connected` | -| `auth_callback_errors_test.py::test_rsa4d_callback_403_connecting_failed` | `Timed out waiting for connection state failed; it was disconnected` | -| `auth_callback_errors_test.py::test_rsa4d_callback_403_reauth_failed` | `Timed out waiting for connection state failed; it was connected` | - -**Status:** open bug. Preserving the callback error's `statusCode` — or letting an -`AblyException` from the callback through unwrapped — also restores the `cause` chain -recorded under Adapted Tests. - #### A failed RTN22 reauth leaves no trace on the connection — 1 test **Spec points:** RSA4c1, RSA4c3. `WebSocketTransport.on_protocol_message` (`websockettransport.py:170-175`) handles a server -AUTH by awaiting `auth.authorize()` inside a bare `except Exception` that only logs. Nothing -reaches `on_error_from_authorize`, so no 80019 is built and `connection.errorReason` stays -as it was. +AUTH by awaiting `auth.authorize()` inside a bare `except Exception` that only logs. Only a +403, which `authorize()` hands on for RSA4d, reaches `on_error_from_authorize`; any other +failure builds no 80019, and `connection.errorReason` stays as it was. **Tests affected:** `test_rsa4c3_callback_error_stays_connected` — `Timed out waiting for errorReason to be set`. @@ -2041,34 +2018,6 @@ asserting `clientId == 'my-client'` with the spec expectation in a comment. Adapted rather than gated because the behaviour is stable and the rest of each test — action, channel, payload — is worth running. -### `AblyException`'s status code and code are transposed at six sites - -The constructor is `AblyException(message, status_code, code)` (`util/exceptions.py:15`). -Six raises put the Ably error code in the status slot and the HTTP status in the code slot, -so the resulting exception reports each as the other: - -| Site | As written | Should be | -|---|---|---| -| `realtime/channel.py:203` | `AblyException("Unable to detach; channel state = failed", 90001, 400)` | `400, 90001` | -| `realtime/channel.py:217` | `AblyException("Detach request superseded by a subsequent attach request", 90000, 409)` | `409, 90000` | -| `realtime/connectionmanager.py:343` | `AblyException("Connection failed", 80000, 500)` | `500, 80000` | -| `types/mixins.py:84` | `AblyException('VCDiff decoder not available', 40019, 40019)` | `400, 40019` | -| `types/mixins.py:88` | `AblyException('VCDiff decode failure', 40018, 40018)` | `400, 40018` | -| `types/mixins.py:111` | `AblyException('VCDiff decode failure', 40018, 40018) from e` | `400, 40018` | - -The two neighbours that get it right — `channel.py:860` (`408, 90007`) and -`message.py:304` (`400, 40018`, for the sibling of the `mixins.py` errors) — make this a -repeated slip rather than a misunderstanding of the constructor. - -**Tests affected:** `test_rtl5b_detach_failed_errors` and `test_rtl4h_attach_while_detaching` -assert on `status_code` with a comment recording the transposition. -`test_rtn7d_fail_disconnected_no_queue`, `test_rtn7e_pending_fail_closed` and -`test_rtn7e_multiple_pending_fail` assert only what the specification asks — that a code is -present — so they pass; the wrong value is recorded here rather than asserted. - -**Status:** open bug. A mechanical fix, and a caller branching on `status_code` today gets a -five-digit number. - ### A refused connection and a connect timeout reach no failure path **Spec points:** RTN14d most directly; the same defect shapes RTN14e, RTN14f, RTN14h, @@ -2186,31 +2135,19 @@ either — see the UTS Spec Error above. The three want closing together. **Status:** open bug. It is the reason the fake clock exists in this suite at all; see the fake-time section at the end of this file. -### An 80019 from a failed auth carries no `cause`, and a slow callback is not attributed - -**Spec points:** RSA4c, RSA4c1, RSA4c2. - -Two adaptations with one underlying theme — an auth failure reaches the right state with the -wrong explanation. - -- `ConnectionManager.on_error_from_authorize` builds - `AblyException('Client configured authentication provider request failed', 401, 80019)` - with no `cause` argument, so the error the authCallback raised survives only in the log. - RSA4c1/RSA4c2 require `cause` to be set to the underlying error. Same root cause as the - RSA4d entry above: `request_token`'s wrapper loses the original error's shape, and - `on_error_from_authorize` then drops what is left. Adapted, not gated, because the state, - code and status are all correct and worth guarding — only the `cause` link is missing. - Tests: `test_rsa4c2_callback_error_causes_disconnected`, - `test_rsa4c2_callback_error_connecting_disconnected`. -- RSA4c treats an auth attempt that outruns `realtimeRequestTimeout` as an auth error, - giving DISCONNECTED with 80019/401. ably-python applies no timeout to the callback — - `await auth_callback(token_params)` is unbounded — and the CONNECTING transition timer - (`connectionmanager.py:699-724`) ends the attempt instead, with the generic 504/50003 it - raises for any connect that does not complete in time. Test: - `test_rsa4c2_callback_timeout_connecting_disconnected`, asserting 50003/504 on a - `FakeClock`. - -**Status:** open bugs, cosmetic in effect — the connection recovers either way, but the +### A slow authCallback is not attributed to the auth provider + +**Spec points:** RSA4c, RSA4c2. + +RSA4c treats an auth attempt that outruns `realtimeRequestTimeout` as an auth error, giving +DISCONNECTED with 80019/401. ably-python applies no timeout to the callback — +`await auth_callback(token_params)` is unbounded — and the CONNECTING transition timer +(`connectionmanager.py:699-724`) ends the attempt instead, with the generic 504/50003 it +raises for any connect that does not complete in time. Test: +`test_rsa4c2_callback_timeout_connecting_disconnected`, asserting 50003/504 on a +`FakeClock`. + +**Status:** open bug, cosmetic in effect — the connection recovers either way, but the error does not say the auth provider is at fault. ### A token error with no means to renew reports the renewal failure, not the server's error @@ -2364,7 +2301,7 @@ pseudocode is not mistaken for non-compliance. | RTP19a's route | the specification models an ATTACHED without HAS_PRESENCE as `startSync()` then `endSync()`. `on_attached(has_presence=False)` calls `_synthesize_leaves(...)` then `clear()` (`presence.py:611-618`), which is the requirement itself rather than the model of it | | `hasNext` as a value | `push_admin.md` RSH1b2 writes `ASSERT result.hasNext == true`. In Python `has_next` is a bound method (`ably/http/paginatedresult.py:63`), truthy whatever the page holds, so the direct translation asserts nothing. `test_rsh1b2_list_devices_pagination` writes `result.has_next() is True`, confirmed to fail when inverted | | Push `remove` return values | the specification's remove steps assert nothing about a return value ("should not throw"). `PushDeviceRegistrations.remove` / `remove_where` and `PushChannelSubscriptions.remove` / `remove_where` return the `ably.http.http.Response` from the DELETE rather than `None` (`ably/rest/push.py:112-127`, `:176-192`), so the six derived removal tests assert `response.status_code == 204`, which is stronger than the specification asks and matches what `test/ably/rest/restpush_test.py` already asserts | -| `rest/proxy/RSC15l/unreachable-endpoint-error-0` | the specification asserts only that the error carries a non-null `status_code` or `code`, leaving the values open, so the derived test asserts exactly that and is the weakest of the eight proxy tests. What this SDK produces against a refused connection is 500 / 50000 with the message "All connection attempts failed", `catch_all` having wrapped the `httpx.ConnectError`, and that is recorded here rather than asserted, since pinning it would assert more than the specification does | +| `rest/proxy/RSC15l/unreachable-endpoint-error-0` | the specification asserts only that the error carries a non-null `status_code` or `code`, leaving the values open, so the derived test asserts exactly that and is the weakest of the eight proxy tests. What this SDK produces against a refused connection is 500 / 80000, the RSC19e wrapping of the `httpx.ConnectError`, which it keeps as the `cause`, and that is recorded here rather than asserted, since pinning it would assert more than the specification does | | `RSL1k4`'s history read | the event log is per session, so any request the client under test makes through it is recorded — including the `history()` the test verifies deduplication with. The log is therefore read **before** the history call, and the POST count assertion made against that snapshot. The history read itself is a `wall_clock_poll_until` rather than one fetch, for the same reason every other integration verification is: a published message does not reach history at once | | RSP4b1's time bounds | the specification records `time_before = now_millis()` before generating the presence events and `time_after = now_millis()` after, then asserts a `history(start=, end=)` over that window returns them. Read from the runner's clock the window is only as good as the skew against the sandbox, which decides the timestamps actually stored, so a runner running a little fast would exclude the very events the test generated. Both bounds come from `await client.time()` instead — the same instant on the clock that stamps the events. `Presence.history` passes an `int` straight through as milliseconds (`ably/types/presence.py:232-241`), which is what `client.time()` returns, so no conversion is involved | @@ -2375,15 +2312,13 @@ pseudocode is not mistaken for non-compliance. | RSL2 | A space in a channel name is `%20` | `+`, from `parse.quote_plus` in `Channel.__init__`. `quote_plus` is form encoding, and a `+` in a URL *path* is a literal plus, so the name reaching the server is altered | Open bug, and a genuine correctness issue | | RSL8 | `Channel#status` URI-encodes the channel id | `status()` interpolates the name with no escaping at all. `a/b` addresses the wrong resource, `a?b` truncates the name into a query string, `a#b` becomes a fragment | Open bug | | RSL2, RSL11b, RSL15b | `:` is `%3A` | Left literal, from `safe=':'`. RFC 3986 allows `:` in a path segment and Ably uses it for namespaces, so the server receives the same value | Intentional | -| RSC1b | Error code 40106 | A bare `ValueError` from `AblyRest.__init__` with an informative message, not an `AblyException`, so there is no code. The realtime constructor shares it — `test_rtc12_invalid_arguments_error` records the same behaviour | Open bug | | RTC12 / RSC1, RSC1a, RSC1c | A string constructor argument is an API key when it contains `:` and a token when it does not | `AblyRest.__init__` treats its first positional argument as a key unconditionally and hands it to `AuthOptions.set_key`, which requires exactly two colon-separated parts, so a token string raises 40101/401 "key of not len 2 parameters". A token is supplied through the separate `token` or `token_details` arguments. The empty-string case is compliant | Intentional / SDK-wide: the constructor takes credentials as distinct named arguments and has no string-sniffing path to restore | | RSC18 | The constructor rejects basic auth over HTTP | Construction succeeds; 40103 is raised from `make_request` when a request needing Basic Auth is attempted, and no request goes out. RSA1/RSC18 say only "any attempt to use" | Compliant; the UTS is stricter than its source | | REC1b1, REC1c1 | Code 40000, or a message containing "invalid" or "conflict" | 400/40106 with a specific message. The features spec mandates no code | Cosmetic | | RSAN1a3 | Code 40003 for a missing `Annotation.type` | 400/40000 | Cosmetic; worth aligning cross-SDK | -| RSH1a | Empty `recipient` or `data` rejected with code 40000, the error reaching the caller from the server | `PushAdmin.publish` validates its arguments itself and raises before touching the HTTP layer (`ably/rest/push.py:49-59`): a non-dict `recipient` or `data` raises `TypeError`, an empty one `ValueError`. So there is no request, no server error and no `code` to read. The "no HTTP request" half is satisfied, and the repository's own sandbox suite already pins the exception types (`test/ably/rest/restpush_test.py::test_admin_publish`). `test_rsh1a_push_publish_invalid_recipient` in the integration tier asserts `pytest.raises(ValueError)` alongside the unit-tier test | Open bug, minor — a stricter precondition rather than wrong behaviour. The specification's test would need a recipient the SDK will send and the server will reject, an unknown `transportType` say, to exercise the server-side path it describes | +| RSH1a | Empty `recipient` or `data` rejected with code 40000, the error reaching the caller from the server | `PushAdmin.publish` validates its arguments itself and raises before touching the HTTP layer: an empty or non-dict `recipient` or `data` raises an `AblyException` with 400/40000, the code the server gives, which is also a `ValueError`. So no request is made and there is no server error; the integration tier's `test_rsh1a_push_publish_invalid_recipient` asserts the local error's code | Compliant: RSH1a asks for empty values to be "immediately rejected". The specification's test would need a recipient the SDK will send and the server will reject, an unknown `transportType` say, to exercise the server-side path it describes | | HP6 | `errorCode` is a number | The raw header string, `'40101'` | Open bug, trivial | | HP8 | `headers` is a map | A list of `(name, value)` pairs, so the lookup the spec describes is impossible without converting, and case-insensitivity is lost | Open bug; changing the return type is breaking | -| RSC19e | An error indicated idiomatically | `httpx.ConnectError` / `ReadTimeout` reach the caller unwrapped, because `AblyRest.request` carries no `@catch_all` unlike `time()` and `stats()`. The messages do name the failure | Borderline; defensible under RSC19e | | RSC15a | Six hosts tried | Three. `Options.__get_hosts` truncates to `http_max_retry_count`, which TO3l5 sanctions | Intentional | | TI | `ErrorInfo` equality by attributes | No `__eq__`, so errors compare by identity. Python exceptions conventionally do, and the requirement appears nowhere in `features.md`. Adding `__eq__` without `__hash__` would make `AblyException` unhashable and break any caller that puts one in a set | Intentional | | TD5, RSA16a | `capability` is stringified JSON | A `Capability` object, a public convenience type used throughout `auth`. Narrowing the return type to `str` would break every caller that indexes or mutates it, so it is reserved for a future major | Intentional; a breaking change to align | @@ -2890,26 +2825,13 @@ each request and bypasses the client's own HTTP layer entirely — no client-sco observe or stub it. Two issues in one line. The derived tests reach it only through `monkeypatch`; the three REC3 tests are skipped stubs. -**2.8 An authCallback error is always rewritten as 401/40170, so RSA4d is unreachable.** -RSA4d, RSA4d1. `rest/auth.py:182-187` wraps every callback exception as -`AblyException(…, 401, 40170, cause=e)`, discarding the original status, and -`on_error_from_authorize` branches on `status_code == 403` to reach FAILED — a branch an -authCallback can never take. A 403 from an auth provider therefore gives DISCONNECTED with -80019/401 where the spec requires FAILED with 80019/403. The same wrapper is why the 80019 -carries no `cause`, and why RSA4f's non-`AblyException` (`AttributeError: 'int' object has -no attribute 'key_name'`) reaches `Connection#errorReason` with no `code` at all. One issue, -three symptoms. -`test/uts/realtime/unit/auth -k "rsa4d or rsa4f_callback_invalid_type_format"` - -**2.9 `AblyException`'s status code and code are transposed at six sites.** The constructor -is `(message, status_code, code)`. `channel.py:203` (`90001, 400`), `channel.py:217` -(`90000, 409`), `connectionmanager.py:343` (`80000, 500`), `mixins.py:84` (`40019, 40019`), -`mixins.py:88` and `:111` (`40018, 40018`). `channel.py:860` (`408, 90007`) and -`message.py:304` (`400, 40018`) are right, so it is a repeated slip rather than a -misunderstanding. A caller branching on `status_code` today gets a five-digit number. -Mechanical fix. No gated test — three adapted tests assert the wrong values deliberately -(`channel_detach_test.py -k "rtl5b or rtl4h"`), so they turn red when it is fixed, which is -the signal wanted. +**2.8 An authCallback result of the wrong type reaches `errorReason` as an `AttributeError`.** +RSA4f, RSA4c2. `Auth.request_token` has no RSA4f type check, so a callback returning `12345` +falls through to `token_request.key_name` and raises `AttributeError`, which `try_host`'s +`except AblyException` does not catch. `connect_base` then records it as +`Connection#errorReason`, with no `code` at all, where the spec requires DISCONNECTED with +80019/401. +`test/uts/realtime/unit/auth -k rsa4f_callback_invalid_type_format` ### Tier 3 — silently wrong data @@ -3731,8 +3653,8 @@ The header states how many derived tests there are, how many pass, how many are gated and how many cannot run. Those numbers are the check that the file is still true: in pytest cases, the gated count must equal the number of failures under `RUN_DEVIATIONS=1`, and gated plus unrunnable must equal the number of skips without -it. As of this writing that is 170 failures, 1571 passes and 15 skips with the variable -set, and 185 skips and 1571 passes without it, the 1571 being 1423 derived cases and 148 +it. As of this writing that is 166 failures, 1575 passes and 15 skips with the variable +set, and 181 skips and 1575 passes without it, the 1575 being 1427 derived cases and 148 harness ones — 130 under `helpers/` and 18 under `objects/helpers/`. Those figures need the `submodules` checkout CI makes: without it, three `rest/unit/encoding` tests that read the ably-common fixtures skip as well. The tiers are told apart by the junit report's diff --git a/test/uts/realtime/unit/auth/auth_callback_errors_test.py b/test/uts/realtime/unit/auth/auth_callback_errors_test.py index 7f056457..768cd870 100644 --- a/test/uts/realtime/unit/auth/auth_callback_errors_test.py +++ b/test/uts/realtime/unit/auth/auth_callback_errors_test.py @@ -73,10 +73,8 @@ async def auth_callback(params): assert client.connection.error_reason is not None assert client.connection.error_reason.code == 80019 assert client.connection.error_reason.status_code == 401 - # RSA4c2 requires `cause` to carry the underlying 50000 error. - # `ConnectionManager.on_error_from_authorize` builds the 80019 without one, - # so the callback's own error is only in the log. - assert client.connection.error_reason.cause is None + assert client.connection.error_reason.cause is not None + assert client.connection.error_reason.cause.code == 50000 disconnected_changes = [c for c in state_changes if c.current == ConnectionState.DISCONNECTED] assert len(disconnected_changes) >= 1 @@ -151,7 +149,6 @@ async def auth_callback(params): # UTS: realtime/unit/RSA4d/callback-403-connecting-failed-0 -@deviation async def test_rsa4d_callback_403_connecting_failed(): connection_attempted = False @@ -194,7 +191,6 @@ def on_connection_attempt(conn): # UTS: realtime/unit/RSA4d/callback-403-reauth-failed-1 -@deviation async def test_rsa4d_callback_403_reauth_failed(): auth_callback_count = 0 diff --git a/test/uts/realtime/unit/auth/connection_auth_test.py b/test/uts/realtime/unit/auth/connection_auth_test.py index a1c32b77..5f935696 100644 --- a/test/uts/realtime/unit/auth/connection_auth_test.py +++ b/test/uts/realtime/unit/auth/connection_auth_test.py @@ -157,10 +157,8 @@ async def auth_callback(params): assert client.connection.error_reason is not None assert client.connection.error_reason.code == 80019 assert client.connection.error_reason.status_code == 401 - # RSA4c1 requires `cause` to carry the underlying 50000 error. - # `ConnectionManager.on_error_from_authorize` builds the 80019 without one, - # so the callback's own error is only in the log. - assert client.connection.error_reason.cause is None + assert client.connection.error_reason.cause is not None + assert client.connection.error_reason.cause.code == 50000 # UTS: realtime/unit/RSA4c3/callback-error-stays-connected-0 @@ -202,7 +200,6 @@ async def auth_callback(params): # UTS: realtime/unit/RSA4d/callback-403-causes-failed-0 -@deviation async def test_rsa4d_callback_403_causes_failed(): async def auth_callback(params): raise AblyException('Account disabled', 403, 40300) @@ -221,7 +218,6 @@ async def auth_callback(params): # UTS: realtime/unit/RSA4d/callback-403-reauth-causes-failed-1 -@deviation async def test_rsa4d_callback_403_reauth_causes_failed(): auth_callback_count = 0 diff --git a/test/uts/realtime/unit/channels/channel_attach_test.py b/test/uts/realtime/unit/channels/channel_attach_test.py index 8f6b05d7..d59519d9 100644 --- a/test/uts/realtime/unit/channels/channel_attach_test.py +++ b/test/uts/realtime/unit/channels/channel_attach_test.py @@ -153,9 +153,8 @@ def on_message_from_client(msg): # detach and makes the pending detach fail with 90000 with pytest.raises(AblyException) as detach_error: await asyncio.wait_for(detach_future, OPERATION_TIMEOUT) - # The 90000 the library passes lands in `status_code` and the 409 in `code`, - # the two being the other way round in `AblyException(message, status_code, code)` - assert detach_error.value.status_code == 90000 + assert detach_error.value.code == 90000 + assert detach_error.value.status_code == 409 await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) diff --git a/test/uts/realtime/unit/channels/channel_detach_test.py b/test/uts/realtime/unit/channels/channel_detach_test.py index e5a2a17d..f113bfb6 100644 --- a/test/uts/realtime/unit/channels/channel_detach_test.py +++ b/test/uts/realtime/unit/channels/channel_detach_test.py @@ -208,9 +208,8 @@ def on_message_from_client(msg): with pytest.raises(AblyException) as error: await asyncio.wait_for(channel.detach(), OPERATION_TIMEOUT) - # The 90001 the library passes lands in `status_code` and the 400 in `code`, - # the two being the other way round in `AblyException(message, status_code, code)` - assert error.value.status_code == 90001 + assert error.value.code == 90001 + assert error.value.status_code == 400 assert channel.state == ChannelState.FAILED diff --git a/test/uts/realtime/unit/client/realtime_client_test.py b/test/uts/realtime/unit/client/realtime_client_test.py index b680edd5..049ef877 100644 --- a/test/uts/realtime/unit/client/realtime_client_test.py +++ b/test/uts/realtime/unit/client/realtime_client_test.py @@ -71,14 +71,10 @@ async def test_rtc12_constructor_string_detection(): async def test_rtc12_invalid_arguments_error(): mock_ws = succeeding_mock() - # DEVIATION: the spec expects error code 40106 when no valid credentials are - # provided. ably-python rejects the options in the constructor instead, with a - # plain ValueError that carries no Ably error code. This mirrors the REST suite's - # `RSC1b/no-auth-method-error-0`. - with pytest.raises(ValueError) as excinfo: + with pytest.raises(AblyException) as excinfo: realtime_client(mock_ws, key=None) - assert 'key is missing' in str(excinfo.value) + assert excinfo.value.code == 40106 assert mock_ws.connection_attempts == [] diff --git a/test/uts/rest/integration/proxy/rest_fallback_test.py b/test/uts/rest/integration/proxy/rest_fallback_test.py index aed99207..afafb084 100644 --- a/test/uts/rest/integration/proxy/rest_fallback_test.py +++ b/test/uts/rest/integration/proxy/rest_fallback_test.py @@ -167,7 +167,7 @@ async def test_rsc15l_unreachable_endpoint_error(sandbox): # (the exact code/statusCode depends on the SDK's HTTP layer, but it must # be present and non-null so callers can programmatically handle it). # NOTE: the specification leaves the values open. This SDK reports 500 and - # 50000, the connection error having been wrapped by `catch_all`. + # 80000, the connection error having been wrapped per RSC19e. error = excinfo.value assert error is not None assert error.status_code is not None or error.code is not None diff --git a/test/uts/rest/integration/push_admin_test.py b/test/uts/rest/integration/push_admin_test.py index 16720dd3..f997217b 100644 --- a/test/uts/rest/integration/push_admin_test.py +++ b/test/uts/rest/integration/push_admin_test.py @@ -68,15 +68,14 @@ async def test_rsh1a_push_publish_clientid(sandbox): async def test_rsh1a_push_publish_invalid_recipient(sandbox): client = sandbox_rest_client(sandbox.key(0).key_str) - # NOTE: the spec expects the server to reject the empty recipient and the - # error to carry a `code`. `PushAdmin.publish` validates the recipient - # itself (`ably/rest/push.py:57`) and raises `ValueError` before any request - # is made, so there is no server error and no code to read. See the RSH1a row - # under Adapted Tests -> REST behaviours asserted as they are, in - # test/uts/deviations.md. - with pytest.raises(ValueError): + # NOTE: the spec expects the server to reject the empty recipient. + # `PushAdmin.publish` rejects it itself, with the 40000 the server would + # give, before any request is made (RSH1a: "immediately rejected"). + with pytest.raises(AblyException) as excinfo: await client.push.admin.publish({}, {'notification': {'title': 'Test'}}) + assert excinfo.value.code is not None + # UTS: rest/integration/RSH1b3/save-and-get-device-0 async def test_rsh1b3_save_and_get_device(sandbox): diff --git a/test/uts/rest/unit/auth/auth_scheme_test.py b/test/uts/rest/unit/auth/auth_scheme_test.py index e69b732a..d51e94b2 100644 --- a/test/uts/rest/unit/auth/auth_scheme_test.py +++ b/test/uts/rest/unit/auth/auth_scheme_test.py @@ -189,13 +189,13 @@ async def test_rsc1b_no_auth_method_error(): on_request=capture_and_respond(captured_requests), ) - # DEVIATION: the spec expects an error with code 40106 from the first - # request. ably-python rejects the options in the constructor instead, with - # a plain ValueError that carries no Ably error code. - with pytest.raises(ValueError) as excinfo: + # The spec expects the error from the first request. RSC1b gives it to + # the constructor, and ably-python raises it there, so no client exists to + # make a request with. + with pytest.raises(AblyException) as excinfo: rest_client(mock_http, key=None) - assert 'key is missing' in str(excinfo.value) + assert excinfo.value.code == 40106 assert len(captured_requests) == 0 diff --git a/test/uts/rest/unit/push/push_admin_publish_test.py b/test/uts/rest/unit/push/push_admin_publish_test.py index 846baa70..f781e0c1 100644 --- a/test/uts/rest/unit/push/push_admin_publish_test.py +++ b/test/uts/rest/unit/push/push_admin_publish_test.py @@ -120,16 +120,13 @@ async def test_rsh1a_rejects_empty_recipient(): ) client = rest_client(mock_http) - # DEVIATION: the spec asserts error.code == 40000. PushAdmin.publish validates - # locally and raises a plain ValueError, which carries no Ably error code. - with pytest.raises(ValueError) as excinfo: + with pytest.raises(AblyException) as excinfo: await client.push.admin.publish( recipient={}, data={'notification': {'title': 'Test'}}, ) - assert not isinstance(excinfo.value, AblyException) - assert 'recipient' in str(excinfo.value) + assert excinfo.value.code == 40000 assert len(captured_requests) == 0 @@ -143,16 +140,13 @@ async def test_rsh1a_rejects_empty_data(): ) client = rest_client(mock_http) - # DEVIATION: the spec asserts error.code == 40000. PushAdmin.publish validates - # locally and raises a plain ValueError, which carries no Ably error code. - with pytest.raises(ValueError) as excinfo: + with pytest.raises(AblyException) as excinfo: await client.push.admin.publish( recipient={'clientId': 'user-123'}, data={}, ) - assert not isinstance(excinfo.value, AblyException) - assert 'data' in str(excinfo.value) + assert excinfo.value.code == 40000 assert len(captured_requests) == 0 @@ -166,16 +160,13 @@ async def test_rsh1a_rejects_null_recipient(): ) client = rest_client(mock_http) - # DEVIATION: the spec asserts error.code == 40000. PushAdmin.publish rejects a - # non-dict recipient with a plain TypeError, which carries no Ably error code. - with pytest.raises(TypeError) as excinfo: + with pytest.raises(AblyException) as excinfo: await client.push.admin.publish( recipient=None, data={'notification': {'title': 'Test'}}, ) - assert not isinstance(excinfo.value, AblyException) - assert 'recipient' in str(excinfo.value) + assert excinfo.value.code == 40000 assert len(captured_requests) == 0 diff --git a/test/uts/rest/unit/request_test.py b/test/uts/rest/unit/request_test.py index 3aa76313..a1bd7a47 100644 --- a/test/uts/rest/unit/request_test.py +++ b/test/uts/rest/unit/request_test.py @@ -11,6 +11,7 @@ import msgpack import pytest +from ably.pubsub.util.exceptions import AblyException from test.uts.helpers.client import rest_client from test.uts.helpers.deviations import deviation from test.uts.helpers.mock_http import MockHttpClient @@ -426,14 +427,11 @@ async def test_rsc19e_network_error_propagated(): client = rest_client(mock_http, key=SPEC_KEY, fallback_hosts=[]) - # DEVIATION: the spec expects an error carrying code 80000, or a message naming the - # network/connection failure. request() is not wrapped by @catch_all (unlike time() - # and stats()), so the transport exception reaches the caller unwrapped. Its message - # does name the connection failure, which is what RSC19e asks be indicated. - with pytest.raises(httpx.ConnectError) as excinfo: + with pytest.raises(AblyException) as excinfo: await client.request('GET', '/test', version='3') - assert 'Connection refused' in str(excinfo.value) + assert excinfo.value.code == 80000 + assert isinstance(excinfo.value.cause, httpx.ConnectError) # UTS: rest/unit/RSC19e/timeout-error-handling-1 @@ -448,13 +446,11 @@ async def test_rsc19e_timeout_error_handling(): client = rest_client(mock_http, key=SPEC_KEY, http_request_timeout=1, fallback_hosts=[]) - # DEVIATION: the spec expects an error carrying code 50003, or a message naming the - # timeout. request() is not wrapped by @catch_all, so the transport exception reaches - # the caller unwrapped; its message does name the timeout. - with pytest.raises(httpx.ReadTimeout) as excinfo: + with pytest.raises(AblyException) as excinfo: await client.request('GET', '/test', version='3') - assert 'timed out' in str(excinfo.value) + assert excinfo.value.code == 50003 + assert isinstance(excinfo.value.cause, httpx.TimeoutException) # UTS: rest/unit/RSC19e/http-error-no-fallback-2