From 5b5dfb692de909ac068cd2a1a2e8e74c734a177d Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 18:54:40 +0000 Subject: [PATCH 1/4] pubsub: pass the status code and the error code the right way round AblyException takes (message, status_code, code), and ten of the errors the library raises itself passed the two the other way round, so a caller reading .code got the HTTP status and .status_code got the Ably error code: detaching a FAILED channel gave status_code 90001 and code 400. The ten are the detach errors in RealtimeChannel.detach (RTL5b, RTL4h), the default errors for a failed queued message and for a NACK with no error, the vcdiff decoder errors in mixins.py and defaultvcdiffdecoder.py (PC3, RTL18), and the two ChannelOptions type checks. Each now passes the HTTP status first, which also means is_server_error is True for the two 5xx errors and False for the rest, as it should be. This is a breaking change: code that compares code or status_code against the transposed values must compare against the corrected ones, which UPDATING.md lists. The two adapted UTS tests that pinned the transposed status now assert both fields, and the deviations.md entry and candidate issue 2.9, which named six of the ten sites, are removed. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 17 +++++++++ ably/pubsub/realtime/channel.py | 4 +- ably/pubsub/realtime/connectionmanager.py | 4 +- ably/pubsub/types/channeloptions.py | 4 +- ably/pubsub/types/mixins.py | 6 +-- ably/pubsub/vcdiff/defaultvcdiffdecoder.py | 2 +- .../realtime/realtimechannel_publish_test.py | 2 +- test/uts/deviations.md | 38 ------------------- .../unit/channels/channel_attach_test.py | 5 +-- .../unit/channels/channel_detach_test.py | 5 +-- 10 files changed, 32 insertions(+), 55 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 64f69d56..867baebd 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -124,6 +124,23 @@ 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. + ## 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/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..15213db8 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( @@ -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/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/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/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/uts/deviations.md b/test/uts/deviations.md index 28535f9f..fb6c9f07 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2041,34 +2041,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, @@ -2901,16 +2873,6 @@ no attribute 'key_name'`) reaches `Connection#errorReason` with no `code` at all 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. - ### Tier 3 — silently wrong data **3.1 The clientId is sent on outgoing PresenceMessages where the spec requires it absent.** 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 From 4a07522a549695d76c0426d2e25b0165f5770bb2 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 19:02:03 +0000 Subject: [PATCH 2/4] pubsub: raise AblyException for network errors and timeouts Http.make_request re-raised whatever httpx.AsyncClient.send raised once the last host had been tried, so a request that got no response leaked httpx.ConnectError, httpx.ReadTimeout and the rest from every public method built on it: request, publish, history, presence, push admin, batch publish, auth.authorize and request_token among them. Only time() and stats() wrapped them, through catch_all, as a generic 50000. make_request now wraps an httpx.RequestError as an AblyException, 50003 with status 504 for a timeout and 80000 with status 500 for any other network error, carrying the httpx exception as cause and as __cause__ (RSC19e, TI1). The 504/50003 pair is the one the library already uses for a connect or ping that times out. A timeout or network error reaching an auth_url, which goes through its own httpx call, raises 40170 with status 401 and the same cause, as RSA4e requires. catch_all now re-raises an AblyException as it is, rather than chaining it to itself and losing the cause. On a realtime client, a token request that fails this way reaches on_error_from_authorize as an AblyException, so the connection goes DISCONNECTED with an 80019 (RSA4c) rather than recording the httpx exception as its error_reason. This is a breaking change: callers catching httpx exceptions must catch AblyException and read its code, or its cause for the httpx detail. The RSC19e UTS tests and the legacy timeout test now assert the wrapped error; the latter accepts any httpx timeout as the cause, since its connect and read budgets are both live. The RSC19e row in deviations.md is removed. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 27 ++++++++ ably/pubsub/http/auth.py | 10 ++- ably/pubsub/request/http.py | 13 ++++ ably/pubsub/util/exceptions.py | 3 + test/ably/http/httphttp_test.py | 8 +-- test/ably/http/httprequest_test.py | 9 ++- test/unit/http_test.py | 65 ++++++++++++++++++- test/uts/deviations.md | 3 +- .../integration/proxy/rest_fallback_test.py | 2 +- test/uts/rest/unit/request_test.py | 18 ++--- 10 files changed, 135 insertions(+), 23 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 867baebd..d75089aa 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -141,6 +141,33 @@ HTTP status in `code`. They now carry them the right way round: 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: + ... +``` + ## 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..021c8d8b 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: @@ -410,8 +412,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/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/util/exceptions.py b/ably/pubsub/util/exceptions.py index 245d4a43..f961eff9 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 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/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/unit/http_test.py b/test/unit/http_test.py index ebae07b2..b679855b 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,41 @@ 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() diff --git a/test/uts/deviations.md b/test/uts/deviations.md index fb6c9f07..5ad9f7ba 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2336,7 +2336,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 | @@ -2355,7 +2355,6 @@ pseudocode is not mistaken for non-compliance. | 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 | | 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 | 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/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 From ab9c834b30f362653fbc22ce2304f835aa67eed1 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 19:06:58 +0000 Subject: [PATCH 3/4] pubsub: keep an AblyException raised by an auth_callback as it is Auth.request_token wrapped every exception an auth_callback raised as 401/40170, so the statusCode a callback gave was lost. RSA4d fails the connection when the auth provider answers 403, and ConnectionManager.on_error_from_authorize branches on status 403 to do so, but an auth_callback could never reach that branch: a 403 went DISCONNECTED with an 80019/401 and the client kept retrying. RSA4e only asks for the 40170 wrapping of errors that are not already an ErrorInfo. request_token now re-raises an AblyException from the callback unchanged and still wraps anything else as 40170. Auth.authorize, which also serves the RTN22 reauth a server AUTH triggers, hands a 403 to on_error_from_authorize on a realtime client, so a 403 fails the connection whether it comes while connecting, on a reauth or from an explicit authorize() (RSA4d1). Both 80019s on_error_from_authorize builds now carry the error that caused them (RSA4c1, RSA4d). The empty token string error's code, 4017, is corrected to 40170. This is a breaking change: a caller that identified its own callback's failures by code 40170 receives the callback's AblyException instead, and a realtime client whose callback raises a 403 ends FAILED rather than retrying. The four RSA4d UTS tests are un-gated, the two RSA4c tests that pinned the missing cause now assert it, and deviations.md drops the RSA4d entry and the cause half of the RSA4c adaptation, with its counts adjusted. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 23 ++++ ably/pubsub/http/auth.py | 14 ++- ably/pubsub/realtime/connectionmanager.py | 4 +- test/unit/http_test.py | 16 +++ test/uts/deviations.md | 102 ++++++------------ .../unit/auth/auth_callback_errors_test.py | 8 +- .../unit/auth/connection_auth_test.py | 8 +- 7 files changed, 89 insertions(+), 86 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index d75089aa..272ec337 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -168,6 +168,29 @@ except AblyException as e: ... ``` +### 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) +``` + ## 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 021c8d8b..aed3db26 100644 --- a/ably/pubsub/http/auth.py +++ b/ably/pubsub/http/auth.py @@ -153,7 +153,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 @@ -186,6 +193,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: @@ -214,7 +224,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) diff --git a/ably/pubsub/realtime/connectionmanager.py b/ably/pubsub/realtime/connectionmanager.py index 15213db8..bd9a7dcd 100644 --- a/ably/pubsub/realtime/connectionmanager.py +++ b/ably/pubsub/realtime/connectionmanager.py @@ -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: diff --git a/test/unit/http_test.py b/test/unit/http_test.py index b679855b..c5be5803 100644 --- a/test/unit/http_test.py +++ b/test/unit/http_test.py @@ -149,3 +149,19 @@ async def test_realtime_token_request_network_errors_reach_the_connection_wrappe 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/uts/deviations.md b/test/uts/deviations.md index 5ad9f7ba..24035aea 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`. @@ -2158,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 @@ -2861,16 +2826,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.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 @@ -3692,8 +3654,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 From 9a2e03773fec89c8554529b8c4edd42338aad068 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 19:12:19 +0000 Subject: [PATCH 4/4] pubsub: raise a 40106 AblyException for invalid client arguments RSC1b requires invalid constructor arguments, such as no key, no token and no means to create one, to give an error with code 40106. The HTTP client and Auth raised a bare ValueError instead, which has no code, for no credentials, a key alongside key_name or key_secret, a token_details that is not a TokenDetails and use_token_auth=False with no key; the realtime factory shares them. push.admin.publish likewise raised TypeError or ValueError, with no code, for a recipient or data that is empty or not a dict, where RSH1a's tests expect 40000. These now raise InvalidArgumentException, an AblyException that is also a ValueError so that `except ValueError` keeps working: 40106 with status 401 for the credential errors, and 40000 with status 400 for push publish. The REC1b and REC1c option conflicts already raised 40106 with status 400 and keep that status, now as the same subclass. The status codes follow the evidence. RSC1b and the UTS specs give the code only. ably-common's errors/codes/40106.md says 40106 "is reported with code 40106 and HTTP status 401". ably-js raises 40106/401 for no credentials (auth.ts, both sites) and 40106/400 for the REC1b and REC1c conflicts (defaults.ts), which is exactly the split adopted here. ably-java raises 400/40106 only from requestToken and 400/40000 from its constructor; ably-dotnet raises 40106 with status 404; ably-go never raises 40106 and reports a missing key as 40101/401; ably-cocoa raises an NSException with no code. So ably-js, the reference SDK, and the code's own documentation agree on 401 for missing credentials, and ably-js on 400 for the option conflicts. This is a breaking change: code catching TypeError from push.admin.publish must catch AblyException or ValueError. Code catching ValueError from the factories is unaffected, and can now read the code. The RSC1b, RTC12 and RSH1a UTS tests assert the spec's codes, and their rows in deviations.md are removed or, for RSH1a, marked compliant. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 31 +++++++++++++++++++ ably/pubsub/http/auth.py | 14 ++++++--- ably/pubsub/http/http.py | 12 ++++--- ably/pubsub/http/push.py | 9 +++--- ably/pubsub/server/__init__.py | 14 ++++----- ably/pubsub/server/sync.py | 7 ++--- ably/pubsub/types/options.py | 8 ++--- ably/pubsub/util/exceptions.py | 8 +++++ test/ably/http/httppush_test.py | 4 +-- test/unit/options_test.py | 17 ++++++++++ test/uts/deviations.md | 3 +- .../unit/client/realtime_client_test.py | 8 ++--- test/uts/rest/integration/push_admin_test.py | 13 ++++---- test/uts/rest/unit/auth/auth_scheme_test.py | 10 +++--- .../rest/unit/push/push_admin_publish_test.py | 21 ++++--------- 15 files changed, 113 insertions(+), 66 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 272ec337..9865dc14 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -191,6 +191,37 @@ async def auth_callback(token_params): 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 aed3db26..f4a2a713 100644 --- a/ably/pubsub/http/auth.py +++ b/ably/pubsub/http/auth.py @@ -18,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"] @@ -62,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 @@ -85,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 = {} 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/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/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 f961eff9..a1572d58 100644 --- a/ably/pubsub/util/exceptions.py +++ b/ably/pubsub/util/exceptions.py @@ -111,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/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/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 24035aea..96654a0d 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2312,12 +2312,11 @@ 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 | | RSC15a | Six hosts tried | Three. `Options.__get_hosts` truncates to `http_max_retry_count`, which TO3l5 sanctions | Intentional | 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/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