From 95cbbcc0254728dfe4ec8ce4b721cdf7a12c0aea Mon Sep 17 00:00:00 2001 From: Isaac Doidge Date: Wed, 4 Nov 2026 01:56:56 +1000 Subject: [PATCH 1/4] feat(client)!: typed HTTP errors, a configurable timeout and opt-in GET retries A response with a status other than 200 raises an ApiError whose message is the `message` field of the node's JSON error body, falling back to the body and then to the HTTP reason phrase. The error also keeps the response headers and body. 404, 408, 410, 429 and 503 raise subclasses of ApiError: ApiNotFoundError, ApiRequestTimeoutError, ApiGoneError (with oldest_block, from the x-supra-oldest-block header), ApiRateLimitError (with retry_after_seconds, from the body's retry_after_seconds or the Retry-After header) and ApiServiceUnavailableError. ApiClientConfig gains two keyword-only fields. `timeout` (seconds, default 60) limits connecting, reading, writing and waiting for a pooled connection; the pool wait had no limit before. `retry` takes a RetryPolicy that repeats GET requests after a 408, 429 or 503 response or a timeout, up to a bounded number of attempts, waiting for the time the response asks for (up to a cap) or an exponential backoff. It defaults to None, so no request is repeated unless the caller sets it. POST requests are never repeated. A request that exceeds the timeout raises ClientTimeoutError, a subclass of httpx.TimeoutException. A view response without `result` raises UnexpectedResponseError, which names the field and subclasses both ApiError and KeyError. The new classes and RetryPolicy are exported from supra_sdk.clients. BREAKING CHANGE: ApiError.message is the node's error message instead of the whole response body; the body is ApiError.body. A request timeout raises ClientTimeoutError instead of the specific httpx timeout class (ReadTimeout, ConnectTimeout, ...); `except httpx.TimeoutException` still matches it. Co-Authored-By: Claude Opus 5.5 --- README.md | 31 ++ supra_sdk/clients/__init__.py | 30 +- supra_sdk/clients/api_client.py | 468 ++++++++++++++++++++++++- supra_sdk/clients/rest/general.py | 11 +- supra_sdk/clients/rest/rest_client.py | 21 ++ supra_sdk/clients/rest/supra_client.py | 6 +- tests/unit/test_api_errors.py | 400 +++++++++++++++++++++ tests/unit/test_request_urls.py | 2 +- 8 files changed, 944 insertions(+), 25 deletions(-) create mode 100644 tests/unit/test_api_errors.py diff --git a/README.md b/README.md index 2260951..9b84692 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,37 @@ python -m pip install supra-sdk Check [examples](examples/README.md) section. +### Errors, timeouts and retries + +A response with a status other than 200 raises `ApiError` (from `supra_sdk.clients`), whose `message` is the node's +error message, `status_code` the HTTP status, `headers` the response headers and `body` the response body. These +statuses raise a subclass of `ApiError`: + +| Status | Class | Extra attribute | +|---|---|---| +| 404 | `ApiNotFoundError` | | +| 408 | `ApiRequestTimeoutError` | | +| 410 | `ApiGoneError` | `oldest_block`, from the `x-supra-oldest-block` header | +| 429 | `ApiRateLimitError` | `retry_after_seconds`, from the body or the `Retry-After` header | +| 503 | `ApiServiceUnavailableError` | | + +A 200 response without a field the SDK reads raises `UnexpectedResponseError`, which is also a `KeyError`. A request +that exceeds the timeout raises `ClientTimeoutError`, which is a `httpx.TimeoutException`. + +Each phase of a request (connecting, waiting for a pooled connection, writing, reading) is limited to 60 seconds; +`timeout` changes the limit. GET requests are repeated after a 408, 429 or 503 response or a timeout only when +`retry` is set. POST requests and the faucet request are never repeated. + +```python +from supra_sdk.clients import RetryPolicy +from supra_sdk.clients.rest import SupraClient, SupraClientConfig + +client = SupraClient( + "https://rpc-testnet.supra.com", + SupraClientConfig(timeout=30, retry=RetryPolicy(attempts=3, max_delay_seconds=10)), +) +``` + ## Reference Docs For the function specific documentation, check out the `supra-python-sdk` [reference documentation](https://supra-python-sdk.docs.supra.com/). diff --git a/supra_sdk/clients/__init__.py b/supra_sdk/clients/__init__.py index defffb6..14f3f8d 100644 --- a/supra_sdk/clients/__init__.py +++ b/supra_sdk/clients/__init__.py @@ -1,8 +1,34 @@ # Copyright © Supra # SPDX-License-Identifier: Apache-2.0 -from supra_sdk.clients.api_client import ApiClient, ApiClientConfig +from supra_sdk.clients.api_client import ( + ApiClient, + ApiClientConfig, + ApiError, + ApiGoneError, + ApiNotFoundError, + ApiRateLimitError, + ApiRequestTimeoutError, + ApiServiceUnavailableError, + ClientTimeoutError, + RetryPolicy, + UnexpectedResponseError, +) from supra_sdk.clients.supra_token import SupraTokenClient from supra_sdk.clients.supra_token_object import SupraTokenObjectClient -__all__ = ["ApiClient", "ApiClientConfig", "SupraTokenClient", "SupraTokenObjectClient"] +__all__ = [ + "ApiClient", + "ApiClientConfig", + "ApiError", + "ApiGoneError", + "ApiNotFoundError", + "ApiRateLimitError", + "ApiRequestTimeoutError", + "ApiServiceUnavailableError", + "ClientTimeoutError", + "RetryPolicy", + "SupraTokenClient", + "SupraTokenObjectClient", + "UnexpectedResponseError", +] diff --git a/supra_sdk/clients/api_client.py b/supra_sdk/clients/api_client.py index e30dbeb..45cb820 100644 --- a/supra_sdk/clients/api_client.py +++ b/supra_sdk/clients/api_client.py @@ -1,8 +1,15 @@ # Copyright © Supra # SPDX-License-Identifier: Apache-2.0 +import asyncio import json -from dataclasses import dataclass +import math +import re +import time +from collections.abc import Mapping +from dataclasses import dataclass, field +from datetime import UTC +from email.utils import parsedate_to_datetime from http import HTTPStatus from typing import Any @@ -10,6 +17,74 @@ from supra_sdk.metadata import Metadata +DEFAULT_TIMEOUT_SECONDS = 60.0 +"""Default limit, in seconds, of each phase of a request: connecting, reading, writing and waiting for a connection +from the pool.""" + +OLDEST_BLOCK_HEADER = "x-supra-oldest-block" +"""Response header in which the node reports the height of the oldest block it still stores.""" + +# Statuses after which a GET is repeated when `ApiClientConfig.retry` is set. +_RETRYABLE_STATUSES = frozenset( + { + HTTPStatus.REQUEST_TIMEOUT, + HTTPStatus.TOO_MANY_REQUESTS, + HTTPStatus.SERVICE_UNAVAILABLE, + } +) + +_DECIMAL = re.compile(r"[0-9]+") + +# Indirection so that tests can replace the wait between attempts. +_sleep = asyncio.sleep + + +@dataclass(frozen=True) +class RetryPolicy: + """Policy for repeating GET requests that failed in a way that a later attempt can succeed. + + A GET request is repeated after a response with status 408, 429 or 503 and after a request timeout, until + `attempts` requests in total have been sent. The wait before a repeat is the number of seconds the response asks + for (the `retry_after_seconds` field of the body, otherwise the `Retry-After` header). When the response asks for + nothing, the wait is `base_delay_seconds` doubled after each attempt. No wait is longer than `max_delay_seconds`: + a response that asks for a longer wait is not repeated, and its error is raised at once. POST requests are never + repeated, and neither is the faucet request. + + Attributes: + attempts (int): Maximum number of requests sent, the first included. At least 1. Default to 3. + max_delay_seconds (float): Longest wait, in seconds, before a repeat. Default to 10. + base_delay_seconds (float): Wait, in seconds, before the first repeat when the response does not ask for one. + Default to 0.25. + + Raises: + ValueError: If `attempts` is not an integer of at least 1, or a delay is negative or not finite. + + """ + + attempts: int = 3 + max_delay_seconds: float = 10.0 + base_delay_seconds: float = 0.25 + + def __post_init__(self): + if ( + isinstance(self.attempts, bool) + or not isinstance(self.attempts, int) + or self.attempts < 1 + ): + raise ValueError( + f"attempts must be an integer of at least 1, got {self.attempts!r}" + ) + for name in ("max_delay_seconds", "base_delay_seconds"): + value = getattr(self, name) + if not _is_non_negative_number(value): + raise ValueError( + f"{name} must be a finite number of at least 0, got {value!r}" + ) + + def backoff_seconds(self, attempt: int) -> float: + """The wait before repeating attempt number `attempt` (counted from 1) when the response asks for none.""" + return min(self.max_delay_seconds, self.base_delay_seconds * 2 ** (attempt - 1)) + @dataclass class ApiClientConfig: @@ -18,11 +93,18 @@ class ApiClientConfig: Attributes: http2 (bool): Whether to use HTTP/2 for requests. Default to False. access_token (Optional[str]): Optional access token (JWT) for authentication. Default to None. + timeout (float): Limit, in seconds, of each phase of a request: connecting, reading, writing and waiting for a + free connection from the pool. A request that exceeds it raises `ClientTimeoutError`. Keyword only. + Default to 60. + retry (RetryPolicy | None): Policy for repeating GET requests after a 408, 429 or 503 response or a timeout. + Keyword only. Default to None: no request is repeated. """ http2: bool = False access_token: str | None = None + timeout: float = field(default=DEFAULT_TIMEOUT_SECONDS, kw_only=True) + retry: RetryPolicy | None = field(default=None, kw_only=True) def raise_if_access_token_not_exists(self): """Raise an error if the access token is not set. @@ -69,7 +151,8 @@ def __init__( Default to None, which uses httpx's default network transport. Raises: - ValueError: If `base_url` contains a query string or a fragment. + ValueError: If `base_url` contains a query string or a fragment, or `api_client_config.timeout` is not a + finite number greater than 0. """ if "?" in base_url or "#" in base_url: @@ -77,9 +160,14 @@ def __init__( f"base_url must not contain a query string or a fragment: {base_url!r}" ) api_client_config = api_client_config or ApiClientConfig() + timeout_seconds = api_client_config.timeout + if not _is_non_negative_number(timeout_seconds) or timeout_seconds == 0: + raise ValueError( + f"timeout must be a finite number of seconds greater than 0, got {timeout_seconds!r}" + ) self.base_url = base_url if base_url.endswith("/") else base_url + "/" limits = httpx.Limits() - timeout = httpx.Timeout(60.0, pool=None) + timeout = httpx.Timeout(timeout_seconds) headers = {Metadata.SUPRA_HEADER: Metadata.get_supra_header_val()} self._client = httpx.AsyncClient( http2=api_client_config.http2, @@ -112,19 +200,30 @@ async def get( headers: dict[str, str] | None = None, params: dict[str, Any] | None = None, strict_mode: bool | None = True, + retryable: bool = True, ) -> httpx.Response: """Performs an asynchronous GET request. + When `ApiClientConfig.retry` is set and `retryable` is true, the request is repeated under that policy after a + 408, 429 or 503 response or a timeout (see `RetryPolicy`). The response of the last attempt is returned or, in + strict mode, raised as an error. + Args: endpoint (str): Endpoint to call. headers (dict[str, str] | None): Optional headers. Default to None. params (dict[str, Any] | None): Optional query parameters. Default to None. strict_mode (bool | None): If enabled then raises an error when request response status code is not 200. Default to True. + retryable (bool): Whether the request may be repeated under `ApiClientConfig.retry`. Pass False for a + request that has an effect on the node. Default to True. Returns: httpx.Response: The response from the server. + Raises: + ApiError: In strict mode, if the status code is not 200. The subclass names the status: see `ApiError`. + ClientTimeoutError: If the last attempt exceeded `ApiClientConfig.timeout`. + """ params = params or {} params = {key: val for key, val in params.items() if val is not None} @@ -133,11 +232,41 @@ async def get( if self.api_client_config.access_token: headers["Authorization"] = f"Bearer {self.api_client_config.access_token}" - response = await self._client.get( - url=self._url(endpoint), params=params, headers=headers - ) + url = self._url(endpoint) + policy = self.api_client_config.retry if retryable else None + attempts = policy.attempts if policy else 1 + attempt = 1 + while True: + last = attempt >= attempts + try: + response = await self._client.get( + url=url, params=params, headers=headers + ) + except httpx.TimeoutException as error: + if last or policy is None: + raise self._timeout_error(url, error) from error + await _sleep(policy.backoff_seconds(attempt)) + attempt += 1 + continue + if ( + last + or policy is None + or response.status_code not in _RETRYABLE_STATUSES + ): + break + delay = policy.backoff_seconds(attempt) + requested = retry_after_seconds( + _json_or_none(response.text), response.headers + ) + if requested is not None: + if requested > policy.max_delay_seconds: + break + delay = requested + await _sleep(delay) + attempt += 1 + if strict_mode and response.status_code != HTTPStatus.OK: - raise ApiError(response.text, response.status_code) + raise api_error_from_response(response) return response async def post( @@ -150,6 +279,8 @@ async def post( ) -> httpx.Response: """Performs an asynchronous POST request. + A POST request is sent once: it is never repeated under `ApiClientConfig.retry`. + Args: endpoint (str): API endpoint. params (dict[str, Any] | None): Query parameters. Default to None. @@ -161,6 +292,10 @@ async def post( Returns: httpx.Response: The response from the server. + Raises: + ApiError: In strict mode, if the status code is not 200. The subclass names the status: see `ApiError`. + ClientTimeoutError: If the request exceeded `ApiClientConfig.timeout`. + """ params = params or {} params = {key: val for key, val in params.items() if val is not None} @@ -173,45 +308,346 @@ async def post( else: content = data - response = await self._client.post( - url=self._url(endpoint), - params=params, - headers=headers, - content=content, - ) + url = self._url(endpoint) + try: + response = await self._client.post( + url=url, + params=params, + headers=headers, + content=content, + ) + except httpx.TimeoutException as error: + raise self._timeout_error(url, error) from error if strict_mode and response.status_code != HTTPStatus.OK: - raise ApiError(response.text, response.status_code) + raise api_error_from_response(response) return response + def _timeout_error( + self, url: str, error: httpx.TimeoutException + ) -> "ClientTimeoutError": + return ClientTimeoutError( + url, self.api_client_config.timeout, type(error).__name__ + ) + class ApiError(Exception): """Exception raised when the API returns a non-200 response. + It is the base class of the errors for specific statuses, so `except ApiError` matches every error response: + + - `ApiNotFoundError`: 404. + - `ApiRequestTimeoutError`: 408. + - `ApiGoneError`: 410, with `oldest_block`. + - `ApiRateLimitError`: 429, with `retry_after_seconds`. + - `ApiServiceUnavailableError`: 503. + - `UnexpectedResponseError`: a 200 response whose body does not have the expected shape. + + A request that exceeds the client's timeout raises `ClientTimeoutError`, which is not an `ApiError`. + Attributes: - message (str): The response body. + message (str): The `message` field of the node's JSON error body. When the body has none, the response body + itself, and when the body is empty, the HTTP reason phrase of the status. status_code (int): The HTTP status code returned. + headers (httpx.Headers): The response headers (case-insensitive). Empty when the error was not built from a + response. + body (str): The response body as text. """ message: str status_code: int + headers: httpx.Headers + body: str - def __init__(self, message: str, status_code: int): + def __init__( + self, + message: str, + status_code: int, + headers: Mapping[str, str] | None = None, + body: str | None = None, + ): """Initialize the exception with message and response status code. Args: message (str): Error message. status_code (int): The HTTP status code returned. + headers (Mapping[str, str] | None): The response headers. Default to None (no headers). + body (str | None): The response body. Default to None, which uses `message`. """ self.message = message self.status_code = status_code + self.headers = httpx.Headers(headers) + self.body = message if body is None else body super().__init__(f"{{message: {message}, status_code: {status_code}}}") +class ApiNotFoundError(ApiError): + """Exception raised when the API returns 404: the requested account, resource, module, transaction or route is + not available on the node. + + Nodes before `supra_node_v12.0.0` answer a missing account or resource with 400 instead, which raises `ApiError`. + """ + + +class ApiRequestTimeoutError(ApiError): + """Exception raised when the API returns 408: the server, or a gateway in front of it, stopped waiting for the + request. A GET can be repeated, which `RetryPolicy` does when set.""" + + +class ApiGoneError(ApiError): + """Exception raised when the API returns 410: the requested data existed, but the node has pruned it. + + Attributes: + oldest_block (int | None): Height of the oldest block the node still stores, from the `x-supra-oldest-block` + response header; a request starting from that height can be answered. `None` when the header is absent or + not a decimal integer. + + """ + + oldest_block: int | None + + def __init__( + self, + message: str, + status_code: int, + headers: Mapping[str, str] | None = None, + body: str | None = None, + ): + """Initialize the exception and read `oldest_block` from the headers. + + Args: + message (str): Error message. + status_code (int): The HTTP status code returned. + headers (Mapping[str, str] | None): The response headers. Default to None (no headers). + body (str | None): The response body. Default to None, which uses `message`. + + """ + super().__init__(message, status_code, headers, body) + value = self.headers.get(OLDEST_BLOCK_HEADER) + self.oldest_block = ( + int(value) + if value is not None and _DECIMAL.fullmatch(value.strip()) + else None + ) + + +class ApiRateLimitError(ApiError): + """Exception raised when the API returns 429: the caller sent too many requests, for example faucet requests + for the same account. + + Attributes: + retry_after_seconds (float | None): Number of seconds the server asks the caller to wait: the + `retry_after_seconds` field of the JSON body, otherwise the `Retry-After` header (seconds or an HTTP + date). `None` when the response gives neither. + + """ + + retry_after_seconds: float | None + + def __init__( + self, + message: str, + status_code: int, + headers: Mapping[str, str] | None = None, + body: str | None = None, + ): + """Initialize the exception and read `retry_after_seconds` from the body and the headers. + + Args: + message (str): Error message. + status_code (int): The HTTP status code returned. + headers (Mapping[str, str] | None): The response headers. Default to None (no headers). + body (str | None): The response body. Default to None, which uses `message`. + + """ + super().__init__(message, status_code, headers, body) + self.retry_after_seconds = retry_after_seconds( + _json_or_none(self.body), self.headers + ) + + +class ApiServiceUnavailableError(ApiError): + """Exception raised when the API returns 503: the node cannot serve the request at the moment, for example + because its transaction pool is full. The request can be repeated later.""" + + +class UnexpectedResponseError(ApiError, KeyError): + """Exception raised when a 200 response does not have the shape the SDK expects, such as a missing field. + + It is also a `KeyError`, which the SDK raised for a missing field before this class existed. + + Attributes: + field (str): The field the SDK expected in the response body. + + """ + + field: str + + def __init__(self, field: str, response: httpx.Response): + """Initialize the exception from the response that lacks `field`. + + Args: + field (str): The field the SDK expected in the response body. + response (httpx.Response): The response. + + """ + self.field = field + super().__init__( + f"The response of {_request_url(response)} has no {field!r} field", + response.status_code, + response.headers, + response.text, + ) + + def __str__(self) -> str: + # `KeyError.__str__` would quote the message. + return Exception.__str__(self) + + +class ClientTimeoutError(httpx.TimeoutException): + """Exception raised when a request exceeds `ApiClientConfig.timeout` in any phase: connecting, waiting for a + connection from the pool, writing or reading. + + It is a `httpx.TimeoutException`, the error raised before this class existed, and not an `ApiError`: the server + sent no response. + + Attributes: + url (str): The request URL, without query parameters. + timeout_seconds (float): The timeout that was exceeded. + phase (str): Name of the `httpx` timeout that was raised: `ConnectTimeout`, `PoolTimeout`, `WriteTimeout` or + `ReadTimeout`. + + """ + + url: str + timeout_seconds: float + phase: str + + def __init__(self, url: str, timeout_seconds: float, phase: str): + """Initialize the exception with the request URL, the timeout and the phase that timed out. + + Args: + url (str): The request URL, without query parameters. + timeout_seconds (float): The timeout that was exceeded. + phase (str): Name of the `httpx` timeout that was raised. + + """ + self.url = url + self.timeout_seconds = timeout_seconds + self.phase = phase + super().__init__( + f"Request to {url} timed out after {timeout_seconds} seconds ({phase})" + ) + + class AuthorizationKeyNotSpecifiedError(Exception): """Exception raised when consensus api endpoints are accessed without defining `access_token` in `ClientConfig`.""" def __init__(self): """Initialize the exception with a default message.""" super().__init__("Authorization key is not specified") + + +_ERRORS_BY_STATUS: dict[int, type[ApiError]] = { + HTTPStatus.NOT_FOUND: ApiNotFoundError, + HTTPStatus.REQUEST_TIMEOUT: ApiRequestTimeoutError, + HTTPStatus.GONE: ApiGoneError, + HTTPStatus.TOO_MANY_REQUESTS: ApiRateLimitError, + HTTPStatus.SERVICE_UNAVAILABLE: ApiServiceUnavailableError, +} + + +def api_error_from_response(response: httpx.Response) -> ApiError: + """Builds the error for a non-200 response: the subclass of `ApiError` for its status, or `ApiError` itself. + + Args: + response (httpx.Response): The response. + + Returns: + ApiError: The error, with the node's `message`, the status code, the headers and the body. + + """ + body = response.text + error_class = _ERRORS_BY_STATUS.get(response.status_code, ApiError) + return error_class( + _error_message(body, response.status_code), + response.status_code, + response.headers, + body, + ) + + +def retry_after_seconds(data: Any, headers: Mapping[str, str]) -> float | None: + """The number of seconds an error response asks the caller to wait before repeating the request. + + Args: + data (Any): The parsed JSON body, or None. Its `retry_after_seconds` field, a non-negative number or a + decimal string, is used when present. + headers (Mapping[str, str]): The response headers. Their `Retry-After` value, delta-seconds or an HTTP date, + is used when the body has no `retry_after_seconds`. + + Returns: + float | None: The number of seconds, or None when the response asks for no wait. + + """ + if isinstance(data, dict): + value = data.get("retry_after_seconds") + if isinstance(value, str): + try: + value = float(value) + except ValueError: + value = None + if isinstance(value, int | float) and _is_non_negative_number(value): + return float(value) + + header = httpx.Headers(headers).get("retry-after") + if header is None: + return None + header = header.strip() + if _DECIMAL.fullmatch(header): + return float(header) + try: + date = parsedate_to_datetime(header) + except (TypeError, ValueError): + return None + if date.tzinfo is None: + date = date.replace(tzinfo=UTC) + return float(max(0, math.ceil(date.timestamp() - time.time()))) + + +def _error_message(body: str, status_code: int) -> str: + data = _json_or_none(body) + if isinstance(data, dict): + message = data.get("message") + if isinstance(message, str) and message: + return message + if body: + return body + try: + return HTTPStatus(status_code).phrase + except ValueError: + return f"HTTP {status_code}" + + +def _json_or_none(text: str) -> Any: + try: + return json.loads(text) + except ValueError: + return None + + +def _is_non_negative_number(value: Any) -> bool: + return ( + isinstance(value, int | float) + and not isinstance(value, bool) + and math.isfinite(value) + and value >= 0 + ) + + +def _request_url(response: httpx.Response) -> str: + try: + return str(response.request.url.copy_with(query=None)) + except RuntimeError: + return "the request" diff --git a/supra_sdk/clients/rest/general.py b/supra_sdk/clients/rest/general.py index 8584774..f5c7a2b 100644 --- a/supra_sdk/clients/rest/general.py +++ b/supra_sdk/clients/rest/general.py @@ -49,16 +49,19 @@ async def view( Returns: list[Any]: Execution results of the view function. + Raises: + UnexpectedResponseError: If the node answers 200 with a body that has no `result` field. + """ data = { "function": function, "type_arguments": type_arguments, "arguments": arguments, } - res_data = ( - await self.api_client.post(endpoint=VIEW_FUNCTION_ENDPOINT, data=data) - ).json() - return res_data["result"] + response = await self.api_client.post( + endpoint=VIEW_FUNCTION_ENDPOINT, data=data + ) + return self._response_field(response, "result") async def get_table_item( self, diff --git a/supra_sdk/clients/rest/rest_client.py b/supra_sdk/clients/rest/rest_client.py index 85a31c2..c1c97a4 100644 --- a/supra_sdk/clients/rest/rest_client.py +++ b/supra_sdk/clients/rest/rest_client.py @@ -1,7 +1,12 @@ # Copyright © Supra # SPDX-License-Identifier: Apache-2.0 +from typing import Any + +import httpx + from supra_sdk.clients import ApiClient +from supra_sdk.clients.api_client import UnexpectedResponseError class RestClient: @@ -25,3 +30,19 @@ def __init__(self, api_client: ApiClient): """ self.api_client = api_client + + @staticmethod + def _response_field(response: httpx.Response, field: str) -> Any: + """Returns `field` of the JSON object in the body of `response`. + + Raises: + UnexpectedResponseError: If the body is not a JSON object with `field`. + + """ + try: + data = response.json() + except ValueError: + raise UnexpectedResponseError(field, response) from None + if not isinstance(data, dict) or field not in data: + raise UnexpectedResponseError(field, response) + return data[field] diff --git a/supra_sdk/clients/rest/supra_client.py b/supra_sdk/clients/rest/supra_client.py index bf48767..2f19309 100644 --- a/supra_sdk/clients/rest/supra_client.py +++ b/supra_sdk/clients/rest/supra_client.py @@ -223,6 +223,8 @@ def __init__( api_client_config = ApiClientConfig( http2=supra_client_config.http2, access_token=supra_client_config.access_token, + timeout=supra_client_config.timeout, + retry=supra_client_config.retry, ) super().__init__(ApiClient(base_url, api_client_config)) @@ -391,7 +393,7 @@ async def wait_for_transaction(self, tx_hash: str) -> dict[str, Any]: may still succeed later, e.g. because its sequence number is too new), a missing status or an unknown one. A 404 whose body is the node's "transaction not found" message (a JSON object with a `message` starting "Information not available for" or containing "Cannot find transaction") means "not seen yet" and polling - continues; any other 404, such as a wrong base URL or a missing route, is raised as `ApiError`. The final + continues; any other 404, such as a wrong base URL or a missing route, is raised as `ApiNotFoundError`. The final data is returned whatever the status is, so the caller must check `status`. The wait never starts a poll after `TransactionConfig.transaction_wait_time_in_seconds` has passed, bounds each @@ -1163,7 +1165,7 @@ def _is_transaction_not_found(error: ApiError) -> bool: if error.status_code != HTTPStatus.NOT_FOUND: return False try: - message = json.loads(error.message).get("message") + message = json.loads(error.body).get("message") except (ValueError, AttributeError): return False return isinstance(message, str) and any( diff --git a/tests/unit/test_api_errors.py b/tests/unit/test_api_errors.py new file mode 100644 index 0000000..c70e6b9 --- /dev/null +++ b/tests/unit/test_api_errors.py @@ -0,0 +1,400 @@ +# Copyright © Supra +# SPDX-License-Identifier: Apache-2.0 + +"""Offline tests for the typed HTTP errors, the request timeout and the GET retry policy.""" + +import unittest +from collections.abc import Callable +from datetime import UTC, datetime, timedelta +from email.utils import format_datetime +from unittest import mock + +import httpx + +from supra_sdk.account_address import AccountAddress +from supra_sdk.clients import ( + ApiClient, + ApiClientConfig, + ApiError, + ApiGoneError, + ApiNotFoundError, + ApiRateLimitError, + ApiRequestTimeoutError, + ApiServiceUnavailableError, + ClientTimeoutError, + RetryPolicy, + UnexpectedResponseError, +) +from supra_sdk.clients.rest import SupraClient, SupraClientConfig + +ADDRESS = AccountAddress.from_str("0x1") +ACCOUNT = {"sequence_number": "0", "authentication_key": "0x1"} + +Handler = Callable[[httpx.Request], httpx.Response] + + +class Node: + """Fake node that answers each request with the next queued response; the last one repeats.""" + + def __init__(self, *responses: httpx.Response | Exception): + self.responses = list(responses) + self.requests: list[httpx.Request] = [] + + def __call__(self, request: httpx.Request) -> httpx.Response: + self.requests.append(request) + response = ( + self.responses.pop(0) if len(self.responses) > 1 else self.responses[0] + ) + if isinstance(response, Exception): + raise response + return response + + +class Sleeps: + """Records the waits between attempts instead of sleeping.""" + + def __init__(self): + self.delays: list[float] = [] + + async def __call__(self, delay: float) -> None: + self.delays.append(delay) + + +class ClientTestCase(unittest.IsolatedAsyncioTestCase): + async def make_client( + self, handler: Handler, config: SupraClientConfig | None = None + ) -> SupraClient: + client = SupraClient("http://node.invalid/", config) + api_client_config = client.api_client.api_client_config + await client.api_client.close() + client.api_client = ApiClient( + "http://node.invalid/", + api_client_config, + transport=httpx.MockTransport(handler), + ) + self.addAsyncCleanup(client.close) + return client + + def patch_sleep(self) -> Sleeps: + sleeps = Sleeps() + patcher = mock.patch("supra_sdk.clients.api_client._sleep", new=sleeps) + patcher.start() + self.addCleanup(patcher.stop) + return sleeps + + +class ErrorTypeTests(ClientTestCase): + async def asyncSetUp(self): + self.node = Node(httpx.Response(200, json=ACCOUNT)) + self.client = await self.make_client(self.node) + + async def error_for(self, response: httpx.Response) -> ApiError: + self.node.responses = [response] + with self.assertRaises(ApiError) as ctx: + await self.client.account(ADDRESS) + return ctx.exception + + async def test_status_selects_the_error_class(self): + cases = { + 400: ApiError, + 404: ApiNotFoundError, + 408: ApiRequestTimeoutError, + 410: ApiGoneError, + 429: ApiRateLimitError, + 500: ApiError, + 503: ApiServiceUnavailableError, + } + for status, error_class in cases.items(): + with self.subTest(status=status): + error = await self.error_for( + httpx.Response(status, json={"message": "m"}) + ) + self.assertIs(type(error), error_class) + self.assertEqual(error.status_code, status) + + async def test_message_is_the_node_message(self): + # The body the node answers for a missing account. + body = {"message": "Not Found: Resource not found: 0x1::account::Account"} + error = await self.error_for(httpx.Response(404, json=body)) + self.assertEqual(error.message, body["message"]) + self.assertEqual( + error.body, + '{"message":"Not Found: Resource not found: 0x1::account::Account"}', + ) + self.assertIn(body["message"], str(error)) + + async def test_message_falls_back_to_the_body_then_the_reason_phrase(self): + cases = [ + (httpx.Response(500, text="boom"), "boom"), + ( + httpx.Response(404, json=["Information not available"]), + '["Information not available"]', + ), + (httpx.Response(404, json={"message": ""}), '{"message":""}'), + (httpx.Response(408, text=""), "Request Timeout"), + (httpx.Response(599, text=""), "HTTP 599"), + ] + for response, message in cases: + with self.subTest(message=message): + error = await self.error_for(response) + self.assertEqual(error.message, message) + + async def test_headers_are_kept(self): + error = await self.error_for( + httpx.Response(500, json={"message": "m"}, headers={"X-Custom": "v"}) + ) + self.assertEqual(error.headers["x-custom"], "v") + + async def test_rate_limit_retry_after_from_the_body(self): + error = await self.error_for( + httpx.Response( + 429, + json={"message": "slow down", "retry_after_seconds": 76680}, + headers={"Retry-After": "5"}, + ) + ) + self.assertIsInstance(error, ApiRateLimitError) + assert isinstance(error, ApiRateLimitError) + self.assertEqual(error.retry_after_seconds, 76680) + self.assertEqual(error.message, "slow down") + + async def test_rate_limit_retry_after_from_the_header(self): + future = format_datetime( + datetime.now(UTC) + timedelta(seconds=120), usegmt=True + ) + cases = [ + ({"retry_after_seconds": "12"}, {}, 12), + ({"message": "m"}, {"Retry-After": "30"}, 30), + ({"retry_after_seconds": -1}, {"Retry-After": "7"}, 7), + ({"retry_after_seconds": "x"}, {}, None), + ({"message": "m"}, {"Retry-After": "soon"}, None), + ({"message": "m"}, {}, None), + ] + for body, headers, expected in cases: + with self.subTest(body=body, headers=headers): + error = await self.error_for( + httpx.Response(429, json=body, headers=headers) + ) + assert isinstance(error, ApiRateLimitError) + self.assertEqual(error.retry_after_seconds, expected) + error = await self.error_for( + httpx.Response(429, text="", headers={"Retry-After": future}) + ) + assert isinstance(error, ApiRateLimitError) + assert error.retry_after_seconds is not None + self.assertTrue(115 <= error.retry_after_seconds <= 121) + + async def test_gone_oldest_block_from_the_header(self): + cases = [ + ({"x-supra-oldest-block": "149399"}, 149399), + ({}, None), + ({"x-supra-oldest-block": "abc"}, None), + ({"x-supra-oldest-block": "-5"}, None), + ] + for headers, expected in cases: + with self.subTest(headers=headers): + error = await self.error_for( + httpx.Response(410, json={"message": "pruned"}, headers=headers) + ) + assert isinstance(error, ApiGoneError) + self.assertEqual(error.oldest_block, expected) + + async def test_error_built_without_a_response(self): + error = ApiError("text", 400) + self.assertEqual(error.body, "text") + self.assertEqual(len(error.headers), 0) + self.assertEqual(str(error), "{message: text, status_code: 400}") + + +class UnexpectedResponseTests(ClientTestCase): + async def test_view_without_result_names_the_field(self): + for response in ( + httpx.Response(200, json={"other": 1}), + httpx.Response(200, json=[1]), + httpx.Response(200, text="not json"), + ): + with self.subTest(body=response.text): + client = await self.make_client(Node(response)) + with self.assertRaises(UnexpectedResponseError) as ctx: + await client.view("0x1::m::f", [], []) + error = ctx.exception + self.assertIsInstance(error, KeyError) + self.assertIsInstance(error, ApiError) + self.assertEqual(error.field, "result") + self.assertEqual(error.status_code, 200) + self.assertIn("'result'", str(error)) + self.assertIn("rpc/v3/view", str(error)) + + async def test_view_result_is_returned(self): + client = await self.make_client( + Node(httpx.Response(200, json={"result": ["1"]})) + ) + self.assertEqual(await client.view("0x1::m::f", [], []), ["1"]) + + +class TimeoutTests(ClientTestCase): + async def test_timeout_sets_every_phase_including_the_pool(self): + node = Node(httpx.Response(200, json=ACCOUNT)) + client = await self.make_client(node, SupraClientConfig(timeout=5)) + timeout = client.api_client._client.timeout + self.assertEqual( + (timeout.connect, timeout.read, timeout.write, timeout.pool), + (5, 5, 5, 5), + ) + await client.account(ADDRESS) + self.assertEqual( + node.requests[0].extensions["timeout"], + {"connect": 5, "read": 5, "write": 5, "pool": 5}, + ) + + async def test_default_timeout_is_60_seconds_with_a_pool_limit(self): + client = ApiClient("http://node.invalid/") + self.addAsyncCleanup(client.close) + self.assertEqual(client._client.timeout.read, 60) + self.assertEqual(client._client.timeout.pool, 60) + + async def test_invalid_timeout_is_rejected(self): + for value in (0, -1, float("inf"), float("nan"), True): + with self.subTest(value=value), self.assertRaises(ValueError): + ApiClient("http://node.invalid/", ApiClientConfig(timeout=value)) + + async def test_timeout_raises_client_timeout_error(self): + node = Node(httpx.ReadTimeout("read timed out")) + client = await self.make_client(node, SupraClientConfig(timeout=2)) + with self.assertRaises(ClientTimeoutError) as ctx: + await client.account(ADDRESS) + self.assertIsInstance(ctx.exception, httpx.TimeoutException) + self.assertEqual(ctx.exception.timeout_seconds, 2) + self.assertEqual(ctx.exception.phase, "ReadTimeout") + self.assertEqual(len(node.requests), 1) + + async def test_post_timeout_raises_client_timeout_error(self): + node = Node(httpx.ConnectTimeout("connect timed out")) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + with self.assertRaises(ClientTimeoutError): + await client.view("0x1::m::f", [], []) + self.assertEqual(len(node.requests), 1) + + +class RetryTests(ClientTestCase): + async def test_retry_policy_validation(self): + for kwargs in ( + {"attempts": 0}, + {"attempts": 1.5}, + {"attempts": True}, + {"max_delay_seconds": -1}, + {"base_delay_seconds": float("inf")}, + ): + with self.subTest(kwargs=kwargs), self.assertRaises(ValueError): + RetryPolicy(**kwargs) # type: ignore[arg-type] + + async def test_get_is_not_retried_by_default(self): + node = Node(httpx.Response(408, text=""), httpx.Response(200, json=ACCOUNT)) + client = await self.make_client(node) + with self.assertRaises(ApiRequestTimeoutError): + await client.account(ADDRESS) + self.assertEqual(len(node.requests), 1) + + async def test_408_then_200_returns_the_200_body(self): + sleeps = self.patch_sleep() + node = Node(httpx.Response(408, text=""), httpx.Response(200, json=ACCOUNT)) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + self.assertEqual(await client.account(ADDRESS), ACCOUNT) + self.assertEqual(len(node.requests), 2) + self.assertEqual(sleeps.delays, [0.25]) + + async def test_timeout_then_200_returns_the_200_body(self): + self.patch_sleep() + node = Node( + httpx.ReadTimeout("read timed out"), httpx.Response(200, json=ACCOUNT) + ) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + self.assertEqual(await client.account(ADDRESS), ACCOUNT) + self.assertEqual(len(node.requests), 2) + + async def test_attempts_are_bounded_with_exponential_backoff(self): + sleeps = self.patch_sleep() + node = Node(httpx.Response(503, json={"message": "busy"})) + policy = RetryPolicy(attempts=4, base_delay_seconds=1, max_delay_seconds=3) + client = await self.make_client(node, SupraClientConfig(retry=policy)) + with self.assertRaises(ApiServiceUnavailableError) as ctx: + await client.account(ADDRESS) + self.assertEqual(ctx.exception.message, "busy") + self.assertEqual(len(node.requests), 4) + self.assertEqual(sleeps.delays, [1, 2, 3]) + + async def test_timeouts_are_bounded(self): + self.patch_sleep() + node = Node(httpx.ReadTimeout("read timed out")) + client = await self.make_client( + node, SupraClientConfig(retry=RetryPolicy(attempts=2)) + ) + with self.assertRaises(ClientTimeoutError): + await client.account(ADDRESS) + self.assertEqual(len(node.requests), 2) + + async def test_retry_after_is_honoured(self): + sleeps = self.patch_sleep() + node = Node( + httpx.Response(429, json={"message": "m", "retry_after_seconds": 2}), + httpx.Response(429, json={"message": "m"}, headers={"Retry-After": "1"}), + httpx.Response(200, json=ACCOUNT), + ) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + self.assertEqual(await client.account(ADDRESS), ACCOUNT) + self.assertEqual(sleeps.delays, [2, 1]) + + async def test_retry_after_above_the_cap_is_raised_at_once(self): + sleeps = self.patch_sleep() + node = Node( + httpx.Response(429, json={"message": "m", "retry_after_seconds": 76680}), + httpx.Response(200, json=ACCOUNT), + ) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + with self.assertRaises(ApiRateLimitError) as ctx: + await client.account(ADDRESS) + self.assertEqual(ctx.exception.retry_after_seconds, 76680) + self.assertEqual(len(node.requests), 1) + self.assertEqual(sleeps.delays, []) + + async def test_other_errors_are_not_retried(self): + self.patch_sleep() + node = Node(httpx.Response(200, json=ACCOUNT)) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + for status in (400, 404, 410, 500, 502): + with self.subTest(status=status): + node.responses = [httpx.Response(status, json={"message": "m"})] + node.requests = [] + with self.assertRaises(ApiError): + await client.account(ADDRESS) + self.assertEqual(len(node.requests), 1) + + async def test_post_is_never_retried(self): + sleeps = self.patch_sleep() + node = Node(httpx.Response(200, json={"result": []})) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + for status in (408, 429, 503): + with self.subTest(status=status): + node.responses = [httpx.Response(status, json={"message": "m"})] + node.requests = [] + with self.assertRaises(ApiError): + await client.view("0x1::m::f", [], []) + with self.assertRaises(ApiError): + await client.submit(b"\x00") + self.assertEqual(len(node.requests), 2) + self.assertTrue(all(r.method == "POST" for r in node.requests)) + self.assertEqual(sleeps.delays, []) + + async def test_non_strict_get_returns_the_last_response(self): + self.patch_sleep() + node = Node(httpx.Response(503, text="busy")) + client = await self.make_client( + node, SupraClientConfig(retry=RetryPolicy(attempts=2)) + ) + response = await client.api_client.get("rpc/v3/x", strict_mode=False) + self.assertEqual(response.status_code, 503) + self.assertEqual(len(node.requests), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_request_urls.py b/tests/unit/test_request_urls.py index 78cb9bf..dcf11d4 100644 --- a/tests/unit/test_request_urls.py +++ b/tests/unit/test_request_urls.py @@ -121,7 +121,7 @@ async def test_requests_use_the_real_client_configuration(self): await rec.close() self.assertIn(Metadata.SUPRA_HEADER, rec.last.headers) self.assertEqual(client.timeout.read, 60.0) - self.assertIsNone(client.timeout.pool) + self.assertEqual(client.timeout.pool, 60.0) async def test_post_keeps_path_prefix(self): rec = await RecordingClient.create("https://proxy.example/supra") From 42c4252c7b33b66106073f8bf553a4eaeb45c475 Mon Sep 17 00:00:00 2001 From: Isaac Doidge Date: Wed, 4 Nov 2026 01:58:05 +1000 Subject: [PATCH 2/4] fix(faucet): handle the "TryLater" answer and never repeat the faucet request The node answers a faucet request that no faucet account is free to fund with 200 and the JSON string "TryLater". faucet() read it as an object and raised AttributeError. It now returns None for it, so faucet(..., wait_for_faucet=False) returns None and faucet(...) raises FaucetRequestNotAcceptedError, as documented. A 200 body that is neither a JSON object nor "TryLater" raises UnexpectedResponseError. The faucet request is sent once even when ApiClientConfig.retry is set, because each accepted request funds the account. Co-Authored-By: Claude Opus 5.5 --- supra_sdk/clients/rest/faucet.py | 23 +++++- supra_sdk/clients/rest/supra_client.py | 7 ++ tests/unit/test_faucet.py | 105 +++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_faucet.py diff --git a/supra_sdk/clients/rest/faucet.py b/supra_sdk/clients/rest/faucet.py index 562e398..9580ba9 100644 --- a/supra_sdk/clients/rest/faucet.py +++ b/supra_sdk/clients/rest/faucet.py @@ -4,6 +4,7 @@ from typing import Any from supra_sdk.account_address import AccountAddress +from supra_sdk.clients.api_client import UnexpectedResponseError from supra_sdk.clients.rest.rest_client import RestClient from supra_sdk.clients.rest.rest_types import ( FAUCET_ENDPOINT, @@ -11,6 +12,9 @@ format_endpoint, ) +# The body of a 200 faucet response when no faucet account is free to fund the request. +_FAUCET_TRY_LATER = "TryLater" + class FaucetRestClient(RestClient): """A class that provides methods to invoke `Faucet` REST endpoints from a Supra RPC node. @@ -23,6 +27,10 @@ class FaucetRestClient(RestClient): async def faucet(self, address: AccountAddress | str) -> str | None: """Requests faucet funds to be sent to the given account address. + The node answers `{"Accepted": }` when it submitted a funding transaction and `"TryLater"` when no + faucet account was free to fund it; a `"TryLater"` answer does not count against the address. The request is + sent once, whatever `ApiClientConfig.retry` says, because each accepted request funds the account. + Args: address (AccountAddress | str): The target account address to receive funds, as an `AccountAddress` or a hex string. @@ -30,9 +38,22 @@ async def faucet(self, address: AccountAddress | str) -> str | None: Returns: str | None: Faucet transaction hash if faucet request is accepted else None. + Raises: + ApiRateLimitError: If the address is inside the cooldown after its last grant or has reached the daily + number of grants (429). + ApiError: If the node answers with a status other than 200, for example 400 when the account's + SUPRA balance is at or above the faucet's cap. + UnexpectedResponseError: If the node answers 200 with a body that is neither a JSON object nor + `"TryLater"`. + """ endpoint = format_endpoint(FAUCET_ENDPOINT, address=address) - res_data = (await self.api_client.get(endpoint=endpoint)).json() + response = await self.api_client.get(endpoint=endpoint, retryable=False) + res_data = response.json() + if res_data == _FAUCET_TRY_LATER: + return None + if not isinstance(res_data, dict): + raise UnexpectedResponseError("Accepted", response) return res_data.get("Accepted", None) async def faucet_transaction_by_hash(self, tx_hash: str) -> dict[str, Any]: diff --git a/supra_sdk/clients/rest/supra_client.py b/supra_sdk/clients/rest/supra_client.py index 2f19309..9f76237 100644 --- a/supra_sdk/clients/rest/supra_client.py +++ b/supra_sdk/clients/rest/supra_client.py @@ -1099,6 +1099,9 @@ async def faucet( ) -> str | None: """Requests faucet funds to be sent to the given account address. + The faucet does not accept the request when the node answers `"TryLater"` (no faucet account was free to fund + it); the request can be sent again later. The request is sent once, whatever `ApiClientConfig.retry` says. + Args: address (AccountAddress | str): The target account address to receive funds, as an `AccountAddress` or a hex string. @@ -1109,6 +1112,10 @@ async def faucet( Raises: FaucetRequestNotAcceptedError: If waiting is requested and the faucet did not accept the request. + ApiRateLimitError: If the address is inside the cooldown after its last grant or has reached the daily + number of grants (429). + UnexpectedResponseError: If the node answers 200 with a body that is neither a JSON object nor + `"TryLater"`. TransactionFailedError: If waiting is requested and the final status of the faucet transaction is not `Success`. TransactionWaitTimeoutReachedError: If waiting is requested and the faucet transaction is not final within diff --git a/tests/unit/test_faucet.py b/tests/unit/test_faucet.py new file mode 100644 index 0000000..c5a37e7 --- /dev/null +++ b/tests/unit/test_faucet.py @@ -0,0 +1,105 @@ +# Copyright © Supra +# SPDX-License-Identifier: Apache-2.0 + +"""Offline tests for the faucet request.""" + +import unittest +from unittest import mock + +import httpx + +from supra_sdk.account_address import AccountAddress +from supra_sdk.clients import ( + ApiClient, + ApiRateLimitError, + ApiServiceUnavailableError, + RetryPolicy, + UnexpectedResponseError, +) +from supra_sdk.clients.rest import ( + FaucetRequestNotAcceptedError, + SupraClient, + SupraClientConfig, +) + +ADDRESS = AccountAddress.from_str("0x1") + + +class FaucetTests(unittest.IsolatedAsyncioTestCase): + async def make_client( + self, *responses: httpx.Response, config: SupraClientConfig | None = None + ) -> SupraClient: + self.requests: list[httpx.Request] = [] + queue = list(responses) + + def handler(request: httpx.Request) -> httpx.Response: + self.requests.append(request) + return queue.pop(0) if len(queue) > 1 else queue[0] + + client = SupraClient("http://node.invalid/", config) + api_client_config = client.api_client.api_client_config + await client.api_client.close() + client.api_client = ApiClient( + "http://node.invalid/", + api_client_config, + transport=httpx.MockTransport(handler), + ) + self.addAsyncCleanup(client.close) + return client + + async def test_try_later_without_waiting_returns_none(self): + client = await self.make_client(httpx.Response(200, json="TryLater")) + self.assertIsNone(await client.faucet(ADDRESS, wait_for_faucet=False)) + self.assertEqual(len(self.requests), 1) + + async def test_try_later_with_waiting_raises_not_accepted(self): + client = await self.make_client(httpx.Response(200, json="TryLater")) + with self.assertRaises(FaucetRequestNotAcceptedError): + await client.faucet(ADDRESS) + self.assertEqual(len(self.requests), 1) + + async def test_accepted_without_waiting_returns_the_hash(self): + tx_hash = "0x" + "ab" * 32 + client = await self.make_client(httpx.Response(200, json={"Accepted": tx_hash})) + self.assertEqual(await client.faucet(ADDRESS, wait_for_faucet=False), tx_hash) + + async def test_unexpected_body_raises(self): + for body in ("Something", 5, ["Accepted"]): + with self.subTest(body=body): + client = await self.make_client(httpx.Response(200, json=body)) + with self.assertRaises(UnexpectedResponseError) as ctx: + await client.faucet(ADDRESS, wait_for_faucet=False) + self.assertEqual(ctx.exception.field, "Accepted") + + async def test_rate_limit_is_typed(self): + # The node's body for an address inside the cooldown; the gateway may add `retry_after_seconds`. + client = await self.make_client( + httpx.Response( + 429, + json={ + "message": "Account 0x1 recently received some tokens. Please try again in 60s.", + "retry_after_seconds": 60, + }, + ) + ) + with self.assertRaises(ApiRateLimitError) as ctx: + await client.faucet(ADDRESS, wait_for_faucet=False) + self.assertEqual(ctx.exception.retry_after_seconds, 60) + self.assertIn("recently received some tokens", ctx.exception.message) + + async def test_faucet_request_is_never_retried(self): + sleep = mock.AsyncMock() + with mock.patch("supra_sdk.clients.api_client._sleep", new=sleep): + client = await self.make_client( + httpx.Response(503, json={"message": "m"}), + httpx.Response(200, json={"Accepted": "0x" + "ab" * 32}), + config=SupraClientConfig(retry=RetryPolicy()), + ) + with self.assertRaises(ApiServiceUnavailableError): + await client.faucet(ADDRESS, wait_for_faucet=False) + self.assertEqual(len(self.requests), 1) + sleep.assert_not_called() + + +if __name__ == "__main__": + unittest.main() From a54b852c832bd49007e96e6c60a28de1c1805c41 Mon Sep 17 00:00:00 2001 From: Isaac Doidge Date: Wed, 4 Nov 2026 02:09:40 +1000 Subject: [PATCH 3/4] fix(client): keep the timeout request, parse out-of-range bodies safely and reject malformed faucet answers ClientTimeoutError carries the request of the httpx timeout it replaces, so `error.request` works as it did for httpx.ReadTimeout, and it survives pickling. Parsing an error response raises nothing but the ApiError: a body nested beyond the recursion limit is kept as text, and a retry_after_seconds, Retry-After or x-supra-oldest-block value out of the float or int range is ignored. A timeout outside the float range is a ValueError. The backoff exponent is capped, so a large RetryPolicy.attempts cannot overflow. faucet() raises UnexpectedResponseError for a 200 body that is not JSON, and for an object whose Accepted is missing or not a non-empty string, where it returned None before. None now means only "TryLater". The ApiNotFoundError, ApiError and README texts now say that v11.7 nodes answer a missing resource with 404, and that only view() and faucet() raise UnexpectedResponseError. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +- supra_sdk/clients/api_client.py | 94 ++++++++++------ supra_sdk/clients/rest/faucet.py | 14 +-- supra_sdk/clients/rest/rest_client.py | 18 +++- supra_sdk/clients/rest/supra_client.py | 9 +- tests/unit/test_api_errors.py | 102 ++++++++++++++++-- tests/unit/test_faucet.py | 29 +++-- .../test_transaction_failure_reporting.py | 2 +- 8 files changed, 211 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index 9b84692..58160ea 100644 --- a/README.md +++ b/README.md @@ -31,8 +31,9 @@ statuses raise a subclass of `ApiError`: | 429 | `ApiRateLimitError` | `retry_after_seconds`, from the body or the `Retry-After` header | | 503 | `ApiServiceUnavailableError` | | -A 200 response without a field the SDK reads raises `UnexpectedResponseError`, which is also a `KeyError`. A request -that exceeds the timeout raises `ClientTimeoutError`, which is a `httpx.TimeoutException`. +`view()` and `faucet()` raise `UnexpectedResponseError`, which is also a `KeyError`, when a 200 response does not +have the field they read (`result` and `Accepted`). A request that exceeds the timeout raises `ClientTimeoutError`, +which is a `httpx.TimeoutException` and carries the request as `request`. Each phase of a request (connecting, waiting for a pooled connection, writing, reading) is limited to 60 seconds; `timeout` changes the limit. GET requests are repeated after a 408, 429 or 503 response or a timeout only when diff --git a/supra_sdk/clients/api_client.py b/supra_sdk/clients/api_client.py index 45cb820..bb55fde 100644 --- a/supra_sdk/clients/api_client.py +++ b/supra_sdk/clients/api_client.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 import asyncio +import contextlib import json import math import re @@ -35,6 +36,9 @@ _DECIMAL = re.compile(r"[0-9]+") +# Beyond 64 doublings any base delay above zero exceeds every useful `max_delay_seconds`. +_MAX_BACKOFF_DOUBLINGS = 64 + # Indirection so that tests can replace the wait between attempts. _sleep = asyncio.sleep @@ -83,7 +87,9 @@ def __post_init__(self): def backoff_seconds(self, attempt: int) -> float: """The wait before repeating attempt number `attempt` (counted from 1) when the response asks for none.""" - return min(self.max_delay_seconds, self.base_delay_seconds * 2 ** (attempt - 1)) + # The exponent is capped so that a large `attempts` cannot overflow a float. + doublings = min(attempt - 1, _MAX_BACKOFF_DOUBLINGS) + return min(self.max_delay_seconds, self.base_delay_seconds * 2.0**doublings) @dataclass @@ -325,8 +331,13 @@ async def post( def _timeout_error( self, url: str, error: httpx.TimeoutException ) -> "ClientTimeoutError": + try: + request: httpx.Request | None = error.request + except RuntimeError: + # httpx attaches the request to errors raised while sending one; this covers any it did not. + request = None return ClientTimeoutError( - url, self.api_client_config.timeout, type(error).__name__ + url, self.api_client_config.timeout, type(error).__name__, request=request ) @@ -340,7 +351,7 @@ class ApiError(Exception): - `ApiGoneError`: 410, with `oldest_block`. - `ApiRateLimitError`: 429, with `retry_after_seconds`. - `ApiServiceUnavailableError`: 503. - - `UnexpectedResponseError`: a 200 response whose body does not have the expected shape. + - `UnexpectedResponseError`: a 200 response to `view` or `faucet` whose body does not have the expected shape. A request that exceeds the client's timeout raises `ClientTimeoutError`, which is not an `ApiError`. @@ -386,7 +397,8 @@ class ApiNotFoundError(ApiError): """Exception raised when the API returns 404: the requested account, resource, module, transaction or route is not available on the node. - Nodes before `supra_node_v12.0.0` answer a missing account or resource with 400 instead, which raises `ApiError`. + Nodes before `supra_node_v12.0.0` answer a missing account with 400 instead, which raises `ApiError`; they answer + a missing resource with 404. """ @@ -425,11 +437,11 @@ def __init__( """ super().__init__(message, status_code, headers, body) value = self.headers.get(OLDEST_BLOCK_HEADER) - self.oldest_block = ( - int(value) - if value is not None and _DECIMAL.fullmatch(value.strip()) - else None - ) + self.oldest_block = None + if value is not None and _DECIMAL.fullmatch(value.strip()): + # ValueError: more digits than `int` converts (`sys.get_int_max_str_digits`). + with contextlib.suppress(ValueError): + self.oldest_block = int(value) class ApiRateLimitError(ApiError): @@ -510,7 +522,7 @@ class ClientTimeoutError(httpx.TimeoutException): connection from the pool, writing or reading. It is a `httpx.TimeoutException`, the error raised before this class existed, and not an `ApiError`: the server - sent no response. + sent no response. Like the `httpx` error it replaces, it carries the request as `request`. Attributes: url (str): The request URL, without query parameters. @@ -524,20 +536,37 @@ class ClientTimeoutError(httpx.TimeoutException): timeout_seconds: float phase: str - def __init__(self, url: str, timeout_seconds: float, phase: str): + def __init__( + self, + url: str, + timeout_seconds: float, + phase: str, + *, + request: httpx.Request | None = None, + ): """Initialize the exception with the request URL, the timeout and the phase that timed out. Args: url (str): The request URL, without query parameters. timeout_seconds (float): The timeout that was exceeded. phase (str): Name of the `httpx` timeout that was raised. + request (httpx.Request | None): The request that timed out. Default to None. """ self.url = url self.timeout_seconds = timeout_seconds self.phase = phase super().__init__( - f"Request to {url} timed out after {timeout_seconds} seconds ({phase})" + f"Request to {url} timed out after {timeout_seconds} seconds ({phase})", + request=request, + ) + + def __reduce__(self) -> tuple[Any, ...]: + # The arguments of `__init__` differ from `self.args`, which holds the message only. + return ( + type(self), + (self.url, self.timeout_seconds, self.phase), + self.__dict__, ) @@ -593,27 +622,23 @@ def retry_after_seconds(data: Any, headers: Mapping[str, str]) -> float | None: """ if isinstance(data, dict): value = data.get("retry_after_seconds") - if isinstance(value, str): - try: - value = float(value) - except ValueError: - value = None - if isinstance(value, int | float) and _is_non_negative_number(value): - return float(value) + seconds = _seconds(value.strip() if isinstance(value, str) else value) + if seconds is not None: + return seconds header = httpx.Headers(headers).get("retry-after") if header is None: return None header = header.strip() if _DECIMAL.fullmatch(header): - return float(header) + return _seconds(header) try: date = parsedate_to_datetime(header) - except (TypeError, ValueError): + if date.tzinfo is None: + date = date.replace(tzinfo=UTC) + return float(max(0, math.ceil(date.timestamp() - time.time()))) + except (TypeError, ValueError, OverflowError): return None - if date.tzinfo is None: - date = date.replace(tzinfo=UTC) - return float(max(0, math.ceil(date.timestamp() - time.time()))) def _error_message(body: str, status_code: int) -> str: @@ -631,19 +656,26 @@ def _error_message(body: str, status_code: int) -> str: def _json_or_none(text: str) -> Any: + # A body nested deeper than the interpreter's recursion limit raises RecursionError. try: return json.loads(text) - except ValueError: + except (ValueError, RecursionError): + return None + + +def _seconds(value: Any) -> float | None: + """`value` as a finite, non-negative number of seconds: from an int or a float, or a str in `float` syntax.""" + if isinstance(value, bool) or not isinstance(value, int | float | str): + return None + try: + seconds = float(value) + except (ValueError, OverflowError): return None + return seconds if math.isfinite(seconds) and seconds >= 0 else None def _is_non_negative_number(value: Any) -> bool: - return ( - isinstance(value, int | float) - and not isinstance(value, bool) - and math.isfinite(value) - and value >= 0 - ) + return not isinstance(value, str) and _seconds(value) is not None def _request_url(response: httpx.Response) -> str: diff --git a/supra_sdk/clients/rest/faucet.py b/supra_sdk/clients/rest/faucet.py index 9580ba9..52167ec 100644 --- a/supra_sdk/clients/rest/faucet.py +++ b/supra_sdk/clients/rest/faucet.py @@ -36,25 +36,27 @@ async def faucet(self, address: AccountAddress | str) -> str | None: or a hex string. Returns: - str | None: Faucet transaction hash if faucet request is accepted else None. + str | None: The faucet transaction hash when the node answers `Accepted`, None when it answers + `"TryLater"`. Raises: ApiRateLimitError: If the address is inside the cooldown after its last grant or has reached the daily number of grants (429). ApiError: If the node answers with a status other than 200, for example 400 when the account's SUPRA balance is at or above the faucet's cap. - UnexpectedResponseError: If the node answers 200 with a body that is neither a JSON object nor - `"TryLater"`. + UnexpectedResponseError: If the node answers 200 with a body that is neither `"TryLater"` nor a JSON + object whose `Accepted` is a non-empty string, including a body that is not JSON. """ endpoint = format_endpoint(FAUCET_ENDPOINT, address=address) response = await self.api_client.get(endpoint=endpoint, retryable=False) - res_data = response.json() + res_data = self._response_json(response, "Accepted") if res_data == _FAUCET_TRY_LATER: return None - if not isinstance(res_data, dict): + accepted = res_data.get("Accepted") if isinstance(res_data, dict) else None + if not isinstance(accepted, str) or not accepted: raise UnexpectedResponseError("Accepted", response) - return res_data.get("Accepted", None) + return accepted async def faucet_transaction_by_hash(self, tx_hash: str) -> dict[str, Any]: """Retrieves details of a faucet transaction by its hash. diff --git a/supra_sdk/clients/rest/rest_client.py b/supra_sdk/clients/rest/rest_client.py index c1c97a4..eba01e1 100644 --- a/supra_sdk/clients/rest/rest_client.py +++ b/supra_sdk/clients/rest/rest_client.py @@ -31,6 +31,19 @@ def __init__(self, api_client: ApiClient): """ self.api_client = api_client + @staticmethod + def _response_json(response: httpx.Response, field: str) -> Any: + """Returns the parsed JSON body of `response`, which is expected to hold `field`. + + Raises: + UnexpectedResponseError: If the body is not JSON. + + """ + try: + return response.json() + except (ValueError, RecursionError): + raise UnexpectedResponseError(field, response) from None + @staticmethod def _response_field(response: httpx.Response, field: str) -> Any: """Returns `field` of the JSON object in the body of `response`. @@ -39,10 +52,7 @@ def _response_field(response: httpx.Response, field: str) -> Any: UnexpectedResponseError: If the body is not a JSON object with `field`. """ - try: - data = response.json() - except ValueError: - raise UnexpectedResponseError(field, response) from None + data = RestClient._response_json(response, field) if not isinstance(data, dict) or field not in data: raise UnexpectedResponseError(field, response) return data[field] diff --git a/supra_sdk/clients/rest/supra_client.py b/supra_sdk/clients/rest/supra_client.py index 9f76237..703189e 100644 --- a/supra_sdk/clients/rest/supra_client.py +++ b/supra_sdk/clients/rest/supra_client.py @@ -1108,14 +1108,15 @@ async def faucet( wait_for_faucet (bool): Flag indicates whether wait for faucet should be done or not. Default to True. Returns: - str | None: Faucet transaction hash if faucet request is accepted else None. + str | None: The faucet transaction hash when the node answers `Accepted`; None when it answers + `"TryLater"` and `wait_for_faucet` is False. Raises: - FaucetRequestNotAcceptedError: If waiting is requested and the faucet did not accept the request. + FaucetRequestNotAcceptedError: If waiting is requested and the node answers `"TryLater"`. ApiRateLimitError: If the address is inside the cooldown after its last grant or has reached the daily number of grants (429). - UnexpectedResponseError: If the node answers 200 with a body that is neither a JSON object nor - `"TryLater"`. + UnexpectedResponseError: If the node answers 200 with a body that is neither `"TryLater"` nor a JSON + object whose `Accepted` is a non-empty string. TransactionFailedError: If waiting is requested and the final status of the faucet transaction is not `Success`. TransactionWaitTimeoutReachedError: If waiting is requested and the faucet transaction is not final within diff --git a/tests/unit/test_api_errors.py b/tests/unit/test_api_errors.py index c70e6b9..7212a55 100644 --- a/tests/unit/test_api_errors.py +++ b/tests/unit/test_api_errors.py @@ -3,6 +3,7 @@ """Offline tests for the typed HTTP errors, the request timeout and the GET retry policy.""" +import pickle import unittest from collections.abc import Callable from datetime import UTC, datetime, timedelta @@ -199,6 +200,39 @@ async def test_gone_oldest_block_from_the_header(self): assert isinstance(error, ApiGoneError) self.assertEqual(error.oldest_block, expected) + async def test_deeply_nested_body_is_kept_as_text(self): + body = "[" * 100_000 + for status in (500, 429): + with self.subTest(status=status): + error = await self.error_for(httpx.Response(status, text=body)) + self.assertEqual(error.message, body) + error = await self.error_for(httpx.Response(429, text=body)) + assert isinstance(error, ApiRateLimitError) + self.assertIsNone(error.retry_after_seconds) + + async def test_out_of_range_numbers_are_ignored(self): + huge = "1" + "0" * 400 + cases = [ + ('{"retry_after_seconds": ' + huge + "}", {}, None), + ('{"retry_after_seconds": ' + "1" * 5000 + "}", {}, None), + ('{"retry_after_seconds": "1e999"}', {}, None), + ('{"retry_after_seconds": ' + huge + "}", {"Retry-After": "4"}, 4), + ("{}", {"Retry-After": "9" * 500}, None), + ("{}", {"Retry-After": "Fri, 31 Dec 99999 23:59:59 GMT"}, None), + ] + for body, headers, expected in cases: + with self.subTest(body=body[:40], headers=headers): + error = await self.error_for( + httpx.Response(429, text=body, headers=headers) + ) + assert isinstance(error, ApiRateLimitError) + self.assertEqual(error.retry_after_seconds, expected) + error = await self.error_for( + httpx.Response(410, text="{}", headers={"x-supra-oldest-block": "9" * 5000}) + ) + assert isinstance(error, ApiGoneError) + self.assertIsNone(error.oldest_block) + async def test_error_built_without_a_response(self): error = ApiError("text", 400) self.assertEqual(error.body, "text") @@ -254,9 +288,9 @@ async def test_default_timeout_is_60_seconds_with_a_pool_limit(self): self.assertEqual(client._client.timeout.pool, 60) async def test_invalid_timeout_is_rejected(self): - for value in (0, -1, float("inf"), float("nan"), True): + for value in (0, -1, float("inf"), float("nan"), True, 10**400, "5"): with self.subTest(value=value), self.assertRaises(ValueError): - ApiClient("http://node.invalid/", ApiClientConfig(timeout=value)) + ApiClient("http://node.invalid/", ApiClientConfig(timeout=value)) # type: ignore[arg-type] async def test_timeout_raises_client_timeout_error(self): node = Node(httpx.ReadTimeout("read timed out")) @@ -268,6 +302,34 @@ async def test_timeout_raises_client_timeout_error(self): self.assertEqual(ctx.exception.phase, "ReadTimeout") self.assertEqual(len(node.requests), 1) + async def test_timeout_error_keeps_the_request(self): + node = Node(httpx.ReadTimeout("read timed out")) + client = await self.make_client(node) + with self.assertRaises(httpx.TimeoutException) as ctx: + await client.account(ADDRESS) + self.assertIs(ctx.exception.request, node.requests[0]) + self.assertEqual( + str(ctx.exception.request.url), "http://node.invalid/rpc/v3/accounts/0x1" + ) + + async def test_timeout_error_pickles(self): + node = Node(httpx.ReadTimeout("read timed out")) + client = await self.make_client(node, SupraClientConfig(timeout=2)) + with self.assertRaises(ClientTimeoutError) as ctx: + await client.account(ADDRESS) + restored = pickle.loads(pickle.dumps(ctx.exception)) + self.assertIsInstance(restored, ClientTimeoutError) + self.assertEqual(str(restored), str(ctx.exception)) + self.assertEqual(restored.url, ctx.exception.url) + self.assertEqual(restored.timeout_seconds, 2) + self.assertEqual(restored.phase, "ReadTimeout") + self.assertEqual(restored.request.url, ctx.exception.request.url) + without_request = ClientTimeoutError("http://x/", 1, "PoolTimeout") + restored = pickle.loads(pickle.dumps(without_request)) + self.assertEqual(str(restored), str(without_request)) + with self.assertRaises(RuntimeError): + _ = restored.request + async def test_post_timeout_raises_client_timeout_error(self): node = Node(httpx.ConnectTimeout("connect timed out")) client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) @@ -323,15 +385,43 @@ async def test_attempts_are_bounded_with_exponential_backoff(self): self.assertEqual(len(node.requests), 4) self.assertEqual(sleeps.delays, [1, 2, 3]) - async def test_timeouts_are_bounded(self): - self.patch_sleep() + async def test_timeouts_are_bounded_and_no_wait_follows_the_last(self): + sleeps = self.patch_sleep() node = Node(httpx.ReadTimeout("read timed out")) client = await self.make_client( - node, SupraClientConfig(retry=RetryPolicy(attempts=2)) + node, SupraClientConfig(retry=RetryPolicy(attempts=3)) ) with self.assertRaises(ClientTimeoutError): await client.account(ADDRESS) - self.assertEqual(len(node.requests), 2) + self.assertEqual(len(node.requests), 3) + self.assertEqual(sleeps.delays, [0.25, 0.5]) + + async def test_backoff_of_a_large_attempt_number_is_capped(self): + policy = RetryPolicy(attempts=10**6, max_delay_seconds=7) + self.assertEqual(policy.backoff_seconds(10**6), 7) + self.assertEqual(RetryPolicy(base_delay_seconds=0).backoff_seconds(10**6), 0) + + async def test_retry_after_on_a_503_is_honoured(self): + sleeps = self.patch_sleep() + node = Node( + httpx.Response(503, json={"message": "m"}, headers={"Retry-After": "3"}), + httpx.Response(200, json=ACCOUNT), + ) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + self.assertEqual(await client.account(ADDRESS), ACCOUNT) + self.assertEqual(sleeps.delays, [3]) + + async def test_http_date_retry_after_is_honoured(self): + sleeps = self.patch_sleep() + date = format_datetime(datetime.now(UTC) + timedelta(seconds=5), usegmt=True) + node = Node( + httpx.Response(429, json={"message": "m"}, headers={"Retry-After": date}), + httpx.Response(200, json=ACCOUNT), + ) + client = await self.make_client(node, SupraClientConfig(retry=RetryPolicy())) + self.assertEqual(await client.account(ADDRESS), ACCOUNT) + self.assertEqual(len(sleeps.delays), 1) + self.assertTrue(3 <= sleeps.delays[0] <= 6) async def test_retry_after_is_honoured(self): sleeps = self.patch_sleep() diff --git a/tests/unit/test_faucet.py b/tests/unit/test_faucet.py index c5a37e7..4a99e8a 100644 --- a/tests/unit/test_faucet.py +++ b/tests/unit/test_faucet.py @@ -30,11 +30,11 @@ async def make_client( self, *responses: httpx.Response, config: SupraClientConfig | None = None ) -> SupraClient: self.requests: list[httpx.Request] = [] - queue = list(responses) + self.queue = list(responses) def handler(request: httpx.Request) -> httpx.Response: self.requests.append(request) - return queue.pop(0) if len(queue) > 1 else queue[0] + return self.queue.pop(0) if len(self.queue) > 1 else self.queue[0] client = SupraClient("http://node.invalid/", config) api_client_config = client.api_client.api_client_config @@ -64,12 +64,25 @@ async def test_accepted_without_waiting_returns_the_hash(self): self.assertEqual(await client.faucet(ADDRESS, wait_for_faucet=False), tx_hash) async def test_unexpected_body_raises(self): - for body in ("Something", 5, ["Accepted"]): - with self.subTest(body=body): - client = await self.make_client(httpx.Response(200, json=body)) - with self.assertRaises(UnexpectedResponseError) as ctx: - await client.faucet(ADDRESS, wait_for_faucet=False) - self.assertEqual(ctx.exception.field, "Accepted") + bodies = [ + httpx.Response(200, json="Something"), + httpx.Response(200, json=5), + httpx.Response(200, json=["Accepted"]), + httpx.Response(200, json={}), + httpx.Response(200, json={"Accepted": ""}), + httpx.Response(200, json={"Accepted": 5}), + httpx.Response(200, json={"Accepted": None}), + httpx.Response(200, text="gateway"), + httpx.Response(200, text="[" * 100_000), + ] + client = await self.make_client(bodies[0]) + for response in bodies: + for wait_for_faucet in (False, True): + with self.subTest(body=response.text[:40], wait=wait_for_faucet): + self.queue = [response] + with self.assertRaises(UnexpectedResponseError) as ctx: + await client.faucet(ADDRESS, wait_for_faucet=wait_for_faucet) + self.assertEqual(ctx.exception.field, "Accepted") async def test_rate_limit_is_typed(self): # The node's body for an address inside the cooldown; the gateway may add `retry_after_seconds`. diff --git a/tests/unit/test_transaction_failure_reporting.py b/tests/unit/test_transaction_failure_reporting.py index ea696fb..a6b0f95 100644 --- a/tests/unit/test_transaction_failure_reporting.py +++ b/tests/unit/test_transaction_failure_reporting.py @@ -274,7 +274,7 @@ async def test_faucet_returns_hash_on_success_and_skips_check_without_waiting(se self.assertEqual(node.poll_count, 1) async def test_faucet_request_not_accepted(self): - client = await self.make_client(lambda _: httpx.Response(200, json={})) + client = await self.make_client(lambda _: httpx.Response(200, json="TryLater")) with self.assertRaises(FaucetRequestNotAcceptedError): await client.faucet(AccountAddress.from_str("0x1")) From b1c3b3336700e95b600b26e6afcd2b9cfdfbeae2 Mon Sep 17 00:00:00 2001 From: Isaac Doidge Date: Wed, 4 Nov 2026 10:49:46 +1000 Subject: [PATCH 4/4] fix(client): handle deeply nested 404 bodies and name the problem in faucet errors - `wait_for_transaction` and `faucet()` treat a 404 whose body is nested deeper than the interpreter's recursion limit as an ordinary 404 and raise it, instead of failing with a bare RecursionError. - `UnexpectedResponseError` takes an optional `problem` that completes its message. A faucet 200 whose `Accepted` is empty or not a string says so, a body that is not JSON says so, and a missing field keeps the existing text. - README describes the faucet outcomes: `"TryLater"`, non-JSON bodies and an `Accepted` that is missing, empty or not a string. - `retry_after_seconds` documents the string forms it accepts. - Tests cover an out-of-range integer retry delay. Co-Authored-By: Claude Sonnet 5.5 --- README.md | 6 +++-- supra_sdk/clients/api_client.py | 17 +++++++++---- supra_sdk/clients/rest/faucet.py | 10 ++++++-- supra_sdk/clients/rest/rest_client.py | 6 ++++- supra_sdk/clients/rest/supra_client.py | 2 +- tests/unit/test_api_errors.py | 1 + tests/unit/test_faucet.py | 24 +++++++++++++++++++ .../test_transaction_failure_reporting.py | 1 + 8 files changed, 56 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 185f15a..f4a7e03 100644 --- a/README.md +++ b/README.md @@ -31,8 +31,10 @@ statuses raise a subclass of `ApiError`: | 429 | `ApiRateLimitError` | `retry_after_seconds`, from the body or the `Retry-After` header | | 503 | `ApiServiceUnavailableError` | | -`view()` and `faucet()` raise `UnexpectedResponseError`, which is also a `KeyError`, when a 200 response does not -have the field they read (`result` and `Accepted`). A request that exceeds the timeout raises `ClientTimeoutError`, +`view()` raises `UnexpectedResponseError`, which is also a `KeyError`, when a 200 response is not a JSON object with +the `result` field. `faucet()` returns `None` (or raises `FaucetRequestNotAcceptedError` while waiting) when the +200 response is `"TryLater"`, and raises `UnexpectedResponseError` when the 200 body is not JSON or its `Accepted` +field is missing, empty or not a string. A request that exceeds the timeout raises `ClientTimeoutError`, which is a `httpx.TimeoutException` and carries the request as `request`. Each phase of a request (connecting, waiting for a pooled connection, writing, reading) is limited to 60 seconds; diff --git a/supra_sdk/clients/api_client.py b/supra_sdk/clients/api_client.py index bb55fde..36ec1c1 100644 --- a/supra_sdk/clients/api_client.py +++ b/supra_sdk/clients/api_client.py @@ -496,17 +496,23 @@ class UnexpectedResponseError(ApiError, KeyError): field: str - def __init__(self, field: str, response: httpx.Response): - """Initialize the exception from the response that lacks `field`. + def __init__( + self, field: str, response: httpx.Response, problem: str | None = None + ): + """Initialize the exception from the response that lacks a usable `field`. Args: field (str): The field the SDK expected in the response body. response (httpx.Response): The response. + problem (str | None): What is wrong with the response, completing the sentence "The response of + ...". Defaults to "has no '' field". """ self.field = field + if problem is None: + problem = f"has no {field!r} field" super().__init__( - f"The response of {_request_url(response)} has no {field!r} field", + f"The response of {_request_url(response)} {problem}", response.status_code, response.headers, response.text, @@ -611,8 +617,9 @@ def retry_after_seconds(data: Any, headers: Mapping[str, str]) -> float | None: """The number of seconds an error response asks the caller to wait before repeating the request. Args: - data (Any): The parsed JSON body, or None. Its `retry_after_seconds` field, a non-negative number or a - decimal string, is used when present. + data (Any): The parsed JSON body, or None. Its `retry_after_seconds` field is used when present and is a + finite, non-negative int or float (not a bool), or a string that `float()` accepts for such a value, + for example `"1.5"`, `"1e3"` or `"1_000"`. headers (Mapping[str, str]): The response headers. Their `Retry-After` value, delta-seconds or an HTTP date, is used when the body has no `retry_after_seconds`. diff --git a/supra_sdk/clients/rest/faucet.py b/supra_sdk/clients/rest/faucet.py index 52167ec..b43caa7 100644 --- a/supra_sdk/clients/rest/faucet.py +++ b/supra_sdk/clients/rest/faucet.py @@ -53,9 +53,15 @@ async def faucet(self, address: AccountAddress | str) -> str | None: res_data = self._response_json(response, "Accepted") if res_data == _FAUCET_TRY_LATER: return None - accepted = res_data.get("Accepted") if isinstance(res_data, dict) else None - if not isinstance(accepted, str) or not accepted: + if not isinstance(res_data, dict) or "Accepted" not in res_data: raise UnexpectedResponseError("Accepted", response) + accepted = res_data["Accepted"] + if not isinstance(accepted, str) or not accepted: + raise UnexpectedResponseError( + "Accepted", + response, + "has an 'Accepted' field that is not a non-empty string", + ) return accepted async def faucet_transaction_by_hash(self, tx_hash: str) -> dict[str, Any]: diff --git a/supra_sdk/clients/rest/rest_client.py b/supra_sdk/clients/rest/rest_client.py index eba01e1..9d93157 100644 --- a/supra_sdk/clients/rest/rest_client.py +++ b/supra_sdk/clients/rest/rest_client.py @@ -42,7 +42,11 @@ def _response_json(response: httpx.Response, field: str) -> Any: try: return response.json() except (ValueError, RecursionError): - raise UnexpectedResponseError(field, response) from None + raise UnexpectedResponseError( + field, + response, + f"does not have a JSON body, so it has no {field!r} field", + ) from None @staticmethod def _response_field(response: httpx.Response, field: str) -> Any: diff --git a/supra_sdk/clients/rest/supra_client.py b/supra_sdk/clients/rest/supra_client.py index 284d39c..2f2d3fd 100644 --- a/supra_sdk/clients/rest/supra_client.py +++ b/supra_sdk/clients/rest/supra_client.py @@ -1369,7 +1369,7 @@ def _is_transaction_not_found(error: ApiError) -> bool: return False try: message = json.loads(error.body).get("message") - except (ValueError, AttributeError): + except (ValueError, AttributeError, RecursionError): return False return isinstance(message, str) and any( fragment in message for fragment in _TRANSACTION_NOT_FOUND_MESSAGES diff --git a/tests/unit/test_api_errors.py b/tests/unit/test_api_errors.py index 7212a55..79b396c 100644 --- a/tests/unit/test_api_errors.py +++ b/tests/unit/test_api_errors.py @@ -346,6 +346,7 @@ async def test_retry_policy_validation(self): {"attempts": True}, {"max_delay_seconds": -1}, {"base_delay_seconds": float("inf")}, + {"max_delay_seconds": 10**400}, ): with self.subTest(kwargs=kwargs), self.assertRaises(ValueError): RetryPolicy(**kwargs) # type: ignore[arg-type] diff --git a/tests/unit/test_faucet.py b/tests/unit/test_faucet.py index 4a99e8a..2941642 100644 --- a/tests/unit/test_faucet.py +++ b/tests/unit/test_faucet.py @@ -84,6 +84,30 @@ async def test_unexpected_body_raises(self): await client.faucet(ADDRESS, wait_for_faucet=wait_for_faucet) self.assertEqual(ctx.exception.field, "Accepted") + async def test_unexpected_body_message_names_the_problem(self): + cases = [ + (httpx.Response(200, json={}), "has no 'Accepted' field"), + ( + httpx.Response(200, json={"Accepted": ""}), + "'Accepted' field that is not a non-empty string", + ), + ( + httpx.Response(200, json={"Accepted": 5}), + "'Accepted' field that is not a non-empty string", + ), + ( + httpx.Response(200, text="gateway"), + "does not have a JSON body", + ), + ] + client = await self.make_client(cases[0][0]) + for response, expected in cases: + with self.subTest(body=response.text[:40]): + self.queue = [response] + with self.assertRaises(UnexpectedResponseError) as ctx: + await client.faucet(ADDRESS, wait_for_faucet=False) + self.assertIn(expected, str(ctx.exception)) + async def test_rate_limit_is_typed(self): # The node's body for an address inside the cooldown; the gateway may add `retry_after_seconds`. client = await self.make_client( diff --git a/tests/unit/test_transaction_failure_reporting.py b/tests/unit/test_transaction_failure_reporting.py index a6b0f95..4c6ff1b 100644 --- a/tests/unit/test_transaction_failure_reporting.py +++ b/tests/unit/test_transaction_failure_reporting.py @@ -180,6 +180,7 @@ async def test_other_404_responses_are_raised(self): httpx.Response(404, text="Not Found"), httpx.Response(404, json={"message": "something else"}), httpx.Response(404, json=["Information not available for"]), + httpx.Response(404, text="[" * 100_000), ] for response in others: with self.subTest(body=response.text):