From 78341aa59ed9ed0e28a7b77dc0a3175ce4e2dd11 Mon Sep 17 00:00:00 2001 From: secretpray Date: Sun, 8 Nov 2026 19:31:57 +0200 Subject: [PATCH 1/2] fix: make retries and failover safe for requests that change state A request that starts or stops a session or sets prices is now resent, to the same or another API host, only when the connection could not be established; a timeout no longer sends it a second time. GET, PUT and DELETE requests are also retried after connection errors and timeouts. Retry-After is capped by the new max_retry_after option (60 seconds) and accepts an HTTP date. HTTP 400 raises ValidationError and HTTP 409 the new ConflictError. ConfigurationError is also a ValueError, and invalid OCTA_* numbers raise it. The API token is left out of repr(ClientConfig). --- CHANGELOG.md | 22 +++ src/octaspace/__init__.py | 2 + src/octaspace/_transport/httpx_transport.py | 123 ++++++++------- src/octaspace/_transport/retry.py | 20 ++- src/octaspace/client.py | 14 +- src/octaspace/config.py | 63 +++++--- src/octaspace/errors.py | 10 +- src/octaspace/resources/nodes.py | 5 +- .../resources/services/machine_rental.py | 7 +- src/octaspace/resources/services/render.py | 3 +- .../resources/services/session_proxy.py | 3 +- src/octaspace/resources/services/vpn.py | 3 +- src/octaspace/response.py | 17 +- tests/transport/test_httpx_transport_extra.py | 14 -- tests/transport/test_reliability.py | 149 ++++++++++++++++++ 15 files changed, 333 insertions(+), 122 deletions(-) create mode 100644 tests/transport/test_reliability.py diff --git a/CHANGELOG.md b/CHANGELOG.md index abaf06f..77d335a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,23 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. ## [Unreleased] +### Added + +- `ConflictError` for HTTP 409. +- The `max_retry_after` option (`OCTA_MAX_RETRY_AFTER`): the longest + `Retry-After` the client waits for, 60 seconds by default. +- `Retry-After` given as an HTTP date. + ### Changed +- HTTP 400 raises `ValidationError` instead of `APIError`. +- GET, PUT and DELETE requests are retried after connection errors and + timeouts, not only after error responses. +- `ConfigurationError` is also a `ValueError`; invalid `OCTA_*` numbers raise + it. +- Any transport failure raises `APIConnectionError` or `APITimeoutError` + instead of an `httpx` exception. + - **Breaking:** Python 3.11 or later is required; Python 3.10 is no longer supported. @@ -16,6 +31,13 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. - **Breaking:** the `octaspace-smoke` command and `python -m octaspace`. +### Fixed + +- With several API hosts, a request that changes something (starting or + stopping a session, setting prices) is no longer sent to another host after + a timeout, so that it cannot run twice. +- The API token no longer appears in `repr()` of `ClientConfig`. + ## [0.1.0] - 2026-04-14 ### Added diff --git a/src/octaspace/__init__.py b/src/octaspace/__init__.py index 9ac83a2..1040551 100644 --- a/src/octaspace/__init__.py +++ b/src/octaspace/__init__.py @@ -13,6 +13,7 @@ AuthenticationError, BadGatewayError, ConfigurationError, + ConflictError, GatewayTimeoutError, NetworkError, NotFoundError, @@ -92,6 +93,7 @@ def shutdown_shared_client() -> None: "BadGatewayError", "ClientConfig", "ConfigurationError", + "ConflictError", "GatewayTimeoutError", "NetworkError", "NotFoundError", diff --git a/src/octaspace/_transport/httpx_transport.py b/src/octaspace/_transport/httpx_transport.py index d8b673f..7c20f70 100644 --- a/src/octaspace/_transport/httpx_transport.py +++ b/src/octaspace/_transport/httpx_transport.py @@ -6,10 +6,10 @@ import httpx from ..config import ClientConfig -from ..errors import APIConnectionError, APITimeoutError, raise_for_response +from ..errors import APIConnectionError, APITimeoutError, NetworkError, raise_for_response from ..hooks import HookDispatcher from ..response import APIResponse -from .retry import compute_backoff, is_retryable_response +from .retry import can_resend, compute_backoff, is_retryable_response from .rotator import UrlRotator @@ -34,36 +34,26 @@ def request( body: Any = None, headers: dict[str, str] | None = None, ) -> APIResponse: - normalized_method = method.upper() - normalized_path = path if path.startswith("/") else f"/{path}" - response: APIResponse | None = None - attempts = self._config.max_retries + 1 - for attempt in range(attempts): - response = self._send_with_failover( - normalized_method, - normalized_path, - params=params, - body=body, - headers=headers, - ) - if ( - is_retryable_response(normalized_method, response.status_code) - and attempt < attempts - 1 - ): - delay = compute_backoff( - attempt=attempt, - retry_interval=self._config.retry_interval, - backoff_factor=self._config.backoff_factor, - retry_after=response.retry_after, + method = method.upper() + path = path if path.startswith("/") else f"/{path}" + attempt = 0 + while True: + try: + response = self._send_with_failover( + method, path, params=params, body=body, headers=headers ) + except httpx.TransportError as exc: + if attempt >= self._config.max_retries or not can_resend(method, exc): + raise _network_error(exc) from exc + time.sleep(self._backoff(attempt)) + else: + delay = self._retry_delay(method, response, attempt) + if delay is None: + if response.is_error: + raise_for_response(response) + return response time.sleep(delay) - continue - break - - assert response is not None - if response.is_error: - raise_for_response(response) - return response + attempt += 1 def get( self, @@ -132,6 +122,25 @@ def transport_stats(self) -> dict[str, Any]: stats["url"] = self._config.urls[0] return stats + def _retry_delay(self, method: str, response: APIResponse, attempt: int) -> float | None: + """Seconds to wait before repeating the request, or ``None`` not to repeat it.""" + if attempt >= self._config.max_retries: + return None + if not is_retryable_response(method, response.status_code): + return None + retry_after = response.retry_after + if retry_after is not None and retry_after > self._config.max_retry_after: + return None + return self._backoff(attempt, retry_after=retry_after) + + def _backoff(self, attempt: int, *, retry_after: int | None = None) -> float: + return compute_backoff( + attempt=attempt, + retry_interval=self._config.retry_interval, + backoff_factor=self._config.backoff_factor, + retry_after=retry_after, + ) + def _send_with_failover( self, method: str, @@ -141,31 +150,17 @@ def _send_with_failover( body: Any = None, headers: dict[str, str] | None = None, ) -> APIResponse: - ordered_urls = self._ordered_urls() - last_error: Exception | None = None - for base_url in ordered_urls: + """Send to the next healthy host; move on to another one only when that is safe.""" + *fallbacks, last = self._ordered_urls() + for base_url in fallbacks: try: - response = self._send_to_url( - base_url, - method, - path, - params=params, - body=body, - headers=headers, + return self._send_to_url( + base_url, method, path, params=params, body=body, headers=headers ) - except (APIConnectionError, APITimeoutError) as exc: - last_error = exc - if self._rotator is not None: - self._rotator.mark_failed(base_url) - continue - - if self._rotator is not None: - self._rotator.mark_success(base_url) - return response - - if last_error is not None: - raise last_error - raise APIConnectionError("All API endpoints unavailable") + except httpx.TransportError as exc: + if not can_resend(method, exc): + raise + return self._send_to_url(last, method, path, params=params, body=body, headers=headers) def _send_to_url( self, @@ -177,20 +172,20 @@ def _send_to_url( body: Any = None, headers: dict[str, str] | None = None, ) -> APIResponse: - request_headers = self._default_headers(headers) - url = self._build_url(base_url, path) try: response = self._client.request( method, - url, + self._build_url(base_url, path), params=params, json=body, - headers=request_headers, + headers=self._default_headers(headers), ) - except httpx.TimeoutException as exc: - raise APITimeoutError(str(exc)) from exc - except httpx.NetworkError as exc: - raise APIConnectionError(str(exc)) from exc + except httpx.TransportError: + if self._rotator is not None: + self._rotator.mark_failed(base_url) + raise + if self._rotator is not None: + self._rotator.mark_success(base_url) return self._hooks.response_wrapper(response) def _ordered_urls(self) -> list[str]: @@ -217,3 +212,9 @@ def _default_headers(self, headers: dict[str, str] | None = None) -> dict[str, s @staticmethod def _build_url(base_url: str, path: str) -> str: return f"{base_url.rstrip('/')}/{path.lstrip('/')}" + + +def _network_error(error: httpx.TransportError) -> NetworkError: + if isinstance(error, httpx.TimeoutException): + return APITimeoutError(str(error)) + return APIConnectionError(str(error)) diff --git a/src/octaspace/_transport/retry.py b/src/octaspace/_transport/retry.py index 3b60248..99e535f 100644 --- a/src/octaspace/_transport/retry.py +++ b/src/octaspace/_transport/retry.py @@ -1,13 +1,26 @@ +"""Which requests can be sent again, and how long to wait before that.""" + from __future__ import annotations import random -RETRYABLE_STATUSES = {408, 429, 500, 502, 503, 504} -RETRYABLE_METHODS = {"GET", "HEAD", "OPTIONS", "PUT", "DELETE"} +import httpx + +IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"}) +RETRYABLE_STATUSES = frozenset({408, 429, 500, 502, 503, 504}) + +# Failures before the request leaves the client: resending cannot run it twice. +NOT_SENT_ERRORS = (httpx.ConnectError, httpx.ConnectTimeout, httpx.PoolTimeout) def is_retryable_response(method: str, status_code: int) -> bool: - return method.upper() in RETRYABLE_METHODS and status_code in RETRYABLE_STATUSES + """Whether a response with this status is worth repeating for this method.""" + return method.upper() in IDEMPOTENT_METHODS and status_code in RETRYABLE_STATUSES + + +def can_resend(method: str, error: httpx.TransportError) -> bool: + """Whether a request that failed with ``error`` can safely be sent again.""" + return method.upper() in IDEMPOTENT_METHODS or isinstance(error, NOT_SENT_ERRORS) def compute_backoff( @@ -18,6 +31,7 @@ def compute_backoff( retry_after: int | None = None, jitter: float = 0.1, ) -> float: + """Seconds to wait before the next attempt; ``retry_after`` takes precedence.""" if retry_after is not None: return max(float(retry_after), 0.0) return retry_interval * (backoff_factor**attempt) + random.uniform(0.0, jitter) diff --git a/src/octaspace/client.py b/src/octaspace/client.py index 9e6e3fc..a9f5e24 100644 --- a/src/octaspace/client.py +++ b/src/octaspace/client.py @@ -5,7 +5,6 @@ from ._transport.base import BaseTransport from ._transport.httpx_transport import HttpxTransport from .config import ClientConfig -from .errors import ConfigurationError from .resources import ( AccountsResource, AppsResource, @@ -44,7 +43,7 @@ def __init__( **overrides: Any, ) -> None: self._config = self._build_config(api_key, config, overrides) - self._transport = transport or HttpxTransport(self._config) + self._transport = transport if transport is not None else HttpxTransport(self._config) self.accounts = AccountsResource(self._transport) self.nodes = NodesResource(self._transport) self.sessions = SessionsResource(self._transport) @@ -83,10 +82,7 @@ def _build_config( config: ClientConfig | None, overrides: dict[str, Any], ) -> ClientConfig: - base = config.with_overrides() if config is not None else ClientConfig() - try: - if api_key is not None: - base = base.with_overrides(api_key=api_key) - return base.with_overrides(**overrides) if overrides else base - except ValueError as exc: - raise ConfigurationError(str(exc)) from exc + base = config if config is not None else ClientConfig() + if api_key is not None: + overrides = {**overrides, "api_key": api_key} + return base.with_overrides(**overrides) diff --git a/src/octaspace/config.py b/src/octaspace/config.py index 6efde70..3d93bfc 100644 --- a/src/octaspace/config.py +++ b/src/octaspace/config.py @@ -2,14 +2,17 @@ import logging import os -from dataclasses import dataclass, replace -from typing import Any +from dataclasses import dataclass, field, replace +from typing import Any, TypeVar import httpx from .__about__ import __version__ +from .errors import ConfigurationError from .hooks import RequestHook, ResponseHook +_Number = TypeVar("_Number", int, float) + def _env_bool(value: str | None, default: bool) -> bool: if value is None: @@ -26,10 +29,10 @@ class ClientConfig: for the meaning and default of each field. Raises: - ValueError: A field has an invalid value. + ConfigurationError: A field has an invalid value. """ - api_key: str | None = None + api_key: str | None = field(default=None, repr=False) base_url: str = "https://api.octa.space" base_urls: list[str] | None = None keep_alive: bool = False @@ -43,6 +46,7 @@ class ClientConfig: max_retries: int = 2 retry_interval: float = 0.5 backoff_factor: float = 2.0 + max_retry_after: float = 60.0 ssl_verify: bool = True on_request: RequestHook | None = None on_response: ResponseHook | None = None @@ -52,12 +56,12 @@ class ClientConfig: def __post_init__(self) -> None: self.base_url = self.base_url.strip() if not self.base_url: - raise ValueError("base_url must be a non-empty string") + raise ConfigurationError("base_url must be a non-empty string") if self.base_urls is not None: cleaned_urls = [str(url).strip() for url in self.base_urls if str(url).strip()] if not cleaned_urls: - raise ValueError("base_urls must contain at least one non-empty URL") + raise ConfigurationError("base_urls must contain at least one non-empty URL") self.base_urls = cleaned_urls _validate_non_negative_int("pool_size", self.pool_size) @@ -70,6 +74,7 @@ def __post_init__(self) -> None: _validate_non_negative_int("max_retries", self.max_retries) _validate_non_negative_float("retry_interval", self.retry_interval) _validate_positive_float("backoff_factor", self.backoff_factor) + _validate_non_negative_float("max_retry_after", self.max_retry_after) @property def urls(self) -> list[str]: @@ -106,12 +111,12 @@ def with_overrides(self, **overrides: Any) -> ClientConfig: """Return a copy with the given fields replaced. Raises: - ValueError: A field is unknown or invalid. + ConfigurationError: A field is unknown or invalid. """ try: return replace(self, **overrides) except TypeError as exc: - raise ValueError(str(exc)) from exc + raise ConfigurationError(str(exc)) from exc @classmethod def from_env(cls, **overrides: Any) -> ClientConfig: @@ -123,6 +128,9 @@ def from_env(cls, **overrides: Any) -> ClientConfig: Args: **overrides: Fields that take precedence over the environment. + + Raises: + ConfigurationError: A variable has an invalid value. """ defaults = cls() base_urls = os.getenv("OCTA_BASE_URLS") @@ -133,16 +141,17 @@ def from_env(cls, **overrides: Any) -> ClientConfig: base_url=os.getenv("OCTA_BASE_URL", defaults.base_url), base_urls=[item.strip() for item in base_urls.split(",")] if base_urls else None, keep_alive=_env_bool(os.getenv("OCTA_KEEP_ALIVE"), defaults.keep_alive), - pool_size=int(os.getenv("OCTA_POOL_SIZE", defaults.pool_size)), - max_connections=int(os.getenv("OCTA_MAX_CONNECTIONS", defaults.max_connections)), - pool_timeout=float(os.getenv("OCTA_POOL_TIMEOUT", defaults.pool_timeout)), - idle_timeout=float(os.getenv("OCTA_IDLE_TIMEOUT", defaults.idle_timeout)), - open_timeout=float(os.getenv("OCTA_OPEN_TIMEOUT", defaults.open_timeout)), - read_timeout=float(os.getenv("OCTA_READ_TIMEOUT", defaults.read_timeout)), - write_timeout=float(os.getenv("OCTA_WRITE_TIMEOUT", defaults.write_timeout)), - max_retries=int(os.getenv("OCTA_MAX_RETRIES", defaults.max_retries)), - retry_interval=float(os.getenv("OCTA_RETRY_INTERVAL", defaults.retry_interval)), - backoff_factor=float(os.getenv("OCTA_BACKOFF_FACTOR", defaults.backoff_factor)), + pool_size=_env_number(int, "OCTA_POOL_SIZE", defaults.pool_size), + max_connections=_env_number(int, "OCTA_MAX_CONNECTIONS", defaults.max_connections), + pool_timeout=_env_number(float, "OCTA_POOL_TIMEOUT", defaults.pool_timeout), + idle_timeout=_env_number(float, "OCTA_IDLE_TIMEOUT", defaults.idle_timeout), + open_timeout=_env_number(float, "OCTA_OPEN_TIMEOUT", defaults.open_timeout), + read_timeout=_env_number(float, "OCTA_READ_TIMEOUT", defaults.read_timeout), + write_timeout=_env_number(float, "OCTA_WRITE_TIMEOUT", defaults.write_timeout), + max_retries=_env_number(int, "OCTA_MAX_RETRIES", defaults.max_retries), + retry_interval=_env_number(float, "OCTA_RETRY_INTERVAL", defaults.retry_interval), + backoff_factor=_env_number(float, "OCTA_BACKOFF_FACTOR", defaults.backoff_factor), + max_retry_after=_env_number(float, "OCTA_MAX_RETRY_AFTER", defaults.max_retry_after), ssl_verify=_env_bool(os.getenv("OCTA_SSL_VERIFY"), defaults.ssl_verify), logger=logger, user_agent=os.getenv("OCTA_USER_AGENT"), @@ -150,21 +159,31 @@ def from_env(cls, **overrides: Any) -> ClientConfig: return config.with_overrides(**overrides) if overrides else config +def _env_number(kind: type[_Number], name: str, default: _Number) -> _Number: + value = os.getenv(name) + if value is None: + return default + try: + return kind(value) + except ValueError: + raise ConfigurationError(f"{name} must be a number, got {value!r}") from None + + def _validate_non_negative_int(name: str, value: int) -> None: if value < 0: - raise ValueError(f"{name} must be >= 0") + raise ConfigurationError(f"{name} must be >= 0") def _validate_positive_int(name: str, value: int) -> None: if value <= 0: - raise ValueError(f"{name} must be > 0") + raise ConfigurationError(f"{name} must be > 0") def _validate_non_negative_float(name: str, value: float) -> None: if value < 0: - raise ValueError(f"{name} must be >= 0") + raise ConfigurationError(f"{name} must be >= 0") def _validate_positive_float(name: str, value: float) -> None: if value <= 0: - raise ValueError(f"{name} must be > 0") + raise ConfigurationError(f"{name} must be > 0") diff --git a/src/octaspace/errors.py b/src/octaspace/errors.py index 4391dc6..dc884a1 100644 --- a/src/octaspace/errors.py +++ b/src/octaspace/errors.py @@ -9,7 +9,7 @@ class OctaSpaceError(Exception): """Base class for all SDK exceptions.""" -class ConfigurationError(OctaSpaceError): +class ConfigurationError(OctaSpaceError, ValueError): """Raised when the client is configured incorrectly.""" @@ -54,7 +54,11 @@ class NotFoundError(APIError): class ValidationError(APIError): - """Raised for HTTP 422 responses.""" + """Raised for HTTP 400 and 422 responses: the request was rejected.""" + + +class ConflictError(APIError): + """Raised for HTTP 409 responses.""" class RateLimitError(APIError): @@ -86,9 +90,11 @@ class GatewayTimeoutError(ServerError): STATUS_ERRORS: dict[int, type[APIError]] = { + 400: ValidationError, 401: AuthenticationError, 403: APIPermissionError, 404: NotFoundError, + 409: ConflictError, 422: ValidationError, 429: RateLimitError, 502: BadGatewayError, diff --git a/src/octaspace/resources/nodes.py b/src/octaspace/resources/nodes.py index ac41344..728f52c 100644 --- a/src/octaspace/resources/nodes.py +++ b/src/octaspace/resources/nodes.py @@ -46,7 +46,8 @@ def download_logs(self, node_id: int | str) -> APIResponse: def update_prices(self, node_id: int | str, **prices: Any) -> APIResponse: """Set the prices of a node. - ``PATCH /nodes/{node_id}/prices``. Never retried. + ``PATCH /nodes/{node_id}/prices``. Retried only when the connection + could not be established. Args: node_id: Your node. @@ -54,7 +55,7 @@ def update_prices(self, node_id: int | str, **prices: Any) -> APIResponse: an integer from 0 to 10000. Raises: - APIError: Status 400 for an unknown field or a value out of range. + ValidationError: An unknown field or a value out of range. """ return self.patch(f"/nodes/{self.encode(node_id)}/prices", body=prices) diff --git a/src/octaspace/resources/services/machine_rental.py b/src/octaspace/resources/services/machine_rental.py index e493729..c4b9db4 100644 --- a/src/octaspace/resources/services/machine_rental.py +++ b/src/octaspace/resources/services/machine_rental.py @@ -47,7 +47,8 @@ def create( ) -> APIResponse: """Start a machine rental session. - ``POST /services/mr``. Never retried. + ``POST /services/mr``. Retried only when the connection could not be + established. Args: node_id: The machine to rent. @@ -64,8 +65,8 @@ def create( The response; ``data["uuid"]`` is the session UUID. Raises: - APIError: Status 400 when the session cannot be started, for - example for an insufficient balance. + ValidationError: The session cannot be started, for example for + an insufficient balance. """ payload: MachineRentalCreatePayload = { "node_id": node_id, diff --git a/src/octaspace/resources/services/render.py b/src/octaspace/resources/services/render.py index 79580de..2819126 100644 --- a/src/octaspace/resources/services/render.py +++ b/src/octaspace/resources/services/render.py @@ -19,7 +19,8 @@ def list(self, **params: Any) -> APIResponse: def create(self, **attrs: Any) -> APIResponse: """Start a render session. - ``POST /services/render``. Never retried. + ``POST /services/render``. Retried only when the connection + could not be established. Args: **attrs: The request body: ``node_id`` and ``disk_size`` (GB), diff --git a/src/octaspace/resources/services/session_proxy.py b/src/octaspace/resources/services/session_proxy.py index 8edb7ca..6900f09 100644 --- a/src/octaspace/resources/services/session_proxy.py +++ b/src/octaspace/resources/services/session_proxy.py @@ -37,7 +37,8 @@ def logs(self) -> APIResponse: def stop(self, **params: Any) -> APIResponse: """Stop the session. - ``POST /services/{uuid}/stop``. Never retried. + ``POST /services/{uuid}/stop``. Retried only when the connection could + not be established. Args: **params: Optional ``score``, your integer rating of the session. diff --git a/src/octaspace/resources/services/vpn.py b/src/octaspace/resources/services/vpn.py index e65ad49..628f76a 100644 --- a/src/octaspace/resources/services/vpn.py +++ b/src/octaspace/resources/services/vpn.py @@ -19,7 +19,8 @@ def list(self, **params: Any) -> APIResponse: def create(self, **attrs: Any) -> APIResponse: """Start a VPN session. - ``POST /services/vpn``. Never retried. + ``POST /services/vpn``. Retried only when the connection + could not be established. Args: **attrs: The request body: ``node_id`` (required), ``subkind`` (the diff --git a/src/octaspace/response.py b/src/octaspace/response.py index beed87f..1751660 100644 --- a/src/octaspace/response.py +++ b/src/octaspace/response.py @@ -1,6 +1,9 @@ from __future__ import annotations +import math from dataclasses import dataclass, field +from datetime import UTC, datetime +from email.utils import parsedate_to_datetime from typing import Any import httpx @@ -51,14 +54,22 @@ def request_id(self) -> str | None: @property def retry_after(self) -> int | None: - """The ``Retry-After`` header in seconds, or ``None``.""" + """The ``Retry-After`` header in whole seconds, or ``None``. + + Accepts both forms of the header: a number of seconds or an HTTP date. + """ value = self.headers.get("retry-after") if value is None: return None - try: + if value.strip().isdigit(): return int(value) - except ValueError: + try: + when = parsedate_to_datetime(value) + except (TypeError, ValueError): return None + if when.tzinfo is None: + when = when.replace(tzinfo=UTC) + return max(math.ceil((when - datetime.now(UTC)).total_seconds()), 0) @property def is_success(self) -> bool: diff --git a/tests/transport/test_httpx_transport_extra.py b/tests/transport/test_httpx_transport_extra.py index 9dd1987..6918471 100644 --- a/tests/transport/test_httpx_transport_extra.py +++ b/tests/transport/test_httpx_transport_extra.py @@ -1,10 +1,7 @@ from __future__ import annotations -import pytest - from octaspace._transport.httpx_transport import HttpxTransport from octaspace.config import ClientConfig -from octaspace.errors import APIConnectionError def test_transport_put_delete_and_custom_headers(respx_mock) -> None: @@ -23,14 +20,3 @@ def test_transport_put_delete_and_custom_headers(respx_mock) -> None: assert delete_route.calls.last.request.headers["Connection"] == "close" assert transport.transport_stats()["url"] == "https://api.octa.space" transport.close() - - -def test_transport_raises_when_ordered_urls_empty() -> None: - transport = HttpxTransport(ClientConfig(max_retries=0)) - - transport._ordered_urls = lambda: [] # type: ignore[method-assign] - - with pytest.raises(APIConnectionError, match="All API endpoints unavailable"): - transport._send_with_failover("GET", "/nodes") - - transport.close() diff --git a/tests/transport/test_reliability.py b/tests/transport/test_reliability.py new file mode 100644 index 0000000..ce8d9b3 --- /dev/null +++ b/tests/transport/test_reliability.py @@ -0,0 +1,149 @@ +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from email.utils import format_datetime + +import httpx +import pytest + +from octaspace import ( + APIConnectionError, + APITimeoutError, + ClientConfig, + ConfigurationError, + ConflictError, + OctaSpaceClient, + RateLimitError, + ValidationError, +) +from octaspace._transport.httpx_transport import HttpxTransport +from octaspace.response import APIResponse + +PRIMARY = "https://api.octa.space" +BACKUP = "https://api.octa.computer" + + +@pytest.fixture +def sleeps(monkeypatch: pytest.MonkeyPatch) -> list[float]: + calls: list[float] = [] + monkeypatch.setattr("octaspace._transport.httpx_transport.time.sleep", calls.append) + return calls + + +def two_hosts() -> HttpxTransport: + return HttpxTransport(ClientConfig(base_urls=[PRIMARY, BACKUP])) + + +def test_post_is_not_sent_to_another_host_after_a_timeout(respx_mock, sleeps) -> None: + primary = respx_mock.post(f"{PRIMARY}/services/mr").mock(side_effect=httpx.ReadTimeout("slow")) + backup = respx_mock.post(f"{BACKUP}/services/mr").respond(201, json={"uuid": "u"}) + + with pytest.raises(APITimeoutError): + two_hosts().post("/services/mr", body=[{"id": 0}]) + + assert primary.call_count == 1 + assert backup.call_count == 0 + assert sleeps == [] + + +def test_post_moves_to_another_host_when_it_cannot_connect(respx_mock) -> None: + respx_mock.post(f"{PRIMARY}/services/mr").mock(side_effect=httpx.ConnectError("refused")) + backup = respx_mock.post(f"{BACKUP}/services/mr").respond(201, json={"uuid": "u"}) + + response = two_hosts().post("/services/mr", body=[{"id": 0}]) + + assert response.data == {"uuid": "u"} + assert backup.call_count == 1 + + +def test_get_is_retried_after_a_connection_error(respx_mock, sleeps) -> None: + route = respx_mock.get(f"{PRIMARY}/nodes") + route.side_effect = [httpx.ConnectError("refused"), httpx.Response(200, json=[])] + + response = HttpxTransport(ClientConfig(max_retries=1)).get("/nodes") + + assert response.data == [] + assert route.call_count == 2 + assert len(sleeps) == 1 + + +def test_post_is_not_retried_after_a_timeout(respx_mock, sleeps) -> None: + route = respx_mock.post(f"{PRIMARY}/services/x/stop").mock( + side_effect=httpx.ReadTimeout("slow") + ) + + with pytest.raises(APITimeoutError): + HttpxTransport(ClientConfig(max_retries=2)).post("/services/x/stop") + + assert route.call_count == 1 + assert sleeps == [] + + +def test_protocol_errors_raise_a_connection_error(respx_mock) -> None: + respx_mock.get(f"{PRIMARY}/nodes").mock(side_effect=httpx.RemoteProtocolError("dropped")) + + with pytest.raises(APIConnectionError): + HttpxTransport(ClientConfig(max_retries=0)).get("/nodes") + + +def test_retry_after_longer_than_the_limit_raises_at_once(respx_mock, sleeps) -> None: + route = respx_mock.get(f"{PRIMARY}/nodes").respond(429, headers={"Retry-After": "120"}) + + with pytest.raises(RateLimitError) as info: + HttpxTransport(ClientConfig(max_retries=2, max_retry_after=60)).get("/nodes") + + assert info.value.retry_after == 120 + assert route.call_count == 1 + assert sleeps == [] + + +def test_retry_after_accepts_an_http_date() -> None: + when = datetime.now(UTC) + timedelta(seconds=30) + headers = httpx.Headers({"Retry-After": format_datetime(when, usegmt=True)}) + response = APIResponse(status_code=429, headers=headers, body=b"") + + assert response.retry_after is not None + assert 28 <= response.retry_after <= 30 + + +def test_retry_after_in_the_past_or_invalid() -> None: + past = APIResponse(429, httpx.Headers({"Retry-After": "Wed, 21 Oct 2015 07:28:00 GMT"}), b"") + no_zone = APIResponse( + 429, httpx.Headers({"Retry-After": "Wed, 21 Oct 2015 07:28:00 -0000"}), b"" + ) + invalid = APIResponse(429, httpx.Headers({"Retry-After": "soon"}), b"") + + assert past.retry_after == 0 + assert no_zone.retry_after == 0 + assert invalid.retry_after is None + + +@pytest.mark.parametrize(("status", "error"), [(400, ValidationError), (409, ConflictError)]) +def test_status_errors(respx_mock, status: int, error: type[Exception]) -> None: + respx_mock.post(f"{PRIMARY}/services/mr").respond( + status, json={"message": "Insufficient balance"} + ) + + with pytest.raises(error, match="Insufficient balance"): + HttpxTransport(ClientConfig(max_retries=0)).post("/services/mr") + + +def test_api_key_is_not_in_the_repr() -> None: + config = ClientConfig(api_key="secret-token") + + assert "secret-token" not in repr(config) + assert "secret-token" not in repr(OctaSpaceClient(config=config).config) + + +def test_invalid_environment_number(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OCTA_READ_TIMEOUT", "fast") + + with pytest.raises(ConfigurationError, match="OCTA_READ_TIMEOUT"): + ClientConfig.from_env() + + +def test_configuration_error_is_a_value_error() -> None: + with pytest.raises(ValueError): + ClientConfig(pool_size=-1) + with pytest.raises(ConfigurationError): + OctaSpaceClient(unknown_option=1) From 6ad930e7a9f50f6a085113fc069b3c63e4a858e1 Mon Sep 17 00:00:00 2001 From: secretpray Date: Sun, 8 Nov 2026 19:31:57 +0200 Subject: [PATCH 2/2] docs: describe the new retry, failover and error behaviour --- docs/errors-and-reliability.md | 49 +++++++++++++++++----------------- docs/getting-started.md | 6 +++-- docs/resources.md | 6 ++--- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/docs/errors-and-reliability.md b/docs/errors-and-reliability.md index 8975fe6..bf56f1c 100644 --- a/docs/errors-and-reliability.md +++ b/docs/errors-and-reliability.md @@ -27,11 +27,12 @@ from the API raises a subclass of `APIError`: | `AuthenticationError` | 401 | The API token is missing, wrong or regenerated | | `APIPermissionError` | 403 | Access denied | | `NotFoundError` | 404 | Unknown session, or a node that is not yours | -| `ValidationError` | 422 | A parameter is invalid | +| `ValidationError` | 400, 422 | The request was rejected: a parameter is invalid, or a session cannot be started (for example an insufficient balance) | +| `ConflictError` | 409 | The request conflicts with the current state | | `RateLimitError` | 429 | Too many requests | | `ServerError` | 5xx | A temporary problem on the API side | | `BadGatewayError`, `ServiceUnavailableError`, `GatewayTimeoutError` | 502, 503, 504 | Subclasses of `ServerError` | -| `APIError` | any other | For example 400 when a session cannot be started (insufficient balance) or prices are invalid | +| `APIError` | any other | Base class of all of the above | When the API cannot be reached at all, the error is a `NetworkError`: `APIConnectionError` (DNS, connection or TLS failure) or `APITimeoutError`. @@ -48,6 +49,7 @@ OctaSpaceError ├── AuthenticationError ├── APIPermissionError ├── NotFoundError + ├── ConflictError ├── ValidationError ├── RateLimitError └── ServerError @@ -62,11 +64,11 @@ An `APIError` has the HTTP `status_code`, the `request_id` (or `None`) and the full `response`. Its message is the API's reason when the response has one: ```python -from octaspace import APIError +from octaspace import ValidationError try: client.services.mr.create(node_id=312, image="ubuntu:24.04", disk_size=20) -except APIError as exc: +except ValidationError as exc: print(f"status {exc.status_code}: {exc}") # status 400: Insufficient balance print(exc.response.data) ``` @@ -92,22 +94,22 @@ except RateLimitError as exc: ## Retries -GET requests are retried automatically on the statuses 408, 429, 500, 502, -503 and 504, up to `max_retries` times (default 2), with exponential backoff -and a little jitter. When the response has a `Retry-After` header, the SDK -waits that many seconds instead. After the last attempt the error is raised. +GET requests are retried automatically on connection errors, timeouts and the +statuses 408, 429, 500, 502, 503 and 504, up to `max_retries` times (default +2), with exponential backoff and a little jitter. When the response has a +`Retry-After` header, the SDK waits that long instead; when the API asks to +wait longer than `max_retry_after` (default 60 seconds), the `RateLimitError` +is raised at once. After the last attempt the error is raised. > [!IMPORTANT] > Starting a session (`create()`), stopping it (`stop()`), setting prices and -> generating a wallet are never retried, so that they cannot run twice. -> Decide yourself whether to repeat them after an error. `nodes.reboot()` is a -> GET request and is retried like the others. +> generating a wallet are retried only when the connection could not be +> established, so that they cannot run twice. After a timeout or an error +> response, decide yourself whether to repeat them. `nodes.reboot()` is a GET +> request and is retried like the others. -A connection error or a timeout is not retried on the same host. With -[several API hosts](#several-api-hosts) the request moves to the next host. - -Set the number of retries and the backoff with `max_retries`, -`retry_interval` and `backoff_factor`; see +Set the number of retries and the waits with `max_retries`, `retry_interval`, +`backoff_factor` and `max_retry_after`; see [Client configuration](getting-started.md#options). ## Several API hosts @@ -127,12 +129,9 @@ with OctaSpaceClient( client.nodes.list() ``` -Requests are spread over the hosts in turn. When a host cannot be reached or -times out, the request is sent to the next host, and the failed host is left -out for 30 seconds. An HTTP error response, including 5xx, does not move the -request to another host. - -> [!WARNING] -> This also applies to `create()` and `stop()`. A request that timed out may -> still have reached the API: before you start a session again, check -> `client.sessions.list()`. +Requests are spread over the hosts in turn. When a host cannot be reached, the +request is sent to the next host, and the failed host is left out for 30 +seconds. A GET request also moves to the next host after a timeout. A request +that changes something (`create()`, `stop()`, setting prices) moves only when +the connection could not be established, so that it cannot run twice. An HTTP +error response, including 5xx, does not move the request to another host. diff --git a/docs/getting-started.md b/docs/getting-started.md index f7f68d6..fb05b59 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -40,7 +40,8 @@ finally: client.close() ``` -An unknown or invalid option raises `octaspace.ConfigurationError`. +An unknown or invalid option raises `octaspace.ConfigurationError`, a +subclass of `ValueError`. ## Options @@ -54,6 +55,7 @@ An unknown or invalid option raises `octaspace.ConfigurationError`. | `write_timeout` | `30.0` | `OCTA_WRITE_TIMEOUT` | Seconds to wait while sending a request | | `max_retries` | `2` | `OCTA_MAX_RETRIES` | Retries of failed requests that are safe to repeat; `0` turns them off. See [Retries](errors-and-reliability.md#retries) | | `retry_interval` / `backoff_factor` | `0.5` / `2.0` | `OCTA_RETRY_INTERVAL` / `OCTA_BACKOFF_FACTOR` | Backoff between retries: `retry_interval × backoff_factor^attempt` seconds | +| `max_retry_after` | `60.0` | `OCTA_MAX_RETRY_AFTER` | Longest `Retry-After` the SDK waits for; a longer wait raises the error at once | | `keep_alive` | `False` | `OCTA_KEEP_ALIVE` | Reuse connections, see [below](#keep-alive-connections) | | `pool_size` | `5` | `OCTA_POOL_SIZE` | Connections kept open with `keep_alive` | | `max_connections` | `20` | `OCTA_MAX_CONNECTIONS` | Most connections open at the same time | @@ -82,7 +84,7 @@ with OctaSpaceClient(config=config) as client: Keyword arguments to `from_env()` override the environment: `ClientConfig.from_env(max_retries=0)`. Boolean variables accept `1`, `true`, -`yes` and `on`. +`yes` and `on`. An invalid number raises `ConfigurationError`. ## Shared client diff --git a/docs/resources.md b/docs/resources.md index 0d573d6..f228aa7 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -65,8 +65,8 @@ uuid = response.data["uuid"] The session is created at once; its container is still being set up. Check `is_ready` with [`info()`](#session-details). A rejected request, for example -for an insufficient balance, raises `octaspace.APIError` with status 400 and -the reason as its message. +for an insufficient balance, raises `octaspace.ValidationError` with the +reason as its message. ## VPN @@ -208,7 +208,7 @@ client.nodes.update_prices(312, base=25, storage=2, traffic=1) Pass one or more of `base`, `storage` and `traffic`, each an integer from 0 to 10000; see `node_prices` in the [API reference](https://api.octa.space/api-docs) for the units. Any other -field or value raises `APIError` with status 400. +field or value raises `ValidationError`. ### Reboot