diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cbe7a7..844414b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 A round of platform improvements to `dexpace-sdk-core`: new optional building blocks (typed serialization, webhook verification, pagination, two pipeline policies), tightened retry and tracing behaviour, and a batch of correctness -fixes across bodies, SSE parsing, Digest auth, and error reporting. Everything -lands in `core`; the transport packages are unchanged. No public symbol was -removed, so existing code continues to work without modification. +fixes across bodies, SSE parsing, Digest auth, and error reporting. Most of this +lands in `core`; the transport adapters additionally get consistent connect- vs +read-phase timeout classification and tighter resource release. The only removed +public symbol is the unused `RetryConfig` (see Removed); existing code otherwise +continues to work without modification. ### Added @@ -41,6 +43,12 @@ removed, so existing code continues to work without modification. - **Log correlation** (`instrumentation.correlation`). A `contextvar`-backed correlation id that flows through the pipeline and is attached to log records, so logs from a single logical request can be tied together. +- **Reconnecting SSE client** (`http.sse.connection`). `SseConnection` and + `AsyncSseConnection` resume an interrupted event stream by replaying the + `Last-Event-ID` header and reconnecting with jittered backoff that honours the + server's `retry:` hint. Built on the shared dispatch seam + (`pipeline.dispatch`), which lets both the SSE client and the paginator accept + either a pipeline or a bare send-callable. ### Changed @@ -60,6 +68,20 @@ removed, so existing code continues to work without modification. - **Error reporting** (`errors.http`). HTTP errors now expose whether they are `retryable` and carry a bounded body snapshot for diagnostics, with the snapshot capped so an error never holds an unbounded payload. +- **`HttpRange.suffix`** (`http.common.http_range`) now returns a public + `HttpRange` (carrying an `is_suffix` flag) instead of a private helper type, + so a `bytes=-N` suffix range composes with `HttpRange.format_many` alongside + ordinary ranges. +- **`CallContext`** (`http.context`) is now an `abc.ABC`. It declares no + abstract methods, so existing subclasses are unaffected; the change only + prevents the base from being instantiated directly. + +### Removed + +- **`RetryConfig`** (`pipeline` / `pipeline.step.config`). It was exported but + never wired into the retry policy, so it configured nothing; `RetryPolicy`'s + constructor is the real configuration surface. Code that imported + `RetryConfig` should configure `RetryPolicy` directly. ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 903c8f6..4c01264 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,13 +43,18 @@ bodies are modelled as typed Pythonic abstractions instead. `from_form` / `from_stream` / `from_iter` / `from_file`. `ResponseBody` exposes `iter_bytes` / `bytes` / `string`. Single-use bodies (stream / iter) raise `RuntimeError` on second consumption — call `to_replayable()` - before the first send if retries are needed. + before the first send if retries are needed. `AsyncRequestBody` / + `AsyncResponseBody` are the async twins (`aiter_bytes`), and + `MultipartField` / `MultipartRequestBody` build `multipart/form-data` + payloads. - **Body capture for logging uses `BytesIO`.** `LoggableRequestBody` mirrors writes into a `BytesIO` tap; `LoggableResponseBody` caches drained bytes for repeatable reads. Both honour a configurable byte cap. - **Thread-safety where stated.** `ContextStore` is safe under concurrent - use; individual bodies and streams are not. Per-context lookups rely on - CPython's GIL for atomic dict ops and use a lock only for check-and-set. + use; individual bodies and streams are not. Every store operation + (`get` / `put` / `set` / `remove`) acquires a `threading.Lock`, so the + guarantee survives free-threaded CPython (PEP 703) and runtimes without + atomic dict ops rather than relying on the GIL. - **Public API is narrow.** Helpers and concrete adapter classes are module-private (leading underscore). The public surface for each subpackage is what its `__init__.py` re-exports. @@ -103,14 +108,20 @@ python-sdk/ │ │ │ ├── common/ # Headers, HttpHeaderName, MediaType, │ │ │ │ # Protocol, Url, QueryParams, ETag, │ │ │ │ # HttpRange, RequestConditions, - │ │ │ │ # common_media_types - │ │ │ ├── request/ # Request, RequestBody, FileRequestBody, - │ │ │ │ # LoggableRequestBody, Method - │ │ │ ├── response/ # Response, ResponseBody, - │ │ │ │ # LoggableResponseBody, Status + │ │ │ │ # common_media_types; pagination.py + │ │ │ │ # (ItemPaged/Pager + async twins), + │ │ │ │ # streaming.py (jsonl/chunked-frame iters) + │ │ │ ├── request/ # Request, RequestBody, AsyncRequestBody, + │ │ │ │ # FileRequestBody, LoggableRequestBody, + │ │ │ │ # MultipartField/MultipartRequestBody, Method + │ │ │ ├── response/ # Response, AsyncResponse, ResponseBody, + │ │ │ │ # AsyncResponseBody, LoggableResponseBody, + │ │ │ │ # Status │ │ │ ├── context/ # CallContext, DispatchContext, │ │ │ │ # RequestContext, ExchangeContext, │ │ │ │ # ContextStore + │ │ │ ├── sse/ # Server-Sent Events parser + connection + │ │ │ ├── webhooks/ # webhook signature verification │ │ │ └── auth/ # TokenCredential, BearerTokenPolicy, │ │ │ # BasicAuthPolicy, KeyCredentialPolicy, │ │ │ # ChallengeHandler (Basic/Digest/Composite), @@ -119,19 +130,24 @@ python-sdk/ │ │ │ │ # Stage, StagedPipelineBuilder, defaults, │ │ │ │ # sans-io + transport runners under the hood │ │ │ │ - │ │ │ ├── policies/ # retry, redirect, logging, tracing, - │ │ │ │ # set_date (+ async twins) - │ │ │ └── step/ # PipelineStep, StepMetadata, RetryConfig + │ │ │ ├── policies/ # redirect, idempotency, retry, set_date, + │ │ │ │ # client_identity, logging, tracing + │ │ │ │ # (async twins only for the first five) + │ │ │ └── step/ # PipelineStep, StepMetadata │ │ ├── client/ # HttpClient + AsyncHttpClient Protocols │ │ ├── config/ # Configuration │ │ ├── serde/ # Serde, Serializer, Deserializer Protocols │ │ ├── errors/ # SDK-level exception hierarchy │ │ ├── instrumentation/ # InstrumentationContext, Span, Tracer, - │ │ │ # TracingScope, noops + │ │ │ # TracingScope, noops, metrics, + │ │ │ # correlation, client_logger, http_tracer, + │ │ │ # identifiers, log_level, url_redactor + │ │ ├── pagination/ # Page, Paginator, link-header + strategy │ │ └── util/ # clock, proxy helpers │ └── tests/ # pytest suite — auth/, config/, context/, │ # errors/, http/, instrumentation/, - │ # pipeline/, serde/, sse/, util/ + │ # pagination/, pipeline/, serde/, sse/, + │ # util/, webhooks/ ├── dexpace-sdk-http-stdlib/ # reference stdlib transports: │ │ # UrllibHttpClient, AsyncioHttpClient │ └── src/dexpace/sdk/http/stdlib/ @@ -143,6 +159,10 @@ python-sdk/ └── src/dexpace/sdk/http/requests/ ``` +Community-health and tooling files (`CHANGELOG.md`, `CONTRIBUTING.md`, +`SECURITY.md`, `CODE_OF_CONDUCT.md`, `conftest.py`, `tools/`) are elided from +the tree above. + Every transport package depends on `dexpace-sdk-core` and adapts its HTTP library to the `HttpClient` / `AsyncHttpClient` Protocols. Namespace packaging (no `__init__.py` at `src/dexpace/`, `src/dexpace/sdk/`, or @@ -185,12 +205,16 @@ Layered, bottom-up: evict on `CallContext.close()`. 4. **`pipeline`** — `Policy` (and `AsyncPolicy`) wrap the downstream chain; `Pipeline` / `AsyncPipeline` run an ordered set of policies grouped into - `Stage`s via `StagedPipelineBuilder`. Shipped policies: retry, redirect, - logging, tracing, set-date (each with an async twin under - `pipeline/policies/`). `default_pipeline()` / `default_async_pipeline()` - assemble the standard stack. The lower-level `pipeline/step/PipelineStep` - Protocol (`(input, context) -> output`) plus `StepMetadata` / `RetryConfig` - remain for custom composition. + `Stage`s via `StagedPipelineBuilder`. Shipped policies: redirect, + idempotency, retry, set-date, client-identity, logging, tracing. Async + twins under `pipeline/policies/` exist only for redirect, idempotency, + retry, set-date, and client-identity; logging and tracing are sync-only. + `default_pipeline()` / `default_async_pipeline()` assemble the standard + stack in the order redirect → idempotency → retry → set-date → + client-identity → [auth] → logging → tracing (the async pipeline omits + logging and tracing). The lower-level `pipeline/step/PipelineStep` Protocol + (`(input, context) -> output`) plus `StepMetadata` remain for custom + composition. 5. **`client/HttpClient`** — single-method Protocol (`execute(request) -> Response`). Transport is **not** provided by `core`; the `dexpace-sdk-http-*` packages (stdlib, httpx, aiohttp, requests) each diff --git a/README.md b/README.md index 761de24..7611fb2 100644 --- a/README.md +++ b/README.md @@ -79,8 +79,9 @@ with UrllibHttpClient() as client, client.execute(request) as response: ### A configured pipeline `default_pipeline()` returns a `StagedPipelineBuilder` pre-wired with the -canonical policy stack (redirect, retry, set-date, logging, tracing). Add -authentication and adjust whatever the defaults get wrong for you: +canonical policy stack (redirect, idempotency, retry, set-date, +client-identity, logging, tracing). Add authentication and adjust whatever +the defaults get wrong for you: ```python from dexpace.sdk.core.http.auth import BearerTokenPolicy @@ -112,7 +113,8 @@ from dexpace.sdk.core.http.request import RequestBody RequestBody.from_stream(open("payload.bin", "rb")) RequestBody.from_iter([b"chunk-1", b"chunk-2"]) -# Replayable (transports may use zero-copy sendfile) +# Replayable; a transport could special-case file bodies (e.g. zero-copy +# sendfile), though none of the shipped transports do so today RequestBody.from_file("upload.bin") # Convert any single-use body into a replayable one before retrying @@ -130,8 +132,9 @@ the way back up. The terminal policy hands the request to an `HttpClient` transport. ``` -caller → Pipeline → REDIRECT → RETRY → SET_DATE → AUTH → LOGGING → POST_LOGGING → HttpClient → wire - (pillar) (pillar) (pillar) (pillar) +caller → Pipeline → REDIRECT → POST_REDIRECT → RETRY → POST_RETRY → [AUTH] → LOGGING → POST_LOGGING → HttpClient → wire + (pillar) idempotency (pillar) set-date (pillar) (pillar) tracing + client-identity ``` Ordering is governed by `Stage`, an `IntEnum` whose values sit 100 apart so @@ -169,12 +172,14 @@ Bottom-up, the layers are: | `http.common` | `Headers`, `HttpHeaderName`, `MediaType`, `Protocol`, `Url`, `QueryParams`, `ETag`, `HttpRange`, `RequestConditions`, paging primitives | | `http.context` | `CallContext` → `DispatchContext` → `RequestContext` → `ExchangeContext` chain, `ContextStore` | | `http.auth` | `BearerTokenPolicy`, `BasicAuthPolicy`, `KeyCredentialPolicy`, `DigestChallengeHandler`, RFC 7235 challenge parser, `TokenCache` | -| `http.sse` | `SseParser` for Server-Sent Events streams | +| `http.sse` | `SseParser`, plus reconnecting `SseConnection` / `AsyncSseConnection` (Last-Event-ID replay + backoff) | +| `http.webhooks` | `WebhookVerifier`, `InvalidWebhookSignatureError` — HMAC signature verification with timestamp tolerance | +| `pagination` | `Page`, `Paginator` / `AsyncPaginator`, `PaginationStrategy` (`CursorStrategy`, `PageNumberStrategy`, `LinkHeaderStrategy`) | | `pipeline` | `Pipeline`, `AsyncPipeline`, `Policy` ABC, `Stage` enum, `StagedPipelineBuilder`, `default_pipeline()` | -| `pipeline.policies` | `RetryPolicy`, `RedirectPolicy`, `SetDatePolicy`, `LoggingPolicy`, `TracingPolicy` (+ async twins) | +| `pipeline.policies` | `RedirectPolicy`, `IdempotencyPolicy`, `RetryPolicy`, `SetDatePolicy`, `ClientIdentityPolicy`, `LoggingPolicy`, `TracingPolicy` (async twins for all but logging/tracing) | | `client` | `HttpClient` and `AsyncHttpClient` Protocols | | `serde` | `Serde`, `Serializer`, `Deserializer` Protocols + `JsonSerde` reference impl | -| `instrumentation` | `ClientLogger`, `UrlRedactor`, `Tracer`, `Span`, `InstrumentationContext`, noop singletons | +| `instrumentation` | `ClientLogger`, `UrlRedactor`, `Tracer`, `Span`, `InstrumentationContext`, `contextvars` correlation helpers, noop singletons | | `errors` | `SdkError` hierarchy: `ServiceRequestError`, `ServiceResponseError`, `HttpResponseError[ModelT]`, … | | `util` | `Clock`, `AsyncClock`, `ProxyOptions` | | `config` | `Configuration` (layered env-var + override lookup) + `ConfigurationBuilder` | @@ -203,7 +208,16 @@ Bottom-up, the layers are: OpenTelemetry-compatible spans via `TracingPolicy`, URL redaction with allowlisted query parameters, and capped body capture for diagnostics. - **Server-Sent Events.** A WHATWG-compliant `SseParser` with a bounded - line buffer. + line buffer, plus reconnecting `SseConnection` / `AsyncSseConnection` + that resume with `Last-Event-ID` and honour server `retry:` backoff. +- **Pagination.** A top-level `pagination` package: `Page`, sync and async + `Paginator`s that iterate item-by-item or page-by-page, and pluggable + `PaginationStrategy` (cursor, page-number, and `Link`-header). +- **Webhooks.** `WebhookVerifier` checks HMAC signatures with a timestamp + tolerance and constant-time comparison, raising + `InvalidWebhookSignatureError` on mismatch. +- **Correlation.** `contextvars`-based trace/span propagation so the + idempotency and client-identity policies and logging share one id. - **A lean core.** `dexpace-sdk-core` carries a single runtime dependency (`furl`, which backs `Url` parsing); each transport adapter adds exactly one HTTP library. @@ -220,8 +234,8 @@ uv sync ``` ```bash -uv run pytest -q # 646 tests across 5 packages -uv run mypy --strict # type-check (171 source files) +uv run pytest -q # run the full test suite across 5 packages +uv run mypy --strict # type-check every package under strict mode uv run ruff check # lint uv run ruff format --check # formatting gate ``` diff --git a/docs/architecture.md b/docs/architecture.md index 35e4df2..63186a0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -17,7 +17,13 @@ plug in a concrete transport via the `HttpClient` Protocol. │ - Pipeline │ │ - BearerToken │ │ - Policy │ │ - KeyCredential │ │ - PipelineStep │ │ - BasicAuth │ - │ - retry, log, │ │ - TokenCache │ + │ - redirect, │ │ - TokenCache │ + │ idempotency, │ │ │ + │ retry, │ │ │ + │ set-date, │ │ │ + │ client- │ │ │ + │ identity, │ │ │ + │ logging, │ │ │ │ tracing │ │ │ └──────────┬────────┘ └─────────────────────┘ │ @@ -32,9 +38,13 @@ plug in a concrete transport via the `HttpClient` Protocol. │ ┌──────────▼─────────────────────────────────────────┐ │ client/ HttpClient + AsyncHttpClient │ + │ - Protocols only; transports plug in here │ + └──────────┬─────────────────────────────────────────┘ + │ + ┌──────────▼─────────────────────────────────────────┐ + │ dexpace-sdk-http-stdlib (separate distribution) │ │ - UrllibHttpClient (sync reference) │ │ - AsyncioHttpClient (async reference) │ - │ - real transports plug in here │ └────────────────────────────────────────────────────┘ ``` @@ -67,5 +77,5 @@ The Java port has an `IoProvider` / `Buffer` / `Source` / `Sink` layer (a port of Okio). In Python, `bytes` / `bytearray` / `memoryview` / `BytesIO` / `BinaryIO` already cover the same surface idiomatically. Bodies use `iter_bytes(chunk_size)` for streaming and ordinary stdlib -primitives for everything else. See `to-implement.md` for the design -rationale. +primitives for everything else. See the "Things That Will Bite You" +section in `CLAUDE.md` for the design rationale. diff --git a/docs/auth.md b/docs/auth.md index bbb33c9..73cc2ad 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -33,9 +33,10 @@ All concrete credentials redact secrets in their `__repr__`. - Calls `AccessTokenInfo.needs_refresh()` before each request; refreshes proactively when `refresh_on` has passed or `expires_on - leeway` (default 300 s) has been reached. -- On a 401 response with a `WWW-Authenticate` header, invalidates the - cached token and calls `on_challenge(request, response)`. Override - `on_challenge` in a subclass to handle CAE / claims-challenge flows. +- On any 401 response, invalidates the cached token. If the response + also carries a `WWW-Authenticate` header, then calls + `on_challenge(request, response)`. Override `on_challenge` in a + subclass to handle CAE / claims-challenge flows. ## Token cache diff --git a/docs/bodies.md b/docs/bodies.md index 3ae9304..be99c04 100644 --- a/docs/bodies.md +++ b/docs/bodies.md @@ -17,10 +17,13 @@ classmethod factories for the common shapes. | `RequestBody.from_iter(Iterable[bytes])` | ❌ | Same. The iterable is consumed on first `iter_bytes`. | Single-use bodies raise `RuntimeError` on the second `iter_bytes` call. -The retry policy in `pipeline.policies.retry` does **not** automatically -buffer single-use bodies — that is intentionally explicit: call -`body.to_replayable()` before the first send if the request is in a -retryable scope. +The retry policy in `pipeline.policies.retry` **does** automatically +buffer single-use bodies when retries are enabled: `RetryPolicy.send` +calls `body.to_replayable()` before the first attempt whenever +`total_retries > 0`, so a retry can re-emit the same payload. The +buffering step is skipped when `total_retries == 0`; if you bypass the +retry policy you can still call `body.to_replayable()` yourself before +the first send. ## Response shape @@ -56,6 +59,8 @@ the response side — first access drains the underlying body into a ## Async equivalents `AsyncRequestBody` / `AsyncResponseBody` mirror the sync APIs with -`aiter_bytes` and `async def bytes()` / `string()`. Factories: same -names, plus `from_async_stream` and `from_async_iter` for the -single-use shapes. +`aiter_bytes` and `async def bytes()` / `string()`. The factory sets +differ per side: `AsyncRequestBody` exposes `from_bytes`, `from_string`, +`from_form`, `from_async_stream`, and `from_async_iter`; +`AsyncResponseBody` exposes `from_bytes` and `from_async_stream` only +(there is no `from_async_iter` on the response side). diff --git a/docs/errors.md b/docs/errors.md index 1f6d0a3..3b92479 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -21,6 +21,7 @@ Exception ├─ StreamConsumedError # body already consumed ├─ StreamClosedError # body closed before reading ├─ ResponseNotReadError # attribute access before read + ├─ StreamingError # stream framing / decode error (e.g. SSE) ├─ PipelineAbortedError # SansIO step returned None ├─ SerializationError # also a ValueError └─ DeserializationError # also a ValueError @@ -54,8 +55,10 @@ with client.execute(request) as response: return response ``` -`ErrorMap` is a generic typed alternative that accepts a default error -type for unmatched status codes — useful when wrapping a long-tail API. +`map_error` only raises when `status_code` is a key in the supplied +map; an unmapped code (or a `None` map) is a no-op and returns without +raising. There is no default error type for unmatched codes — handle +the long tail yourself after `map_error` returns. ## Continuation tokens diff --git a/docs/http.md b/docs/http.md index 8e9adb1..9c3b228 100644 --- a/docs/http.md +++ b/docs/http.md @@ -7,12 +7,12 @@ helpers to derive a new instance. ## Request / Response ```python -from dexpace.sdk.core.http.common import Headers +from dexpace.sdk.core.http.common import Headers, Url from dexpace.sdk.core.http.request import Method, Request request = Request( method=Method.POST, - url="https://api.example.com/v1/items", + url=Url.parse("https://api.example.com/v1/items"), headers=Headers({"Content-Type": "application/json"}), ) updated = request.with_added_header("X-Trace", "abc") @@ -22,7 +22,7 @@ updated = request.with_added_header("X-Trace", "abc") Case-insensitive, multi-valued, immutable. Pass `HttpHeaderName` constants (`CONTENT_TYPE`, `AUTHORIZATION`, …) to skip the -`.lower().strip()` step on the hot path: +`.lower()` step on the hot path: ```python from dexpace.sdk.core.http.common.http_header_name import CONTENT_TYPE @@ -33,8 +33,10 @@ assert headers.get(CONTENT_TYPE) == "application/json" ## MediaType / Url / ETag / HttpRange / RequestConditions -Each is a `@dataclass(frozen=True, slots=True)` with a `parse(str)` -classmethod and a `__str__` that round-trips: +Each is a `@dataclass(frozen=True, slots=True)`. `MediaType`, `Url`, and +`ETag` expose a `parse(str)` classmethod and a `__str__` that round-trips. +`HttpRange` renders via `format()` / `to_header_value()`, and +`RequestConditions` applies itself onto a request via `apply_to(request)`: ```python from dexpace.sdk.core.http.common import ETag, HttpRange, MediaType, Url diff --git a/docs/pipelines.md b/docs/pipelines.md index df06aad..f0bb645 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -35,7 +35,7 @@ two parallel variants: ## Construction ```python -from dexpace.sdk.core.client import UrllibHttpClient +from dexpace.sdk.http.stdlib import UrllibHttpClient from dexpace.sdk.core.pipeline import Pipeline from dexpace.sdk.core.pipeline.policies import LoggingPolicy, RetryPolicy, TracingPolicy diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/client/http_client.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/client/http_client.py index 519cd60..bc7d11c 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/client/http_client.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/client/http_client.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -""":class:`HttpClient` Protocol.""" +"""`HttpClient` Protocol.""" from __future__ import annotations @@ -14,7 +14,7 @@ @runtime_checkable class HttpClient(Protocol): - """Transport seam: send one :class:`Request`, get one :class:`Response`. + """Transport seam: send one `Request`, get one `Response`. The SDK is an HTTP-client *toolkit*, not an HTTP client — consuming libraries plug in a transport by implementing this single-method Protocol. @@ -22,7 +22,7 @@ class HttpClient(Protocol): Implementations are expected to be safe for concurrent calls from multiple threads. Per-request state must be confined to local variables or to the - returned :class:`Response` graph. The response body is not pre-buffered — + returned `Response` graph. The response body is not pre-buffered — callers are responsible for closing it. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/config/configuration.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/config/configuration.py index f8f5aa1..456c55c 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/config/configuration.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/config/configuration.py @@ -21,6 +21,7 @@ from __future__ import annotations +import math import os import re from collections.abc import Callable @@ -167,13 +168,13 @@ def get_duration(self, name: str, default_seconds: float) -> float: Returns: Duration in seconds, or ``default_seconds`` on miss / failure / - negative result. + non-finite (``inf`` / ``nan``) / negative result. """ raw = self.get(name) if raw is None: return default_seconds parsed = _parse_duration_seconds(raw) - if parsed is None or parsed < 0: + if parsed is None or not math.isfinite(parsed) or parsed < 0: return default_seconds return parsed diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/__init__.py index f936a82..8a3abf9 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/__init__.py @@ -14,10 +14,10 @@ - ``HttpResponseError`` — a 4xx or 5xx response was received intact. Carries the response so callers can inspect status, headers, and body. -Plus exceptions for body lifecycle violations (``StreamConsumedError``, -``StreamClosedError``, ``ResponseNotReadError``), serialization -(``SerializationError``, ``DeserializationError``), and pipeline aborts -(``PipelineAbortedError``). +Plus exceptions for body / stream lifecycle violations +(``StreamConsumedError``, ``StreamClosedError``, ``ResponseNotReadError``, +``StreamingError``), serialization (``SerializationError``, +``DeserializationError``), and pipeline aborts (``PipelineAbortedError``). """ from __future__ import annotations diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/base.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/base.py index 6085c01..1d6c0dc 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/base.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/errors/base.py @@ -52,7 +52,7 @@ def __init__( exc_info = sys.exc_info() self.exc_type = exc_info[0] or (type(error) if error is not None else None) self.exc_value = exc_info[1] or error - self.exc_traceback = exc_info[2] + self.exc_traceback = exc_info[2] or (error.__traceback__ if error is not None else None) self.message = str(message) self.continuation_token = continuation_token super().__init__(self.message) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/_shielded.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/_shielded.py new file mode 100644 index 0000000..0c9d4d7 --- /dev/null +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/_shielded.py @@ -0,0 +1,75 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Cancellation-shielded cleanup shared by the async request/response bodies. + +Both the request and response sides release transport resources from a +``finally`` block that may run while an ``asyncio.CancelledError`` is already +propagating. ``_shielded_cleanup`` is the single convention they share to run +that release to completion before letting the cancellation continue. It lives +here — under ``http`` rather than ``http.response`` — so the request-side body +can import it without reaching into the response package and recreating the +import cycle between the two body modules. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Awaitable + + +async def _shielded_cleanup(cleanup: Awaitable[object]) -> None: + """Run a cleanup coroutine without letting cancellation interrupt it. + + This is the single cancellation convention used by the async request and + response bodies: a ``finally`` block that releases transport resources may + run while an ``asyncio.CancelledError`` is already propagating through the + enclosing task. Awaiting the cleanup directly would let that cancellation + interrupt it mid-way, leaking the underlying connection. + + The cleanup is wrapped in ``asyncio.shield`` so it always runs to + completion. If the surrounding scope is cancelled, the ``CancelledError`` + raised by ``shield`` is caught and the wait retried until the shielded + cleanup finishes; the cancellation is then re-raised so it continues to + propagate. Cleanup never swallows cancellation — it merely defers it + until the resource is released. A ``CancelledError`` raised because the + cleanup *itself* was cancelled is propagated immediately. + + A pending outer cancellation always wins: if the cleanup runs to + completion but raises an ordinary exception while a cancellation is + waiting, the cancellation is re-raised (the cleanup error does not mask + it). When no cancellation is pending, a cleanup failure surfaces to the + caller unchanged. + + Args: + cleanup: The resource-release coroutine to run to completion. + + Raises: + asyncio.CancelledError: Re-raised after the cleanup completes when + the enclosing scope was cancelled while the cleanup ran. + Exception: Whatever the cleanup coroutine raised, when no outer + cancellation is pending. + """ + inner = asyncio.ensure_future(cleanup) + cancelled = False + while not inner.done(): + try: + await asyncio.shield(inner) + except asyncio.CancelledError: + if inner.cancelled(): + # The cleanup itself was cancelled, not just our wait on it. + raise + # An outer cancellation hit our wait, not the shielded cleanup. + # Keep waiting until the cleanup finishes, then re-raise so the + # cancellation continues to propagate. + cancelled = True + except Exception: + # The cleanup failed; ``inner`` retains the exception, surfaced + # below. A pending cancellation still takes precedence. + break + if cancelled: + raise asyncio.CancelledError + inner.result() + + +__all__ = ["_shielded_cleanup"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/access_token.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/access_token.py index dfe5b02..91563ff 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/access_token.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/access_token.py @@ -67,7 +67,7 @@ def needs_refresh( seconds (default 5 minutes). clock: Optional ``Clock`` (or ``AsyncClock``) used as the time source when ``now`` is not supplied. Defaults to - :data:`~dexpace.sdk.core.util.clock.SYSTEM_CLOCK`. + `SYSTEM_CLOCK`. ``expires_on`` is wall-clock, so ``clock.now()`` (not ``monotonic()``) is consulted. ``AsyncClock`` is accepted because its ``now()`` method is synchronous — only diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/challenge.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/challenge.py index 1c6ac6f..a88818b 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/challenge.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/challenge.py @@ -8,13 +8,20 @@ the scheme + parameter map for each. Quoted-string values are unquoted and ``quoted-pair`` escapes (``\\X`` → ``X``) decoded per RFC 7230 §3.2.6. +Auth-params within a challenge must be comma-separated; this parser does not +recover whitespace-separated parameters. ``token68`` credentials (as used by +the ``Negotiate`` and ``NTLM`` schemes) are not supported — only ``scheme +auth-param`` challenges are recognised. + Malformed tokens are skipped rather than aborting the parse; callers see whatever valid challenges were extracted. """ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass +from types import MappingProxyType @dataclass(frozen=True, slots=True) @@ -25,18 +32,22 @@ class AuthenticateChallenge: scheme: The auth scheme name as it appeared on the wire (e.g. ``"Basic"``, ``"Digest"``, ``"Bearer"``). Compare with ``casefold()`` for case-insensitive matching. - parameters: Parameter map for the challenge. Keys are lower-cased on - parse so lookups are case-insensitive per RFC 7235 §2.1. - Values preserve their original casing. + parameters: Read-only parameter map for the challenge. Keys are + lower-cased on parse so lookups are case-insensitive per RFC 7235 + §2.1. Values preserve their original casing. """ scheme: str - parameters: dict[str, str] + parameters: Mapping[str, str] def parse_challenges(header_value: str) -> list[AuthenticateChallenge]: """Parse a ``WWW-Authenticate`` or ``Proxy-Authenticate`` header. + Auth-params within a challenge are recognised only when comma-separated; + whitespace-separated params are not recovered. ``token68`` credentials + (e.g. ``Negotiate``/``NTLM``) are unsupported and yield no parameters. + Args: header_value: The full header value (a single line concatenated across folded continuations is fine — RFC 7230 deprecates @@ -48,32 +59,37 @@ def parse_challenges(header_value: str) -> list[AuthenticateChallenge]: """ tokens = _split_top_level(header_value) challenges: list[AuthenticateChallenge] = [] - current: AuthenticateChallenge | None = None + current_scheme: str | None = None + current_params: dict[str, str] = {} for token in tokens: token = token.strip() if not token: continue - scheme, params, has_params = _try_parse_scheme_start(token) + scheme, params, _has_params = _try_parse_scheme_start(token) if scheme is not None: - if current is not None: - challenges.append(current) - current = AuthenticateChallenge(scheme=scheme, parameters=params) - if not has_params: - continue + if current_scheme is not None: + challenges.append(_freeze(current_scheme, current_params)) + current_scheme = scheme + current_params = params continue # Otherwise this token is a continuation parameter for the current # challenge: ``key=value`` or ``key="quoted"``. - if current is None: + if current_scheme is None: continue key, value = _try_parse_param(token) if key is None or value is None: continue - current.parameters[key] = value - if current is not None: - challenges.append(current) + current_params[key] = value + if current_scheme is not None: + challenges.append(_freeze(current_scheme, current_params)) return challenges +def _freeze(scheme: str, params: dict[str, str]) -> AuthenticateChallenge: + """Build a challenge whose parameters are a read-only view.""" + return AuthenticateChallenge(scheme=scheme, parameters=MappingProxyType(dict(params))) + + def _split_top_level(value: str) -> list[str]: """Split on commas at the top level, respecting quoted strings.""" out: list[str] = [] @@ -114,7 +130,9 @@ def _try_parse_scheme_start( """Detect a ``scheme [param]`` token. A scheme is recognised when the first whitespace-delimited word is a - valid token68/token AND it is not itself a ``key=value`` pair. Returns + valid token AND it is not itself a ``key=value`` pair. ``token68`` + credentials (e.g. ``Negotiate``/``NTLM``) are not parsed — only + comma-separated ``key=value`` auth-params are recovered. Returns ``(scheme, params, has_params)``. If ``token`` is purely a continuation parameter, returns ``(None, {}, False)``. """ @@ -154,7 +172,11 @@ def _try_parse_scheme_start( def _split_param_list(value: str) -> list[str]: - """Split a parameter list (space or comma separated) at the top level.""" + """Split a comma-separated parameter list at the top level. + + Auth-params must be comma-separated; whitespace alone does not delimit + parameters, so a whitespace-separated run collapses into one segment. + """ out: list[str] = [] buf: list[str] = [] in_quote = False diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/digest.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/digest.py index c5e50c0..7ec85ce 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/digest.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/digest.py @@ -19,6 +19,7 @@ import secrets import threading from collections.abc import Callable +from dataclasses import dataclass from typing import Final from ..common.url import Url @@ -28,6 +29,21 @@ # Type alias for the algorithm parameter strings recognised on the wire. DigestAlgorithm = str + +@dataclass(frozen=True, slots=True) +class _ResolvedChallenge: + """The validated parameters extracted from a selected Digest challenge.""" + + algorithm: str + algo_key: str + hasher: Callable[[bytes], hashlib._Hash] + realm: str + nonce: str + opaque: str | None + charset: str + qop: str | None + + _DEFAULT_PREFERENCE: Final[tuple[DigestAlgorithm, ...]] = ( "SHA-256-sess", "SHA-256", @@ -51,7 +67,11 @@ class DigestChallengeHandler: password: Secret used to compute ``HA1``. preferred_algorithms: Ordered preference for the algorithm parameter when the server offers more than one Digest challenge. The - first preference matched against an offered challenge wins. + first preference matched against an offered challenge wins. The + tuple also acts as an allow-list: an offered algorithm outside it + is never selected, even as a fallback, so narrowing the tuple to + ``("SHA-256",)`` declines an ``MD5``-only server. The default + contains every supported algorithm, so ``MD5`` stays reachable. cnonce_factory: Override for the client nonce generator. Defaults to ``secrets.token_hex(16)``; tests inject a deterministic value to assert against RFC fixtures. @@ -99,55 +119,95 @@ def handle( selected = self._select(challenges) if selected is None: return None - algorithm = selected.parameters.get("algorithm", "MD5") - algo_key = algorithm.upper() - hasher = _HASHERS.get(algo_key) - if hasher is None: - return None - realm = selected.parameters.get("realm", "") - nonce = selected.parameters.get("nonce", "") - opaque = selected.parameters.get("opaque") - charset = _select_charset(selected.parameters.get("charset")) - qop = self._pick_qop(selected.parameters.get("qop")) - if qop is None and "qop" in selected.parameters: - # The server advertised qop but did not include ``auth``: we - # only implement auth, so we cannot satisfy this challenge. + resolved = self._resolve(selected) + if resolved is None: return None nc = self._next_nc() cnonce = self._cnonce_factory() uri = _request_uri(url) response = self._compute_response( - hasher=hasher, - algorithm=algo_key, + hasher=resolved.hasher, + algorithm=resolved.algo_key, method=str(method), uri=uri, - realm=realm, - nonce=nonce, + realm=resolved.realm, + nonce=resolved.nonce, nc=nc, cnonce=cnonce, - qop=qop, - charset=charset, + qop=resolved.qop, + charset=resolved.charset, ) header_value = _format_header( username=self._username, - realm=realm, - nonce=nonce, + realm=resolved.realm, + nonce=resolved.nonce, uri=uri, response=response, - algorithm=algorithm, + algorithm=resolved.algorithm, cnonce=cnonce, nc=nc, - qop=qop, - opaque=opaque, + qop=resolved.qop, + opaque=resolved.opaque, ) header_name = "Proxy-Authorization" if is_proxy else "Authorization" return header_name, header_value + def _resolve(self, selected: AuthenticateChallenge) -> _ResolvedChallenge | None: + """Validate and extract the parameters needed to answer ``selected``. + + Returns ``None`` (decline) when the algorithm is unsupported, the + credentials are not encodable under the negotiated charset, or the + server advertised a ``qop`` we do not implement. + """ + algorithm = selected.parameters.get("algorithm", "MD5") + algo_key = algorithm.upper() + hasher = _HASHERS.get(algo_key) + if hasher is None: + return None + charset = _select_charset(selected.parameters.get("charset")) + if not self._credentials_encodable(charset): + # A charset-less challenge defaults to ISO-8859-1, which cannot + # represent CJK/emoji credentials. Decline rather than letting a + # ``UnicodeEncodeError`` escape the documented contract. + return None + qop = self._pick_qop(selected.parameters.get("qop")) + if qop is None and "qop" in selected.parameters: + # The server advertised qop but did not include ``auth``: we + # only implement auth, so we cannot satisfy this challenge. + return None + if qop is None and algo_key.endswith("-SESS"): + # A session variant folds ``cnonce`` into HA1, but without ``qop`` + # the response header omits ``cnonce``/``nc`` (RFC 7616 §3.4), so + # the server cannot reconstruct HA1. Such a challenge is + # self-contradictory (session variants were introduced alongside + # qop); decline rather than emit an unverifiable header. + return None + return _ResolvedChallenge( + algorithm=algorithm, + algo_key=algo_key, + hasher=hasher, + realm=selected.parameters.get("realm", ""), + nonce=selected.parameters.get("nonce", ""), + opaque=selected.parameters.get("opaque"), + charset=charset, + qop=qop, + ) + def _select(self, challenges: list[AuthenticateChallenge]) -> AuthenticateChallenge | None: - """Return the best Digest challenge per ``preferred_algorithms``.""" + """Return the best Digest challenge per ``preferred_algorithms``. + + ``preferred_algorithms`` is both a preference order and an allow-list: + an offered algorithm is only accepted if it appears in the tuple. The + first preference matched against an offered challenge wins; failing an + exact-preference match, the first challenge whose algorithm is still + within the allow-list is taken. The default preference contains every + supported algorithm, so ``MD5`` remains reachable unless the caller + narrows the tuple to exclude it. + """ digest_challenges = [c for c in challenges if c.scheme.casefold() == "digest"] if not digest_challenges: return None + allowed = {preferred.upper() for preferred in self._preferred} # First, match by preference order. for preferred in self._preferred: target = preferred.upper() @@ -155,10 +215,11 @@ def _select(self, challenges: list[AuthenticateChallenge]) -> AuthenticateChalle algo = challenge.parameters.get("algorithm", "MD5").upper() if algo == target and algo in _HASHERS: return challenge - # Fallback: take the first challenge whose algorithm we recognise. + # Fallback: the first challenge whose algorithm we recognise *and* + # the caller allowed via ``preferred_algorithms``. for challenge in digest_challenges: algo = challenge.parameters.get("algorithm", "MD5").upper() - if algo in _HASHERS: + if algo in _HASHERS and algo in allowed: return challenge return None @@ -171,6 +232,15 @@ def _next_nc(self) -> str: value = self._counter return f"{value:08x}" + def _credentials_encodable(self, charset: str) -> bool: + """Report whether username/password survive ``charset`` encoding.""" + try: + self._username.encode(charset) + self._password.encode(charset) + except UnicodeEncodeError: + return False + return True + @staticmethod def _pick_qop(qop_param: str | None) -> str | None: if qop_param is None: @@ -256,11 +326,13 @@ def _format_header( f'response="{response}"', # ``algorithm`` is conventionally sent unquoted (RFC 7616 §3.4). f"algorithm={algorithm}", - f'cnonce="{_quote(cnonce)}"', - # ``nc`` is conventionally unquoted (8 lowercase hex digits). - f"nc={nc}", ] if qop is not None: + # RFC 7616 §3.4: ``cnonce`` and ``nc`` are sent only alongside + # ``qop``. A qop-less (RFC 2069) response must omit both. + parts.append(f'cnonce="{_quote(cnonce)}"') + # ``nc`` is conventionally unquoted (8 lowercase hex digits). + parts.append(f"nc={nc}") parts.append(f"qop={qop}") if opaque is not None: parts.append(f'opaque="{_quote(opaque)}"') diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/policies.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/policies.py index a45f586..f36edac 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/policies.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/auth/policies.py @@ -147,6 +147,9 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: handler_header = self._apply_challenge_handler(request, response, status) if handler_header is not None: request = request.with_header(*handler_header) + # The rejected challenge response is not handed back; close it to + # release the pooled connection before the authenticated retry. + response.close() response = self.next.send(request, ctx) if int(response.status) not in (401, 407): return response @@ -157,6 +160,8 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: return response if "WWW-Authenticate" in response.headers and self.on_challenge(request, response): request = self._authorize(request, ctx, force_refresh=True) + # Release the rejected 401 before retrying with the refreshed token. + response.close() response = self.next.send(request, ctx) if int(response.status) != 401: return response @@ -245,10 +250,25 @@ class AsyncBearerTokenPolicy(AsyncPolicy): double-checked pattern; the underlying ``AsyncTokenCredential`` is invoked at most once per refresh window even when many tasks call ``send`` concurrently. + + Like the sync policy, an optional ``challenge_handler`` plugs in a + scheme-aware handler (e.g. ``DigestChallengeHandler``); it is consulted + before ``on_challenge`` on 401/407 and, if it satisfies the challenge, + the returned ``(name, value)`` pair is stamped on the retried request. + A 401 invalidates the cached origin token; a 407 leaves it alone because + the proxy, not the origin, rejected the request. """ STAGE = Stage.AUTH - __slots__ = ("_audience", "_cache", "_clock", "_credential", "_lock", "_scopes") + __slots__ = ( + "_audience", + "_cache", + "_challenge_handler", + "_clock", + "_credential", + "_lock", + "_scopes", + ) def __init__( self, @@ -257,6 +277,7 @@ def __init__( cache: TokenCache | None = None, audience: str | None = None, clock: AsyncClock | None = None, + challenge_handler: ChallengeHandler | None = None, ) -> None: if not scopes: raise ValueError("at least one scope is required") @@ -269,20 +290,70 @@ def __init__( # variants), so the protocol mismatch on ``sleep`` is irrelevant. self._clock: AsyncClock = clock if clock is not None else ASYNC_SYSTEM_CLOCK self._lock = asyncio.Lock() + self._challenge_handler = challenge_handler async def send(self, request: Request, ctx: PipelineContext) -> AsyncResponse: request = await self._authorize(request, ctx) response = await self.next.send(request, ctx) - if int(response.status) != 401: + status = int(response.status) + if status not in (401, 407): + return response + # On 401 the bearer token was rejected: drop it from the cache so the + # ``on_challenge`` fallback path forces a refresh. A 407 means the + # proxy rejected us, not the origin — leave the cached token alone. + if status == 401: + self._cache.set(self._scopes, _expired_token(), self._audience) + handler_header = self._apply_challenge_handler(request, response, status) + if handler_header is not None: + request = request.with_header(*handler_header) + # The rejected challenge response is not handed back; close it to + # release the pooled connection before the authenticated retry. + await response.close() + response = await self.next.send(request, ctx) + if int(response.status) not in (401, 407): + return response + raise ClientAuthenticationError(response=response) + if status != 401: + # No handler match on a 407; surface the response unchanged so + # callers can inspect proxy-auth failures themselves. return response - self._cache.set(self._scopes, _expired_token(), self._audience) if "WWW-Authenticate" in response.headers and await self.on_challenge(request, response): request = await self._authorize(request, ctx, force_refresh=True) + # Release the rejected 401 before retrying with the refreshed token. + await response.close() response = await self.next.send(request, ctx) if int(response.status) != 401: return response raise ClientAuthenticationError(response=response) + def _apply_challenge_handler( + self, + request: Request, + response: AsyncResponse, + status: int, + ) -> tuple[str, str] | None: + """Delegate to the configured ``ChallengeHandler``, if any. + + Returns the ``(name, value)`` pair to stamp on the retry, or ``None`` + when no handler is configured or the handler declines the challenge. + """ + if self._challenge_handler is None: + return None + is_proxy = status == 407 + header_name = "Proxy-Authenticate" if is_proxy else "WWW-Authenticate" + raw = response.headers.get(header_name) + if raw is None: + return None + challenges = parse_challenges(raw) + if not challenges or not self._challenge_handler.can_handle(challenges): + return None + return self._challenge_handler.handle( + request.method, + request.url, + challenges, + is_proxy=is_proxy, + ) + async def on_challenge( self, request: Request, diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/common_media_types.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/common_media_types.py index 7becf85..bf63dd6 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/common_media_types.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/common_media_types.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Pre-constructed :class:`MediaType` constants for the most common content types. +"""Pre-constructed `MediaType` constants for the most common content types. Exposed as module-level constants. Reusing these avoids re-parsing the same media-type string on hot paths. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/etag.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/etag.py index 94d3fb3..41797d9 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/etag.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/etag.py @@ -83,8 +83,10 @@ def parse(cls, raw: str) -> Self: The parsed ETag. Raises: - ValueError: If ``raw`` is not a valid quoted entity-tag, or - if the quoted body is empty. + ValueError: If ``raw`` is not a valid quoted entity-tag, if the + quoted body is empty, or if the body contains a double-quote, + space, or control character (forbidden in the opaque tag by + RFC 7232 §2.3). """ text = raw.strip() weak = False @@ -96,6 +98,13 @@ def parse(cls, raw: str) -> Self: value = text[1:-1] if not value: raise ValueError(f"Invalid ETag: empty quoted body in {raw!r}") + if '"' in value: + raise ValueError(f"Invalid ETag: embedded double-quote in {raw!r}") + # RFC 7232 §2.3: etagc = %x21 / %x23-7E / obs-text. Everything at or + # below SP (0x20) and DEL (0x7F) is outside the entity-tag character + # set; obs-text (0x80-0xFF) stays permitted. + if any(ord(ch) <= 0x20 or ord(ch) == 0x7F for ch in value): + raise ValueError(f"Invalid ETag: illegal character in {raw!r}") return cls(value=value, weak=weak) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/headers.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/headers.py index 6f30ffa..48aafde 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/headers.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/headers.py @@ -26,7 +26,7 @@ type _HeaderValue = str | Iterable[str] # ``Mapping[str, …]`` rather than ``Mapping[_Name, …]`` because ``dict[str, T]`` # is not a subtype of ``Mapping[str | X, T]`` (key invariance). Callers -# wanting :class:`HttpHeaderName` keys can pass the iterable-of-pairs form. +# wanting `HttpHeaderName` keys can pass the iterable-of-pairs form. type _Entries = Mapping[str, _HeaderValue] | Iterable[tuple[_Name, _HeaderValue]] @@ -37,8 +37,8 @@ class Headers: membership, and equality are all case-insensitive. Insertion order of distinct names is preserved. - Multi-value semantics: :meth:`with_added` appends to the values list for - a name; :meth:`with_set` replaces the entire list. This matches the HTTP + Multi-value semantics: `with_added` appends to the values list for + a name; `with_set` replaces the entire list. This matches the HTTP requirement that some headers (``Set-Cookie``, ``WWW-Authenticate``, ``Via``) may legitimately repeat. @@ -61,10 +61,7 @@ def __init__(self, entries: _Entries | None = None) -> None: existing = data.get(key, ()) new_values: tuple[str, ...] = (value,) if isinstance(value, str) else tuple(value) for v in new_values: - if "\r" in v or "\n" in v or "\0" in v: - raise ValueError( - f"invalid header value for {key!r}: contains control characters" - ) + _check_value(key, v) data[key] = (*existing, *new_values) object.__setattr__(self, "_data", tuple(data.items())) object.__setattr__(self, "_hash", None) @@ -124,6 +121,7 @@ def items(self) -> tuple[tuple[str, tuple[str, ...]], ...]: def with_added(self, name: _Name, value: str) -> Self: """Return a new ``Headers`` with ``value`` appended to ``name``'s list.""" target = _normalize(name) + _check_value(target, value) entries: list[tuple[str, tuple[str, ...]]] = [] appended = False for key, values in self._data: @@ -144,6 +142,8 @@ def with_set(self, name: _Name, *values: str) -> Self: if not values: return self.without(name) target = _normalize(name) + for value in values: + _check_value(target, value) entries: list[tuple[str, tuple[str, ...]]] = [] replaced = False for key, existing in self._data: @@ -170,6 +170,8 @@ def with_merged(self, other: Headers) -> Self: """Append every entry from ``other`` to this headers.""" merged: dict[str, tuple[str, ...]] = dict(self._data) for key, values in other._data: + for value in values: + _check_value(key, value) merged[key] = merged.get(key, ()) + values return _construct(type(self), tuple(merged.items())) @@ -177,10 +179,10 @@ def __or__(self, other: Headers) -> Self: """Return ``self.with_merged(other)`` — enables ``a | b`` syntax. Args: - other: Another :class:`Headers` whose entries are appended. + other: Another `Headers` whose entries are appended. Returns: - A new :class:`Headers` combining both sides; ``self``'s values for + A new `Headers` combining both sides; ``self``'s values for each name appear before ``other``'s. """ if not isinstance(other, Headers): @@ -211,7 +213,7 @@ def __repr__(self) -> str: @classmethod def empty(cls) -> Headers: - """Return the shared empty :class:`Headers` instance.""" + """Return the shared empty `Headers` instance.""" return _EMPTY @@ -227,6 +229,13 @@ def _normalize(name: _Name) -> str: return lowered +def _check_value(key: str, value: str) -> None: + # Reject CR, LF, and NUL to prevent header injection / response splitting. + # ``key`` is the already-normalised name, used only for the error message. + if "\r" in value or "\n" in value or "\0" in value: + raise ValueError(f"invalid header value for {key!r}: contains control characters") + + def _construct[H: Headers]( cls: type[H], data: tuple[tuple[str, tuple[str, ...]], ...], diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_header_name.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_header_name.py index 2f69a67..644b382 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_header_name.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_header_name.py @@ -31,6 +31,15 @@ class HttpHeaderName: value: str canonical_name: str + def __post_init__(self) -> None: + # Normalise ``value`` to lower case so a hand-built instance (e.g. + # ``HttpHeaderName('Content-Type', 'Content-Type')``) still matches the + # lower-cased keys ``Headers`` stores, and so equality/hashing key on a + # canonical form regardless of the casing passed in. + lowered = self.value.lower() + if lowered != self.value: + object.__setattr__(self, "value", lowered) + @classmethod def of(cls, canonical_name: str) -> Self: """Build from a canonical name, deriving the lower-case form. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_range.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_range.py index 41f829e..fae4276 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_range.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/http_range.py @@ -13,46 +13,60 @@ class HttpRange: """A single byte range. - Two shapes are supported: + Three shapes are supported: - Bounded: ``HttpRange(N, M)`` → ``bytes=N-(N+M-1)``. - Open-ended: ``HttpRange(N)`` → ``bytes=N-`` ("from N to the end"). + - Suffix: ``HttpRange.suffix(N)`` → ``bytes=-N`` (the last N bytes). - Last-N-bytes ranges (``bytes=-N``) are not representable directly because - ``start`` is constrained to be non-negative. Use ``HttpRange.suffix(N)`` - to construct the suffix variant, which is emitted by a sibling type. + A suffix range sets ``is_suffix`` and reuses ``count`` as the number of + trailing bytes; ``start`` is ignored for that shape. Because a suffix + range is an ordinary ``HttpRange``, it can be mixed freely into a + ``Sequence[HttpRange]`` passed to ``format_many``. Prefer the + ``HttpRange.suffix`` factory over constructing one with ``is_suffix=True`` + directly. HTTP allows multipart ranges (``bytes=0-99,200-299``); model those as a sequence of ``HttpRange`` and join via ``HttpRange.format_many`` when emitting a header value. Attributes: - start: First byte index (inclusive). Non-negative. - count: Number of bytes to fetch, or ``None`` for open-ended. + start: First byte index (inclusive). Non-negative. Ignored when + ``is_suffix`` is set. + count: Number of bytes to fetch, or ``None`` for open-ended. For a + suffix range, the number of trailing bytes (required, positive). + is_suffix: When ``True``, serialise as ``bytes=-count`` instead of a + ``start``-anchored range. Raises: - ValueError: If ``start`` is negative, or ``count`` is set but - non-positive. + ValueError: If ``start`` is negative; if ``count`` is set but + non-positive; or if ``is_suffix`` is set without a positive + ``count``. """ start: int = 0 count: int | None = None + is_suffix: bool = False def __post_init__(self) -> None: if self.start < 0: raise ValueError(f"start must be non-negative, got {self.start}") if self.count is not None and self.count <= 0: raise ValueError(f"count must be positive when set, got {self.count}") + if self.is_suffix and self.count is None: + raise ValueError("suffix range requires a positive count") @property def end(self) -> int | None: - """Inclusive end byte index, or ``None`` for open-ended ranges.""" - if self.count is None: + """Inclusive end byte index, or ``None`` for open-ended or suffix ranges.""" + if self.count is None or self.is_suffix: return None return self.start + self.count - 1 def format(self) -> str: """Format as the value portion of a ``Range`` header (no ``bytes=`` prefix).""" + if self.is_suffix: + return f"-{self.count}" end = self.end if end is None: return f"{self.start}-" @@ -63,21 +77,22 @@ def to_header_value(self) -> str: return f"bytes={self.format()}" @classmethod - def suffix(cls, count: int) -> _SuffixHttpRange: + def suffix(cls, count: int) -> HttpRange: """Construct a last-N-bytes range (``bytes=-N``). Args: count: Number of trailing bytes to request. Must be positive. Returns: - A ``_SuffixHttpRange`` that serialises to ``bytes=-N``. + An ``HttpRange`` with ``is_suffix`` set that serialises to + ``bytes=-N``. Raises: ValueError: If ``count`` is non-positive. """ if count <= 0: raise ValueError(f"suffix count must be positive, got {count}") - return _SuffixHttpRange(count=count) + return cls(count=count, is_suffix=True) @classmethod def format_many(cls, ranges: Sequence[HttpRange]) -> str: @@ -98,17 +113,4 @@ def format_many(cls, ranges: Sequence[HttpRange]) -> str: return f"bytes={joined}" -@dataclass(frozen=True, slots=True) -class _SuffixHttpRange: - """``bytes=-N`` — the last ``count`` bytes of the resource.""" - - count: int - - def format(self) -> str: - return f"-{self.count}" - - def to_header_value(self) -> str: - return f"bytes={self.format()}" - - __all__ = ["HttpRange"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/media_type.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/media_type.py index d5c9677..7f21c82 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/media_type.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/media_type.py @@ -60,8 +60,8 @@ class MediaType: Immutable and hashable. ``parameters`` is stored as a tuple of sorted ``(key, value)`` pairs so two equivalent media types compare equal and hash - equally regardless of construction order. Construct via :meth:`of` or - :meth:`parse` rather than the dataclass constructor directly so the type, + equally regardless of construction order. Construct via `of` or + `parse` rather than the dataclass constructor directly so the type, subtype, and parameter keys are normalised to lower case. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/pagination.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/pagination.py index 9be9af8..bce316d 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/pagination.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/pagination.py @@ -18,7 +18,6 @@ Iterable, Iterator, ) -from typing import Any from ...errors import SdkError @@ -65,21 +64,42 @@ def __next__(self) -> Iterator[T]: return iter(items) -class ItemPaged[T](Iterator[T]): +class ItemPaged[T, R](Iterator[T]): """Flat iterator over the items of a paged response. - Wraps a :class:`Pager` and yields individual items rather than pages. - Use :meth:`by_page` when page-level iteration is required. + Wraps a ``Pager`` and yields individual items rather than pages. Use + ``by_page`` when page-level iteration is required. + + The second type parameter ``R`` is the page type produced by ``get_next`` + and consumed by ``extract_data``. """ - def __init__(self, *args: Any, **kwargs: Any) -> None: - self._args = args - self._kwargs = kwargs + __slots__ = ("_extract_data", "_flat", "_get_next") + + def __init__( + self, + get_next: Callable[[str | None], R], + extract_data: Callable[[R], tuple[str | None, Iterable[T]]], + ) -> None: + self._get_next = get_next + self._extract_data = extract_data self._flat: Iterator[T] | None = None def by_page(self, continuation_token: str | None = None) -> Iterator[Iterator[T]]: - kwargs = {**self._kwargs, "continuation_token": continuation_token} - return Pager(*self._args, **kwargs) + """Return a page-level iterator, optionally resuming from a token. + + Args: + continuation_token: When set, resume paging from that page rather + than the first. + + Returns: + An iterator yielding one ``Iterator[T]`` per page. + """ + return Pager( + self._get_next, + self._extract_data, + continuation_token=continuation_token, + ) def __iter__(self) -> Iterator[T]: return self @@ -126,12 +146,22 @@ async def __anext__(self) -> AsyncIterator[T]: return _SyncToAsync(iter(items)) -class AsyncItemPaged[T](AsyncIterator[T]): - """Flat async iterator over items of a paged response.""" +class AsyncItemPaged[T, R](AsyncIterator[T]): + """Flat async iterator over items of a paged response. - def __init__(self, *args: Any, **kwargs: Any) -> None: - self._args = args - self._kwargs = kwargs + The second type parameter ``R`` is the page type produced by ``get_next`` + and consumed by ``extract_data``. + """ + + __slots__ = ("_current", "_extract_data", "_get_next", "_pages") + + def __init__( + self, + get_next: Callable[[str | None], Awaitable[R]], + extract_data: Callable[[R], Awaitable[tuple[str | None, Iterable[T]]]], + ) -> None: + self._get_next = get_next + self._extract_data = extract_data self._pages: AsyncIterator[AsyncIterator[T]] | None = None self._current: AsyncIterator[T] | None = None @@ -139,8 +169,20 @@ def by_page( self, continuation_token: str | None = None, ) -> AsyncIterator[AsyncIterator[T]]: - kwargs = {**self._kwargs, "continuation_token": continuation_token} - return AsyncPager(*self._args, **kwargs) + """Return a page-level async iterator, optionally resuming from a token. + + Args: + continuation_token: When set, resume paging from that page rather + than the first. + + Returns: + An async iterator yielding one ``AsyncIterator[T]`` per page. + """ + return AsyncPager( + self._get_next, + self._extract_data, + continuation_token=continuation_token, + ) def __aiter__(self) -> AsyncIterator[T]: return self diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/protocol.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/protocol.py index fb6563a..9a86ddf 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/protocol.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/protocol.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""HTTP protocol versions describable on a :class:`Response`.""" +"""HTTP protocol versions describable on a `Response`.""" from __future__ import annotations @@ -12,7 +12,7 @@ class Protocol(StrEnum): """Wire-protocol identifier negotiated for an HTTP exchange. - The wire form (returned by ``__str__`` and consumed by :meth:`parse`) is + The wire form (returned by ``__str__`` and consumed by `parse`) is lower-case with a slash separator, matching ALPN identifiers. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/request_conditions.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/request_conditions.py index 5cbffea..f0238aa 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/request_conditions.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/common/request_conditions.py @@ -83,13 +83,14 @@ def _format_etags(tags: Sequence[ETag]) -> str: def _format_http_date(value: datetime) -> str: - # email.utils.format_datetime produces the IMF-fixdate form (RFC 7231 - # §7.1.1.1) when given a timezone-aware datetime. Force UTC if naive — - # callers passing local-time naive datetimes is a common bug source and - # silently misinterpreting them produces wrong cache behaviour. - if value.tzinfo is None: - value = value.replace(tzinfo=UTC) - return format_datetime(value, usegmt=True) + # email.utils.format_datetime(usegmt=True) produces the IMF-fixdate form + # (RFC 7231 §7.1.1.1) but requires tzinfo to be exactly timezone.utc; any + # other offset raises ValueError. Treat a naive datetime as UTC — passing + # local-time naive datetimes is a common bug source and silently + # misinterpreting them produces wrong cache behaviour — and convert an + # aware datetime with any other offset to UTC before formatting. + normalized = value.replace(tzinfo=UTC) if value.tzinfo is None else value.astimezone(UTC) + return format_datetime(normalized, usegmt=True) __all__ = ["RequestConditions"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/__init__.py index b81f691..4e945c6 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/__init__.py @@ -5,14 +5,14 @@ A call moves through three immutable context shapes as it executes: -1. :class:`DispatchContext` — carries only the :class:`InstrumentationContext`; +1. `DispatchContext` — carries only the `InstrumentationContext`; created before a request payload exists. -2. :class:`RequestContext` — adds the outgoing :class:`Request`; produced by - :meth:`DispatchContext.to_request_context` once a request has been built. -3. :class:`ExchangeContext` — adds the :class:`Response`; produced by - :meth:`RequestContext.to_exchange_context` once a response has arrived. +2. `RequestContext` — adds the outgoing `Request`; produced by + `DispatchContext.to_request_context` once a request has been built. +3. `ExchangeContext` — adds the `Response`; produced by + `RequestContext.to_exchange_context` once a response has arrived. -Each promotion is registered with :data:`ContextStore` under the call's trace +Each promotion is registered with `ContextStore` under the call's trace id so downstream observers see the latest snapshot for a call by trace id. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/call_context.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/call_context.py index cc00a87..f901d69 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/call_context.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/call_context.py @@ -5,6 +5,7 @@ from __future__ import annotations +import abc from types import TracebackType from typing import TYPE_CHECKING, Self @@ -12,23 +13,23 @@ from ...instrumentation import InstrumentationContext -class CallContext: +class CallContext(abc.ABC): """Base for in-flight call contexts. - Subclasses (:class:`DispatchContext`, :class:`RequestContext`, - :class:`ExchangeContext`) are frozen dataclasses carrying additional + Subclasses (`DispatchContext`, `RequestContext`, + `ExchangeContext`) are frozen dataclasses carrying additional request / response data. Implements the context-manager protocol so - callers can ``with`` a context to ensure the :data:`ContextStore` entry is + callers can ``with`` a context to ensure the `ContextStore` entry is evicted on exit. - The shared :data:`ContextStore` is thread-safe; contexts for different + The shared `ContextStore` is thread-safe; contexts for different trace ids can be promoted concurrently without external synchronisation. """ instrumentation_context: InstrumentationContext # supplied by subclasses def close(self) -> None: - """Remove this context from :data:`ContextStore` (idempotent).""" + """Remove this context from `ContextStore` (idempotent).""" from .context_store import ContextStore ContextStore.remove(self.instrumentation_context.trace_id.value) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/context_store.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/context_store.py index 081630c..80a9e69 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/context_store.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/context_store.py @@ -14,12 +14,12 @@ class _ContextStore: """Process-wide registry mapping a call's trace id to its current - :class:`CallContext`. + `CallContext`. - Each promotion (:meth:`DispatchContext.to_request_context`, - :meth:`RequestContext.to_exchange_context`) overwrites the entry so the + Each promotion (`DispatchContext.to_request_context`, + `RequestContext.to_exchange_context`) overwrites the entry so the latest snapshot is visible to downstream observers keyed by trace id. - Entries are removed when callers honour :meth:`CallContext.close`. + Entries are removed when callers honour `CallContext.close`. Thread-safe — every operation acquires the lock so the guarantee survives free-threaded CPython (PEP 703) and non-CPython runtimes that @@ -65,7 +65,7 @@ def remove(self, trace_id: str) -> None: self._contexts.pop(trace_id, None) -#: Process-wide :class:`_ContextStore` singleton. +#: Process-wide `_ContextStore` singleton. ContextStore = _ContextStore() __all__ = ["ContextStore"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/dispatch_context.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/dispatch_context.py index ecf52ce..b805484 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/dispatch_context.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/dispatch_context.py @@ -20,19 +20,19 @@ class DispatchContext(CallContext): """First link in the context promotion chain. - Carries only the :class:`InstrumentationContext`. Once a :class:`Request` - has been built, :meth:`to_request_context` promotes this into a - :class:`RequestContext`. The promotion produces a new immutable instance - and re-registers it with :data:`ContextStore` under the same trace id so + Carries only the `InstrumentationContext`. Once a `Request` + has been built, `to_request_context` promotes this into a + `RequestContext`. The promotion produces a new immutable instance + and re-registers it with `ContextStore` under the same trace id so downstream observers see the latest snapshot. """ instrumentation_context: InstrumentationContext def to_request_context(self, request: Request) -> RequestContext: - """Promote into a :class:`RequestContext` bound to ``request``. + """Promote into a `RequestContext` bound to ``request``. - Stores the new context in :data:`ContextStore` keyed by trace id. + Stores the new context in `ContextStore` keyed by trace id. """ from .context_store import ContextStore from .request_context import RequestContext diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/request_context.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/request_context.py index a1b889e..cb9ceb5 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/request_context.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/context/request_context.py @@ -22,9 +22,9 @@ class RequestContext(CallContext): """Second link in the context promotion chain. - Adds the outgoing :class:`Request` to the call's - :class:`InstrumentationContext`. Once a :class:`Response` arrives, - :meth:`to_exchange_context` promotes this into an :class:`ExchangeContext`. + Adds the outgoing `Request` to the call's + `InstrumentationContext`. Once a `Response` arrives, + `to_exchange_context` promotes this into an `ExchangeContext`. """ instrumentation_context: InstrumentationContext @@ -34,9 +34,9 @@ def to_exchange_context( self, response: Response | AsyncResponse, ) -> ExchangeContext: - """Promote into an :class:`ExchangeContext` bound to ``response``. + """Promote into an `ExchangeContext` bound to ``response``. - Stores the new context in :data:`ContextStore` keyed by trace id. + Stores the new context in `ContextStore` keyed by trace id. """ from .context_store import ContextStore from .exchange_context import ExchangeContext diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/async_request_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/async_request_body.py index b654350..a9be257 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/async_request_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/async_request_body.py @@ -10,8 +10,10 @@ from typing import Protocol, runtime_checkable from urllib.parse import quote +from .._shielded import _shielded_cleanup from ..common import common_media_types from ..common.media_type import MediaType +from .request_body import _check_chunk_size @runtime_checkable @@ -34,10 +36,11 @@ class AsyncRequestBody(ABC): """Async twin of ``RequestBody``. Produces bytes via ``aiter_bytes``; ``write_to`` is the convenience - drainer for transports holding a ``SupportsAsyncWrite``. Replayability - semantics mirror the sync side: factories ``from_bytes`` / ``from_string`` - / ``from_form`` produce replayable bodies, while ``from_async_iter`` and - ``from_async_stream`` are single-use. + drainer for transports holding a ``SupportsAsyncWrite``. This async + surface ships a subset of the sync factories: ``from_bytes`` / + ``from_string`` / ``from_form`` produce replayable bodies, while + ``from_async_iter`` and ``from_async_stream`` are single-use. The sync + ``from_file`` / ``from_multipart`` factories have no async twin here. """ @abstractmethod @@ -102,7 +105,10 @@ def from_form( fields: Mapping[str, str], encoding: str = "utf-8", ) -> AsyncRequestBody: - encoded = "&".join(f"{quote(k, safe='')}={quote(v, safe='')}" for k, v in fields.items()) + encoded = "&".join( + f"{quote(k, safe='', encoding=encoding)}={quote(v, safe='', encoding=encoding)}" + for k, v in fields.items() + ) return _AsyncBytesBody( encoded.encode(encoding), common_media_types.APPLICATION_FORM_URLENCODED, @@ -149,6 +155,7 @@ async def to_replayable(self) -> AsyncRequestBody: return self async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + _check_chunk_size(chunk_size) view = memoryview(self._data) for start in range(0, len(view), chunk_size): yield bytes(view[start : start + chunk_size]) @@ -177,6 +184,7 @@ def content_length(self) -> int: return self._length async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + _check_chunk_size(chunk_size) del chunk_size if self._consumed: raise RuntimeError( @@ -189,7 +197,16 @@ async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes] class _AsyncStreamBody(AsyncRequestBody): - """Single-use body backed by an async-read stream.""" + """Single-use body backed by an async-read stream. + + Note: + Cancellation contract. ``aiter_bytes`` releases the underlying stream + from a ``finally`` clause. If the producing task is cancelled + mid-iteration that clause runs with a ``CancelledError`` already in + flight, so the close is routed through ``_shielded_cleanup`` to run to + completion before the cancellation continues to propagate — cleanup + never swallows it. + """ __slots__ = ("_consumed", "_length", "_media_type", "_stream") @@ -211,6 +228,7 @@ def content_length(self) -> int: return self._length async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + _check_chunk_size(chunk_size) if self._consumed: raise RuntimeError( "AsyncRequestBody.aiter_bytes was already called — the stream is exhausted." @@ -223,7 +241,7 @@ async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes] return yield chunk finally: - await self._stream.close() + await _shielded_cleanup(self._stream.close()) __all__ = ["AsyncRequestBody", "SupportsAsyncRead", "SupportsAsyncWrite"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/file_request_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/file_request_body.py index 9bf2a2c..d29b2a6 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/file_request_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/file_request_body.py @@ -80,16 +80,19 @@ def media_type(self) -> MediaType | None: return self._media_type def content_length(self) -> int: - if self._count != -1: - return self._count # Stat lazily — the file may grow between body construction and send; # whatever stat returns at call time is the best estimate we have. try: size = self._path.stat().st_size except OSError: - return -1 - remaining = size - self._offset - return max(0, remaining) + # No stat available: fall back to the requested count, or unknown. + return self._count if self._count != -1 else -1 + available = max(0, size - self._offset) + if self._count == -1: + return available + # ``iter_bytes`` stops at EOF, so a count past EOF would over-report; + # advertise only what can actually be read. + return min(self._count, available) def is_replayable(self) -> bool: return True diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/method.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/method.py index 8168f3c..f799f31 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/method.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/method.py @@ -11,7 +11,7 @@ class Method(StrEnum): """HTTP request methods recognized by the SDK. - :class:`enum.StrEnum` (3.11+) gives string-valued members whose ``str()`` + `enum.StrEnum` (3.11+) gives string-valued members whose ``str()`` *is* the wire form, so callers can interpolate or compare against bare strings: ``Method.GET == "GET"``. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request.py index 955d042..58d392b 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request.py @@ -24,11 +24,11 @@ class Request: """Immutable HTTP request handed to a transport. Construct directly via the dataclass constructor or derive a new instance - non-destructively via :func:`dataclasses.replace` or the ``with_*`` helpers. + non-destructively via `dataclasses.replace` or the ``with_*`` helpers. Header / metadata surface is immutable and safe to share across threads; the ``body``, when present, carries single-use stream state — clone before - sharing if you retain a stream-backed body. See :class:`RequestBody`. + sharing if you retain a stream-backed body. See `RequestBody`. """ method: Method diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request_body.py index 1ffb62a..adee0ab 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/request_body.py @@ -63,14 +63,19 @@ def content_length(self) -> int: @abstractmethod def iter_bytes(self, chunk_size: int = 64 * 1024) -> Iterator[bytes]: - """Yield the body's bytes in chunks of at most ``chunk_size`` bytes. + """Yield the body's bytes in chunks. Called once per send attempt; replayable bodies may have this method invoked multiple times across retries (a fresh iterator is returned). + ``chunk_size`` is a hint, not a hard cap: bytes- and file-backed + bodies honour it as a suggested upper bound, but the iterable-backed + body (``from_iter``) passes its source chunks through unchanged and so + may yield chunks larger than ``chunk_size``. + Args: chunk_size: Suggested chunk size. Implementations may yield smaller - chunks at end-of-stream. + chunks at end-of-stream, and ``from_iter`` ignores it entirely. Yields: Successive ``bytes`` chunks until the body is exhausted. @@ -169,7 +174,10 @@ def from_form( Returns: A replayable form body carrying the standard media type. """ - encoded = "&".join(f"{quote(k, safe='')}={quote(v, safe='')}" for k, v in fields.items()) + encoded = "&".join( + f"{quote(k, safe='', encoding=encoding)}={quote(v, safe='', encoding=encoding)}" + for k, v in fields.items() + ) return _BytesBody( encoded.encode(encoding), common_media_types.APPLICATION_FORM_URLENCODED, diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response.py index a7b6df7..4b6ffec 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response.py @@ -9,10 +9,10 @@ from types import TracebackType from typing import TYPE_CHECKING, Self +from .._shielded import _shielded_cleanup from ..common.headers import Headers from ..common.http_header_name import HttpHeaderName from ..common.protocol import Protocol -from .async_response_body import _shielded_cleanup from .status import Status if TYPE_CHECKING: diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response_body.py index 34fb4db..a850949 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/async_response_body.py @@ -5,71 +5,19 @@ from __future__ import annotations -import asyncio from abc import ABC, abstractmethod -from collections.abc import AsyncIterator, Awaitable +from collections.abc import AsyncIterator from types import TracebackType -from typing import Self +from typing import TYPE_CHECKING, Self +from .._shielded import _shielded_cleanup as _shielded_cleanup from ..common.media_type import MediaType -from ..request.async_request_body import SupportsAsyncRead from .response_body import _check_chunk_size -_bytes = bytes - +if TYPE_CHECKING: + from ..request.async_request_body import SupportsAsyncRead -async def _shielded_cleanup(cleanup: Awaitable[object]) -> None: - """Run a cleanup coroutine without letting cancellation interrupt it. - - This is the single cancellation convention used by the async response - bodies: a ``finally`` block that releases transport resources may run - while an ``asyncio.CancelledError`` is already propagating through the - enclosing task. Awaiting the cleanup directly would let that - cancellation interrupt it mid-way, leaking the underlying connection. - - The cleanup is wrapped in ``asyncio.shield`` so it always runs to - completion. If the surrounding scope is cancelled, the ``CancelledError`` - raised by ``shield`` is caught and the wait retried until the shielded - cleanup finishes; the cancellation is then re-raised so it continues to - propagate. Cleanup never swallows cancellation — it merely defers it - until the resource is released. A ``CancelledError`` raised because the - cleanup *itself* was cancelled is propagated immediately. - - A pending outer cancellation always wins: if the cleanup runs to - completion but raises an ordinary exception while a cancellation is - waiting, the cancellation is re-raised (the cleanup error does not mask - it). When no cancellation is pending, a cleanup failure surfaces to the - caller unchanged. - - Args: - cleanup: The resource-release coroutine to run to completion. - - Raises: - asyncio.CancelledError: Re-raised after the cleanup completes when - the enclosing scope was cancelled while the cleanup ran. - Exception: Whatever the cleanup coroutine raised, when no outer - cancellation is pending. - """ - inner = asyncio.ensure_future(cleanup) - cancelled = False - while not inner.done(): - try: - await asyncio.shield(inner) - except asyncio.CancelledError: - if inner.cancelled(): - # The cleanup itself was cancelled, not just our wait on it. - raise - # An outer cancellation hit our wait, not the shielded cleanup. - # Keep waiting until the cleanup finishes, then re-raise so the - # cancellation continues to propagate. - cancelled = True - except Exception: - # The cleanup failed; ``inner`` retains the exception, surfaced - # below. A pending cancellation still takes precedence. - break - if cancelled: - raise asyncio.CancelledError - inner.result() +_bytes = bytes class AsyncResponseBody(ABC): diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/loggable_response_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/loggable_response_body.py index e259d7c..6916408 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/loggable_response_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/loggable_response_body.py @@ -129,8 +129,14 @@ def _drain(self) -> None: take = min(self._max - captured, len(chunk)) chunks.append(chunk[:take]) captured += take - except Exception as exc: + except BaseException as exc: + # Record any failure (including bare ``BaseException``) so + # ``iter_bytes`` re-raises it and never replays a truncated + # read as success. ``KeyboardInterrupt`` / ``SystemExit`` must + # not be swallowed: propagate them immediately. self._error = exc + if isinstance(exc, (KeyboardInterrupt, SystemExit)): + raise finally: self._cached = b"".join(chunks) self._drained = True diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response.py index bcbf57a..506c13d 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response.py @@ -34,7 +34,7 @@ class Response: and the underlying body is closed deterministically on exit. The header / metadata surface is immutable and safe to share across threads; the body, - when present, carries single-use stream state — see :class:`ResponseBody`. + when present, carries single-use stream state — see `ResponseBody`. """ request: Request diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response_body.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response_body.py index fbf0839..888ddd0 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response_body.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/response_body.py @@ -7,7 +7,6 @@ from abc import ABC, abstractmethod from collections.abc import Iterator -from io import BytesIO from types import TracebackType from typing import BinaryIO, Self @@ -228,5 +227,4 @@ def close(self) -> None: self._stream.close() -# Re-exported from ``io`` for adapter convenience. -__all__ = ["BytesIO", "ResponseBody"] +__all__ = ["ResponseBody"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/status.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/status.py index e88f220..f8d8854 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/status.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/response/status.py @@ -11,7 +11,7 @@ class Status(IntEnum): """HTTP status codes recognized by the SDK. - Inheriting from :class:`int` so callers can compare against integers and + Inheriting from `int` so callers can compare against integers and range-check directly: ``response.status == 200`` or ``200 <= status < 300``. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/connection.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/connection.py index ecf41d7..0ac5484 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/connection.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/connection.py @@ -251,7 +251,7 @@ def _stream( class AsyncSseConnection: - """Asynchronous twin of :class:`SseConnection`. + """Asynchronous twin of `SseConnection`. Mirrors the sync client exactly with ``async`` iteration semantics. Each (re)connection is dispatched through ``source`` — an ``AsyncPipeline`` (run diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/parser.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/parser.py index 6692150..8ee9aff 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/parser.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/sse/parser.py @@ -111,9 +111,9 @@ class SseEvent: class SseParser: """Stateful WHATWG SSE parser. - Feed bytes via :meth:`feed`; finished events arrive on the internal - queue and are yielded by :meth:`drain`. Callers typically use the - free functions :func:`parse_events` / :func:`parse_async_events` rather + Feed bytes via `feed`; finished events arrive on the internal + queue and are yielded by `drain`. Callers typically use the + free functions `parse_events` / `parse_async_events` rather than holding a parser directly. """ @@ -150,7 +150,7 @@ def feed(self, chunk: bytes) -> None: if not self._strip_leading_bom(): return # Buffer too short to decide on the BOM yet. while True: - line, consumed = _read_line(self._buffer) + line, consumed = _read_line(self._buffer, at_eos=False) if line is None: if len(self._buffer) > self.max_line_bytes: raise StreamingError(f"SSE line exceeded {self.max_line_bytes} bytes") @@ -173,11 +173,12 @@ def end(self) -> Iterator[SseEvent]: self._strip_leading_bom(at_end=True) if self._buffer: try: - line = self._buffer.decode("utf-8") + line, _ = _read_line(self._buffer, at_eos=True) except UnicodeDecodeError as err: raise StreamingError("Stream ended mid-codepoint") from err self._buffer.clear() - self._process_line(line) + if line is not None: + self._process_line(line) if self._data_lines: self._dispatch() yield from self.drain() @@ -252,22 +253,48 @@ def _dispatch(self) -> None: self._event = "message" -def _read_line(buffer: bytearray) -> tuple[str | None, int]: +def _read_line(buffer: bytearray, *, at_eos: bool) -> tuple[str | None, int]: """Find the next complete line in ``buffer``. - Returns ``(line, consumed)`` where ``line`` is the decoded text without - its terminator and ``consumed`` is the number of bytes (including the + Returns ``(line, consumed)`` where ``line`` is the decoded text without its + terminator and ``consumed`` is the number of bytes (including the terminator) to drop from the buffer. ``(None, 0)`` indicates the buffer - does not yet contain a complete line. + does not yet contain a complete line and the caller should wait for more + input. + + A bare ``CR`` may be the first byte of a ``CRLF`` pair whose ``LF`` has not + arrived yet. When a ``CR`` is the final buffered byte and the stream is + still open (``at_eos`` is false), the line is withheld — ``(None, 0)`` is + returned so the ``CR`` is held until the next byte disambiguates a lone + ``CR`` terminator from a split ``CRLF``. At end-of-stream a trailing ``CR`` + is a complete terminator and the line is returned. + + Args: + buffer: The accumulated, not-yet-consumed bytes. + at_eos: ``True`` when no further input will arrive, so a trailing + ``CR`` and any unterminated residue both resolve to a final line. + + Returns: + ``(line, consumed)`` for a complete line, or ``(None, 0)`` when more + input is needed to decide. At end-of-stream an unterminated residue is + returned as the final line consuming the whole buffer. """ for index, byte in enumerate(buffer): if byte == _LF: return buffer[:index].decode("utf-8"), index + 1 if byte == _CR: - # CR or CRLF — peek at the next byte if present. - if index + 1 < len(buffer) and buffer[index + 1] == _LF: - return buffer[:index].decode("utf-8"), index + 2 + if index + 1 < len(buffer): + # CRLF when the next byte is LF, otherwise a lone-CR terminator. + consumed = index + 2 if buffer[index + 1] == _LF else index + 1 + return buffer[:index].decode("utf-8"), consumed + # CR is the final byte: hold it open until the next byte arrives so + # a split CRLF is not mistaken for two terminators. At EOS it is a + # complete terminator. + if not at_eos: + return None, 0 return buffer[:index].decode("utf-8"), index + 1 + if at_eos and buffer: + return buffer.decode("utf-8"), len(buffer) return None, 0 @@ -290,13 +317,13 @@ def parse_events(chunks: Iterable[bytes]) -> Iterator[SseEvent]: class AsyncSseStream: """Async iterator that drives an ``SseParser`` from an async byte stream. - Construct via :func:`parse_async_events` or directly. Use as + Construct via `parse_async_events` or directly. Use as ``async for event in stream``, ideally inside ``async with`` so the upstream byte stream is released deterministically. Note: Cancellation contract. If the consuming task is cancelled - mid-stream, :meth:`aclose` (run from ``__aexit__`` or directly) + mid-stream, `aclose` (run from ``__aexit__`` or directly) releases the upstream byte iterator. The upstream ``aclose`` is routed through ``_shielded_aclose`` so it runs to completion even while a ``CancelledError`` is in flight; the cancellation is then @@ -334,7 +361,7 @@ async def __anext__(self) -> SseEvent: @property def retry(self) -> int | None: """The most recent ``retry:`` value seen by the underlying parser, in - milliseconds, or ``None``. Mirrors :attr:`SseParser.retry` so a + milliseconds, or ``None``. Mirrors `SseParser.retry` so a reconnecting client can read the server's sticky hint after iteration. """ return self._parser.retry diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/__init__.py index 3711214..751a149 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/__init__.py @@ -6,7 +6,7 @@ Stdlib-only HMAC-SHA256 verification of inbound webhooks: rebuild the signed content ``{id}.{timestamp}.{body}``, compare constant-time against any of the provided ``v1,`` signatures, and reject deliveries outside a ±5-minute -timestamp window. :class:`WebhookVerifier.unwrap` additionally parses the +timestamp window. `WebhookVerifier.unwrap` additionally parses the verified JSON body. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/verification.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/verification.py index f866e3b..4514686 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/verification.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/http/webhooks/verification.py @@ -25,7 +25,7 @@ Verification is constant-time (``hmac.compare_digest``) and rejects timestamps that are too old or too far in the future relative to an injected -:class:`~dexpace.sdk.core.util.Clock`, defaulting to the process clock. +`Clock`, defaulting to the process clock. Example: >>> verifier = WebhookVerifier("whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw") @@ -107,13 +107,17 @@ def _decode_secret(secret: str) -> bytes: The raw key bytes. Raises: - InvalidWebhookSignatureError: If the base64 body is malformed. + InvalidWebhookSignatureError: If the base64 body is malformed or decodes + to an empty key (e.g. ``""`` or a bare ``whsec_`` prefix). """ body = secret[len(_SECRET_PREFIX) :] if secret.startswith(_SECRET_PREFIX) else secret try: - return base64.b64decode(body, validate=True) + key = base64.b64decode(body, validate=True) except (binascii.Error, ValueError) as exc: raise InvalidWebhookSignatureError("malformed webhook secret") from exc + if not key: + raise InvalidWebhookSignatureError("empty webhook secret") + return key def _as_bytes(body: str | bytes) -> bytes: @@ -132,7 +136,7 @@ class WebhookVerifier: prefix is optional). tolerance_seconds: Maximum absolute difference, in seconds, allowed between the signed timestamp and the current time. Defaults to - :data:`DEFAULT_TOLERANCE_SECONDS` (5 minutes). + `DEFAULT_TOLERANCE_SECONDS` (5 minutes). clock: Time source used to evaluate the tolerance window. Defaults to the process clock; inject a fake to test the replay window. @@ -189,7 +193,7 @@ def verify(self, headers: Mapping[str, str], body: str | bytes) -> None: def unwrap(self, headers: Mapping[str, str], body: str | bytes) -> object: """Verify a webhook and return its parsed JSON payload. - A convenience over :meth:`verify` for the common case of a JSON body: + A convenience over `verify` for the common case of a JSON body: the signature is checked against the *raw* bytes first, then the same bytes are parsed. Verifying before parsing guarantees only authentic payloads are ever deserialized. @@ -217,9 +221,14 @@ def _check_timestamp(self, timestamp: str) -> None: """Reject a timestamp that is malformed or outside the tolerance window. Raises: - InvalidWebhookSignatureError: If ``timestamp`` is not an integer or - differs from now by more than the configured tolerance. + InvalidWebhookSignatureError: If ``timestamp`` is not a canonical + non-negative ASCII integer (``int()`` would otherwise accept a + leading sign, surrounding whitespace, PEP 515 underscores, and + non-ASCII digits), or differs from now by more than the + configured tolerance. """ + if not (timestamp.isascii() and timestamp.isdigit()): + raise InvalidWebhookSignatureError("malformed webhook-timestamp") try: signed_at = int(timestamp) except ValueError as exc: @@ -241,8 +250,10 @@ def _matches_any(self, signature_header: str, expected: str) -> bool: Tokens are space-separated. Unknown-version and malformed tokens are skipped rather than raising, so a forward-compatible producer that adds - a future scheme version alongside ``v1`` still verifies. Comparison is - constant-time to avoid leaking the expected signature via timing. + a future scheme version alongside ``v1`` still verifies. A token whose + candidate is not ASCII cannot match a base64 signature, so it is skipped + too rather than letting the encode raise out of verification. Comparison + is constant-time to avoid leaking the expected signature via timing. """ expected_bytes = expected.encode("ascii") matched = False @@ -250,6 +261,10 @@ def _matches_any(self, signature_header: str, expected: str) -> bool: version, _, candidate = token.partition(",") if version != _SIGNATURE_VERSION or not candidate: continue - if hmac.compare_digest(candidate.encode("ascii"), expected_bytes): + try: + candidate_bytes = candidate.encode("ascii") + except UnicodeEncodeError: + continue + if hmac.compare_digest(candidate_bytes, expected_bytes): matched = True return matched diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/client_logger.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/client_logger.py index eee0307..d510a27 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/client_logger.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/client_logger.py @@ -22,7 +22,7 @@ class CorrelationFilter(logging.Filter): """Stamps the active trace/span ids onto every record it sees. - Reads the context-local ids from :mod:`correlation` and attaches them as + Reads the context-local ids from `correlation` and attaches them as ``trace.id`` / ``span.id`` record attributes (plus the dotted-name-safe ``trace_id`` / ``span_id`` aliases for ``%``-style format strings). When no trace is bound the attributes are set to ``None`` so formatters referencing @@ -97,7 +97,7 @@ def verbose(self, message: str, **fields: Any) -> None: def _install_correlation_filter(logger: logging.Logger) -> None: - """Attach a :class:`CorrelationFilter` to ``logger`` exactly once.""" + """Attach a `CorrelationFilter` to ``logger`` exactly once.""" if any(isinstance(existing, CorrelationFilter) for existing in logger.filters): return logger.addFilter(CorrelationFilter()) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/correlation.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/correlation.py index d7839f0..05215ab 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/correlation.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/correlation.py @@ -11,7 +11,7 @@ record. Code that hops to a worker thread (``loop.run_in_executor``) does not inherit -the caller's context automatically; use :func:`bind_correlation` there to +the caller's context automatically; use `bind_correlation` there to re-establish the ids inside the worker. """ @@ -47,7 +47,7 @@ def set_trace_id(value: str | None) -> Token[str | None]: Returns: A reset token; pass it to ``ContextVar.reset`` to restore the prior - value. Prefer :func:`bind_correlation` for scoped use. + value. Prefer `bind_correlation` for scoped use. """ return _trace_id.set(value) @@ -60,7 +60,7 @@ def set_span_id(value: str | None) -> Token[str | None]: Returns: A reset token; pass it to ``ContextVar.reset`` to restore the prior - value. Prefer :func:`bind_correlation` for scoped use. + value. Prefer `bind_correlation` for scoped use. """ return _span_id.set(value) @@ -73,12 +73,16 @@ def bind_correlation( ) -> Iterator[None]: """Bind trace/span ids for the duration of the ``with`` block. - Restores the previous ids on exit, even if the body raises. Only the - arguments that are passed are bound; omitted ids are left untouched. + Restores the previous ids on exit, even if the body raises. Both ids are + always (re)bound for the block: an omitted (or explicitly ``None``) + argument clears that id rather than leaving the inherited value in place. + Pass the current value through if you only mean to override the other id. Args: - trace_id: Trace id to bind for the block, or ``None`` to clear it. - span_id: Span id to bind for the block, or ``None`` to clear it. + trace_id: Trace id to bind for the block. Defaults to ``None``, which + clears any inherited trace id for the block. + span_id: Span id to bind for the block. Defaults to ``None``, which + clears any inherited span id for the block. Yields: Nothing; use as a plain scope guard. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/http_tracer.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/http_tracer.py index 114b213..09a574f 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/http_tracer.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/http_tracer.py @@ -11,7 +11,7 @@ operation so per-call state (attempt counters, timers) stays isolated. Modeled on Google gax's ``ApiTracer``. The SDK ships the contract plus a no-op -default (:class:`NoopHttpTracer` / :data:`NOOP_HTTP_TRACER_FACTORY`); consumers +default (`NoopHttpTracer` / `NOOP_HTTP_TRACER_FACTORY`); consumers plug in a real implementation per their metrics/tracing stack. """ @@ -34,7 +34,10 @@ class HttpTracer(ABC): Implementations are notified from whatever thread or task drives the request and should keep callbacks cheap and non-blocking; they must not - raise. + raise. The SDK does not guard these callbacks: a raising tracer propagates + out of the notifying policy and can mask the original error (for example, + inside ``TracingPolicy._dispatch``), so the no-raise rule is a hard + requirement on the implementation, not something the SDK enforces. """ def operation_started(self) -> None: @@ -108,7 +111,7 @@ def connection_acquired(self, host: str, port: int) -> None: @runtime_checkable class HttpTracerFactory(Protocol): - """Mints a fresh :class:`HttpTracer` per logical operation. + """Mints a fresh `HttpTracer` per logical operation. Policies create one tracer at the start of each operation so per-call state (attempt counters, timers) never leaks across operations. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/identifiers.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/identifiers.py index dc63c93..0d55159 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/identifiers.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/identifiers.py @@ -14,7 +14,7 @@ class TraceId: """Identifier shared by every span in the same logical trace. - The encoding (hex / decimal, length) depends on the :class:`TraceIdType` of + The encoding (hex / decimal, length) depends on the `TraceIdType` of the originating context. """ @@ -66,7 +66,7 @@ class TraceState: class TraceIdType(StrEnum): - """Encoding flavour of a :class:`TraceId`. + """Encoding flavour of a `TraceId`. Different backends expect different wire formats (W3C hex vs Datadog decimal); the type drives how the trace id is rendered for propagation. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/instrumentation_context.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/instrumentation_context.py index ebdccaa..9dd53df 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/instrumentation_context.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/instrumentation_context.py @@ -35,10 +35,10 @@ class InstrumentationContext: context across service boundaries so spans on either side correlate into one logical trace. - The shared no-op singleton :data:`NOOP_INSTRUMENTATION_CONTEXT` is used + The shared no-op singleton `NOOP_INSTRUMENTATION_CONTEXT` is used when tracing is disabled. - ``http_tracer_factory`` mints a per-operation :class:`HttpTracer` for + ``http_tracer_factory`` mints a per-operation `HttpTracer` for fine-grained request telemetry; it defaults to the no-op factory so callers that don't instrument pay nothing. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/noop.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/noop.py index 61eb6c6..3853c12 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/noop.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/noop.py @@ -16,7 +16,7 @@ class _NoopScope(TracingScope): - """Reused across every :meth:`Span.make_current` call so the no-op path + """Reused across every `Span.make_current` call so the no-op path allocates nothing.""" def close(self) -> None: @@ -27,16 +27,16 @@ def close(self) -> None: class _NoopHttpTracer(HttpTracer): - """No-op :class:`HttpTracer` — every event callback inherits the do-nothing - default. Use the shared :data:`NOOP_HTTP_TRACER` singleton.""" + """No-op `HttpTracer` — every event callback inherits the do-nothing + default. Use the shared `NOOP_HTTP_TRACER` singleton.""" -#: Shared no-op :class:`HttpTracer` singleton. Use when tracing is disabled. +#: Shared no-op `HttpTracer` singleton. Use when tracing is disabled. NOOP_HTTP_TRACER: Final[HttpTracer] = _NoopHttpTracer() class _NoopHttpTracerFactory: - """No-op tracer factory — every :meth:`create` returns :data:`NOOP_HTTP_TRACER`.""" + """No-op tracer factory — every `create` returns `NOOP_HTTP_TRACER`.""" def create(self) -> HttpTracer: return NOOP_HTTP_TRACER @@ -47,9 +47,9 @@ def create(self) -> HttpTracer: class _NoopSpan(Span): - """No-op :class:`Span` — records nothing, returns ``self`` from every mutator. + """No-op `Span` — records nothing, returns ``self`` from every mutator. - Use the shared :data:`NOOP_SPAN` singleton; this class is not part of the + Use the shared `NOOP_SPAN` singleton; this class is not part of the public API. """ @@ -74,11 +74,11 @@ def end(self, error: BaseException | None = None) -> None: return None -#: Shared no-op :class:`Span` singleton. Use when tracing is disabled. +#: Shared no-op `Span` singleton. Use when tracing is disabled. NOOP_SPAN: Final[Span] = _NoopSpan() -#: Shared no-op :class:`InstrumentationContext` singleton. +#: Shared no-op `InstrumentationContext` singleton. NOOP_INSTRUMENTATION_CONTEXT: Final[InstrumentationContext] = InstrumentationContext( trace_id_type=TraceIdType.NOOP, trace_id=TraceId.NOOP, @@ -92,7 +92,7 @@ def end(self, error: BaseException | None = None) -> None: class _NoopTracer(Tracer): - """No-op :class:`Tracer` — every :meth:`start_span` returns :data:`NOOP_SPAN`.""" + """No-op `Tracer` — every `start_span` returns `NOOP_SPAN`.""" def start_span( self, @@ -102,7 +102,7 @@ def start_span( return NOOP_SPAN -#: Shared no-op :class:`Tracer` singleton. +#: Shared no-op `Tracer` singleton. NOOP_TRACER: Final[Tracer] = _NoopTracer() diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/span.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/span.py index e13f10d..c432555 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/span.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/span.py @@ -20,9 +20,9 @@ class Span(ABC): across services through their shared trace context and may nest to model parent/child relationships. - Most callers obtain a span from a :class:`Tracer` and end it once the + Most callers obtain a span from a `Tracer` and end it once the underlying operation completes. Operations that don't perform tracing - receive the no-op :data:`NOOP_SPAN` singleton. + receive the no-op `NOOP_SPAN` singleton. """ @property @@ -47,7 +47,7 @@ def set_error(self, error_type: str) -> Self: def make_current(self) -> TracingScope: """Make this span the active span for the current execution context. - Returns a :class:`TracingScope` whose ``close()`` restores the + Returns a `TracingScope` whose ``close()`` restores the previously active span. Use as a context manager to guarantee cleanup. """ @@ -55,7 +55,7 @@ def make_current(self) -> TracingScope: def end(self, error: BaseException | None = None) -> None: """Finish the span. - Pass ``error`` to record an exception as the cause. Calling :meth:`end` + Pass ``error`` to record an exception as the cause. Calling `end` more than once, or on a non-recording span, is a documented no-op. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracer.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracer.py index cce492c..3b76c95 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracer.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracer.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Factory for :class:`Span` instances.""" +"""Factory for `Span` instances.""" from __future__ import annotations diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracing_scope.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracing_scope.py index 472160c..8c3d561 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracing_scope.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/tracing_scope.py @@ -7,10 +7,11 @@ from abc import ABC, abstractmethod from types import TracebackType +from typing import Self class TracingScope(ABC): - """Lifecycle handle for a span activated via :meth:`Span.make_current`. + """Lifecycle handle for a span activated via `Span.make_current`. While the scope is open the associated span is the "current" span for the executing thread; closing the scope restores the previously active span. @@ -22,7 +23,7 @@ class TracingScope(ABC): def close(self) -> None: """Restore the previously active span. Idempotent.""" - def __enter__(self) -> TracingScope: + def __enter__(self) -> Self: return self def __exit__( diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/url_redactor.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/url_redactor.py index 9ec0807..23b492c 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/url_redactor.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/instrumentation/url_redactor.py @@ -80,8 +80,9 @@ def redact(self, url: str | Url) -> str: drops a URL because it's malformed). Returns: - A wire-form URL with userinfo stripped and non-allowlisted - query values replaced by ``REDACTED``. + A wire-form URL with userinfo stripped and each non-allowlisted + parameter collapsed to ``REDACTED=REDACTED`` (both key and value), + so neither the secret nor the parameter name leaks. """ parsed = url if isinstance(url, Url) else _safe_parse(url) if parsed is None: @@ -91,7 +92,7 @@ def redact(self, url: str | Url) -> str: def _redact_parsed(self, parsed: Url) -> Url: redacted_query = QueryParams( [ - (key, value if key in self.allowed_query_keys else _REDACTED) + (key, value) if key in self.allowed_query_keys else (_REDACTED, _REDACTED) for key, values in parsed.query.items() for value in values ] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/__init__.py index c42310c..fd49cd7 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/__init__.py @@ -5,18 +5,18 @@ The public surface is a small set of pieces that compose: -* :class:`Page` — a frozen page of items plus the request that reaches the +* `Page` — a frozen page of items plus the request that reaches the next page (and, when supported, the previous one). It is a context manager so the underlying response closes deterministically. -* :class:`PaginationStrategy` — the SPI that turns one decoded response into a - :class:`Page`. Built-ins: :class:`CursorStrategy` (cursor / token), - :class:`PageNumberStrategy` (page index), and :class:`LinkHeaderStrategy` +* `PaginationStrategy` — the SPI that turns one decoded response into a + `Page`. Built-ins: `CursorStrategy` (cursor / token), + `PageNumberStrategy` (page index), and `LinkHeaderStrategy` (RFC 5988 ``Link`` header). -* :class:`Paginator` / :class:`AsyncPaginator` — iterate the sequence +* `Paginator` / `AsyncPaginator` — iterate the sequence item-by-item by default, or page-by-page via ``by_page``. Each page fetch runs through the full pipeline, so retry, auth, redirect, and tracing apply to every page. -* :func:`parse_link_header` / :func:`find_rel` — the standalone RFC 5988 +* `parse_link_header` / `find_rel` — the standalone RFC 5988 parser the link strategy is built on. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/paginator.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/paginator.py index 442a744..1ac7c1c 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/paginator.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/paginator.py @@ -14,7 +14,7 @@ for item in Paginator(pipeline, strategy, first_request): ... -Use :meth:`Paginator.by_page` when the raw response or page boundaries matter:: +Use `Paginator.by_page` when the raw response or page boundaries matter:: for page in Paginator(pipeline, strategy, first_request).by_page(): with page: @@ -60,8 +60,8 @@ class Paginator[T]: Args: source: Either a ``Pipeline`` (run once per page with a fresh dispatch context) or a send-callable ``Request -> Response``. - strategy: The :class:`PaginationStrategy` that parses each response - into a :class:`Page`. + strategy: The `PaginationStrategy` that parses each response + into a `Page`. initial_request: The request that fetches the first page. max_pages: Optional cap on the number of pages fetched. ``None`` means unbounded (drive it with care against open-ended APIs). @@ -97,7 +97,7 @@ def send(request: Request) -> Response: return source def by_page(self) -> Iterator[Page[T]]: - """Yield each :class:`Page` in turn, honouring ``max_pages``. + """Yield each `Page` in turn, honouring ``max_pages``. Yields: Pages from first to last. Each page owns its response; iterate @@ -126,7 +126,7 @@ def __iter__(self) -> Iterator[T]: class AsyncPaginator[T]: - """Asynchronous twin of :class:`Paginator`. + """Asynchronous twin of `Paginator`. Mirrors the sync paginator exactly with ``async`` iteration semantics. ``source`` is an ``AsyncPipeline`` or an async send-callable. @@ -160,7 +160,7 @@ async def send(request: Request) -> AsyncResponse: return source async def by_page(self) -> AsyncIterator[Page[T]]: - """Async-yield each :class:`Page` in turn, honouring ``max_pages``.""" + """Async-yield each `Page` in turn, honouring ``max_pages``.""" request: Request | None = self._initial count = 0 while request is not None: diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/strategy.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/strategy.py index ece11e7..5aa09e7 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/strategy.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pagination/strategy.py @@ -47,7 +47,7 @@ def headers(self) -> Headers: ... @runtime_checkable class PaginationStrategy[T](Protocol): - """Translates one decoded response into a :class:`Page`. + """Translates one decoded response into a `Page`. Implementations are pure functions of their inputs — they perform no I/O and hold no mutable per-iteration state, so a single strategy instance is diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/__init__.py index 3ce58e6..13e4088 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/__init__.py @@ -15,7 +15,7 @@ from .stage import Stage from .staged_builder import StagedPipelineBuilder from .step import PipelineStep, RequestPipelineStep, ResponsePipelineStep -from .step.config import RetryConfig, StepMetadata +from .step.config import StepMetadata __all__ = [ "AsyncPipeline", @@ -27,7 +27,6 @@ "Policy", "RequestPipelineStep", "ResponsePipelineStep", - "RetryConfig", "Stage", "StagedPipelineBuilder", "StepMetadata", diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/_async_sansio_runner.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/_async_sansio_runner.py index 4b0d64d..64fd3e7 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/_async_sansio_runner.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/_async_sansio_runner.py @@ -4,15 +4,15 @@ """Async SansIO runners — mirror the sync ``_SansIO*Runner`` shapes. Async SansIO steps may be plain callables (``(value, ctx) -> value``) or -async callables (``async def``); both forms are supported. The runner -detects coroutine results and awaits them. This is Azure's -``inspect.iscoroutinefunction`` pattern, adapted to use ``asyncio.iscoroutine`` -on the returned object so wrapped/bound callables still work. +async callables (``async def``); both forms are supported. Detection keys off +the *returned value* rather than the callable: ``_resolve`` awaits anything +that is an ``Awaitable`` (coroutines, futures, and custom awaitables alike) +and passes plain values through unchanged. Inspecting the result rather than +the callable means wrapped, bound, or ``functools.partial`` steps work too. """ from __future__ import annotations -import asyncio from collections.abc import Awaitable, Callable from typing import TYPE_CHECKING, Any @@ -27,7 +27,7 @@ async def _resolve(value: Any) -> Any: - if asyncio.iscoroutine(value) or isinstance(value, Awaitable): + if isinstance(value, Awaitable): return await value return value diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_pipeline.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_pipeline.py index d6d940e..40b8d2a 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_pipeline.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_pipeline.py @@ -95,7 +95,14 @@ async def run( ) -> AsyncResponse: request_ctx = dispatch.to_request_context(request) ctx = PipelineContext(call=request_ctx, options=dict(options)) - return await self._chain.send(request, ctx) + try: + return await self._chain.send(request, ctx) + finally: + # Evict the call's ``ContextStore`` entry once the chain has fully + # unwound — in-chain observers have already read the latest tier. + # The exchange context shares this trace id, so a single close + # clears both tiers and prevents unbounded growth across calls. + request_ctx.close() def _wrap_step(step: Any) -> AsyncPolicy: diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_staged_builder.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_staged_builder.py index 0e5af2a..34be840 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_staged_builder.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/async_staged_builder.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Async twin of :class:`StagedPipelineBuilder`. +"""Async twin of `StagedPipelineBuilder`. Behaviour mirrors the sync builder exactly: pillar enforcement, surgical edits, ``from_pipeline`` round-trip. The only differences are the policy @@ -22,9 +22,9 @@ class AsyncStagedPipelineBuilder: - """Build an :class:`AsyncPipeline` by stage rather than user-specified order. + """Build an `AsyncPipeline` by stage rather than user-specified order. - See :class:`StagedPipelineBuilder` for the full behaviour; this is the + See `StagedPipelineBuilder` for the full behaviour; this is the async counterpart. """ @@ -89,18 +89,32 @@ def remove(self, target: type[AsyncPolicy]) -> Self: return self def build(self) -> AsyncPipeline: - """Flatten the builder's contents into an :class:`AsyncPipeline`.""" + """Flatten the builder's contents into an `AsyncPipeline`.""" return AsyncPipeline(self._client, policies=self._flatten()) @classmethod def from_pipeline(cls, pipeline: AsyncPipeline) -> Self: - """Seed a builder from an existing :class:`AsyncPipeline`.""" + """Seed a builder from an existing `AsyncPipeline`. + + Raises: + ValueError: If the input pipeline's policies do not satisfy + stage ordering, or if the chain contains a list-constructor + SansIO step (a bare callable), which carries no ``STAGE`` + and so cannot be rehydrated. + """ from ._async_transport_runner import _AsyncTransportRunner builder = cls(pipeline.transport) chain: list[AsyncPolicy] = [] node: AsyncPolicy | None = pipeline._chain while node is not None and not isinstance(node, _AsyncTransportRunner): + if getattr(node, "STAGE", None) is None: + raise ValueError( + f"Pipeline node {type(node).__name__} carries no STAGE; " + f"it is a list-constructor SansIO step (a bare callable) " + f"that cannot be rehydrated into a staged builder. Rebuild " + f"the pipeline via the list constructor instead." + ) chain.append(node) node = getattr(node, "next", None) last_stage: Stage | None = None diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/defaults.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/defaults.py index 2d30f25..4073a5b 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/defaults.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/defaults.py @@ -41,7 +41,7 @@ def default_pipeline( logging: LoggingPolicy | None = None, tracing: TracingPolicy | None = None, ) -> StagedPipelineBuilder: - """Pre-configured :class:`StagedPipelineBuilder` with the canonical stack. + """Pre-configured `StagedPipelineBuilder` with the canonical stack. Wires the policies that most consumers want by default in the order their stages dictate: redirect → idempotency → retry → set-date → @@ -55,21 +55,21 @@ def default_pipeline( Args: client: Terminal HTTP transport. - redirect: Override for :class:`RedirectPolicy`. ``None`` uses defaults. - idempotency: Override for :class:`IdempotencyPolicy`. ``None`` uses + redirect: Override for `RedirectPolicy`. ``None`` uses defaults. + idempotency: Override for `IdempotencyPolicy`. ``None`` uses defaults. - retry: Override for :class:`RetryPolicy`. ``None`` uses defaults. - set_date: Override for :class:`SetDatePolicy`. ``None`` uses defaults. - client_identity: Override for :class:`ClientIdentityPolicy`. ``None`` + retry: Override for `RetryPolicy`. ``None`` uses defaults. + set_date: Override for `SetDatePolicy`. ``None`` uses defaults. + client_identity: Override for `ClientIdentityPolicy`. ``None`` uses defaults. auth: Optional authentication policy (``BearerTokenPolicy``, ``BasicAuthPolicy``, ``KeyCredentialPolicy``, etc.). No default — requests pass without authentication when this is ``None``. - logging: Override for :class:`LoggingPolicy`. ``None`` uses defaults. - tracing: Override for :class:`TracingPolicy`. ``None`` uses defaults. + logging: Override for `LoggingPolicy`. ``None`` uses defaults. + tracing: Override for `TracingPolicy`. ``None`` uses defaults. Returns: - A :class:`StagedPipelineBuilder` ready for further customisation or + A `StagedPipelineBuilder` ready for further customisation or immediate ``.build()``. """ builder = StagedPipelineBuilder(client) @@ -95,7 +95,7 @@ def default_async_pipeline( client_identity: AsyncClientIdentityPolicy | None = None, auth: AsyncPolicy | None = None, ) -> AsyncStagedPipelineBuilder: - """Async twin of :func:`default_pipeline`. + """Async twin of `default_pipeline`. Mirrors the sync version's stack minus logging/tracing, which currently only ship as sync policies. Async-side observability lives on the caller's diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/pipeline.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/pipeline.py index 98689e6..3b65ae7 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/pipeline.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/pipeline.py @@ -115,7 +115,14 @@ def run( """ request_ctx = dispatch.to_request_context(request) ctx = PipelineContext(call=request_ctx, options=dict(options)) - return self._chain.send(request, ctx) + try: + return self._chain.send(request, ctx) + finally: + # Evict the call's ``ContextStore`` entry once the chain has fully + # unwound — in-chain observers have already read the latest tier. + # The exchange context shares this trace id, so a single close + # clears both tiers and prevents unbounded growth across calls. + request_ctx.close() def _wrap_step(step: Any) -> Policy: diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_client_identity.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_client_identity.py index dd3f012..1bcfb0c 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_client_identity.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_client_identity.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Async twin of :class:`ClientIdentityPolicy`.""" +"""Async twin of `ClientIdentityPolicy`.""" from __future__ import annotations @@ -19,15 +19,15 @@ class AsyncClientIdentityPolicy(AsyncPolicy): - """Async variant of :class:`ClientIdentityPolicy`. + """Async variant of `ClientIdentityPolicy`. Behaviour mirrors the sync twin: the ``User-Agent`` defaults to - :func:`default_user_agent`, append-vs-replace is selectable, and the token - is guaranteed non-blank. Building the value is synchronous, so :meth:`send` + `default_user_agent`, append-vs-replace is selectable, and the token + is guaranteed non-blank. Building the value is synchronous, so `send` differs only in the ``await`` on the downstream call. Attributes: - STAGE: Pinned to :attr:`Stage.POST_RETRY` at the type level so + STAGE: Pinned to `Stage.POST_RETRY` at the type level so mis-slotting is caught by ``mypy``. """ @@ -39,7 +39,7 @@ def __init__(self, *, user_agent: str | None = None, replace: bool = False) -> N Args: user_agent: ``User-Agent`` token to stamp. ``None`` (the default) - uses :func:`default_user_agent`. An empty or whitespace-only + uses `default_user_agent`. An empty or whitespace-only value is rejected so the header is never blank. replace: When ``True``, overwrite any caller-set ``User-Agent``. When ``False`` (the default), append after the caller's value. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_idempotency.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_idempotency.py index 2f60355..4c004dc 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_idempotency.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_idempotency.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Async twin of :class:`IdempotencyPolicy`.""" +"""Async twin of `IdempotencyPolicy`.""" from __future__ import annotations @@ -25,16 +25,16 @@ class AsyncIdempotencyPolicy(AsyncPolicy): - """Async variant of :class:`IdempotencyPolicy`. + """Async variant of `IdempotencyPolicy`. Behaviour mirrors the sync twin: a key is minted once at - :attr:`Stage.POST_REDIRECT` (outside the retry wrapper) and reused across + `Stage.POST_REDIRECT` (outside the retry wrapper) and reused across every retry of the same write request, and a caller-supplied header is left - untouched. Key generation is synchronous, so :meth:`send` differs from the + untouched. Key generation is synchronous, so `send` differs from the sync version only in the ``await`` on the downstream call. Attributes: - STAGE: Pinned to :attr:`Stage.POST_REDIRECT` at the type level so + STAGE: Pinned to `Stage.POST_REDIRECT` at the type level so mis-slotting is caught by ``mypy``. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_redirect.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_redirect.py index af5f40f..84ce23a 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_redirect.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_redirect.py @@ -3,7 +3,7 @@ """Async twin of ``RedirectPolicy``. -Mirrors :class:`RedirectPolicy` exactly — same status-code matrix, same +Mirrors `RedirectPolicy` exactly — same status-code matrix, same credential stripping, same loop guard — but ``send`` is ``async`` and operates on ``AsyncResponse``. The per-hop decision helpers are shared via delegation to a wrapped sync ``RedirectPolicy`` instance. @@ -27,7 +27,7 @@ class AsyncRedirectPolicy(AsyncPolicy): """Async redirect policy. - Reuses :class:`RedirectPolicy` for configuration and per-hop request + Reuses `RedirectPolicy` for configuration and per-hop request construction (status-code matrix, credential stripping, body replay check). Only the dispatch loop is awaited — ``self.next.send`` is the async chain head. @@ -75,7 +75,14 @@ async def send(self, request: Request, ctx: PipelineContext) -> AsyncResponse: location = response.headers.get("Location") if location is None or not location.strip(): return response - next_request = cfg._build_next_request(current_request, status, location) + try: + next_request = cfg._build_next_request(current_request, status, location) + except RuntimeError: + # A body-preserving redirect with a non-replayable body cannot + # be reissued. Close the in-hand 3xx response before the error + # escapes so the connection is not leaked. + await response.close() + raise if next_request is None: return response next_key = str(next_request.url) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_retry.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_retry.py index 540fb62..22747dd 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_retry.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_retry.py @@ -37,6 +37,34 @@ _LOGGER = logging.getLogger(__name__) +class _SyncClockView: + """Expose an `AsyncClock`'s synchronous readings as a sync `Clock`. + + ``AsyncClock`` already provides synchronous ``now`` and ``monotonic``; + only ``sleep`` is awaitable. ``AsyncRetryPolicy`` drives sleeping through + its own async ``_sleep_bounded``, so the inner ``RetryPolicy`` config + never calls ``sleep``. This view threads the injected clock's wall-time + into that config so an HTTP-date ``Retry-After`` is measured against the + same clock on the async path as on the sync path. + """ + + __slots__ = ("_clock",) + + def __init__(self, clock: AsyncClock) -> None: + self._clock = clock + + def now(self) -> float: + """Return the wrapped clock's wall-clock reading, in seconds.""" + return self._clock.now() + + def monotonic(self) -> float: + """Return the wrapped clock's monotonic reading, in seconds.""" + return self._clock.monotonic() + + def sleep(self, duration: float) -> None: + """No-op: the async retry loop sleeps via its own async path.""" + + class AsyncRetryPolicy(AsyncPolicy): """Async retry policy. @@ -50,6 +78,8 @@ class AsyncRetryPolicy(AsyncPolicy): STAGE = Stage.RETRY + __slots__ = ("_clock", "config") + config: RetryPolicy def __init__( @@ -93,6 +123,10 @@ def __init__( kwargs["retry_on_status_codes"] = retry_on_status_codes if rand is not None: kwargs["rand"] = rand + # Thread the injected clock into the inner config so the HTTP-date + # ``Retry-After`` branch (``RetryPolicy._server_delay``) measures + # delay against the same clock as the sync policy, not wall time. + kwargs["clock"] = _SyncClockView(clock) self.config = RetryPolicy(**kwargs) self._clock = clock @@ -143,6 +177,10 @@ async def send(self, request: Request, ctx: PipelineContext) -> AsyncResponse: ctx.data["retry_count"] = len(history) delay = cfg._delay_for(settings, response) tracer.attempt_failed(_StatusRetryError(int(response.status)), delay) + # The intermediate response is not handed back to the caller, so + # close it to release the pooled connection before sleeping. The + # return branches above keep the response open — the caller owns it. + await response.close() await self._sleep_bounded(delay, absolute_deadline) async def _sleep_bounded( diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_set_date.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_set_date.py index 1005ac5..1d468fb 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_set_date.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/async_set_date.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Async twin of :class:`SetDatePolicy`.""" +"""Async twin of `SetDatePolicy`.""" from __future__ import annotations @@ -19,17 +19,17 @@ class AsyncSetDatePolicy(AsyncPolicy): - """Async variant of :class:`SetDatePolicy`. + """Async variant of `SetDatePolicy`. The clock reading itself is synchronous (``AsyncClock.now`` returns a - plain float), so the body of :meth:`send` is identical to the sync + plain float), so the body of `send` is identical to the sync twin apart from the ``await`` on the downstream send. - The policy is placed at :attr:`Stage.POST_RETRY` so each retry + The policy is placed at `Stage.POST_RETRY` so each retry attempt receives a fresh timestamp. Attributes: - STAGE: Pinned to :attr:`Stage.POST_RETRY` at the type level so + STAGE: Pinned to `Stage.POST_RETRY` at the type level so mis-slotting is caught by ``mypy``. """ @@ -41,7 +41,7 @@ def __init__(self, *, clock: AsyncClock = ASYNC_SYSTEM_CLOCK) -> None: Args: clock: Source of wall-clock time. Defaults to the process-wide - :data:`ASYNC_SYSTEM_CLOCK`; tests substitute a deterministic + `ASYNC_SYSTEM_CLOCK`; tests substitute a deterministic fake. """ self._clock = clock diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/client_identity.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/client_identity.py index 8edefc8..e20f485 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/client_identity.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/client_identity.py @@ -47,7 +47,7 @@ def default_user_agent() -> str: The shape is ``dexpace-sdk/ python/`` — for example ``dexpace-sdk/1.2.0 python/3.12.4``. Transport packages may append their own ``/`` token by passing a longer ``user_agent`` to - :class:`ClientIdentityPolicy`. + `ClientIdentityPolicy`. Returns: A non-empty ``User-Agent`` string. @@ -60,7 +60,7 @@ class ClientIdentityPolicy(Policy): A consistent ``User-Agent`` lets servers and the SDK's own observability attribute traffic to the toolkit and its version. The token string defaults - to :func:`default_user_agent` (``dexpace-sdk/ python/``). + to `default_user_agent` (``dexpace-sdk/ python/``). Two modes control interaction with a caller-set header: @@ -73,7 +73,7 @@ class ClientIdentityPolicy(Policy): an empty ``User-Agent`` header. Attributes: - STAGE: Pinned to :attr:`Stage.POST_RETRY` at the type level so + STAGE: Pinned to `Stage.POST_RETRY` at the type level so mis-slotting is caught by ``mypy``. Example: @@ -90,7 +90,7 @@ def __init__(self, *, user_agent: str | None = None, replace: bool = False) -> N Args: user_agent: ``User-Agent`` token to stamp. ``None`` (the default) - uses :func:`default_user_agent`. An empty or whitespace-only + uses `default_user_agent`. An empty or whitespace-only value is rejected so the header is never blank. replace: When ``True``, overwrite any caller-set ``User-Agent``. When ``False`` (the default), append after the caller's value. diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/idempotency.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/idempotency.py index 22a4ded..bca7bf2 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/idempotency.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/idempotency.py @@ -43,13 +43,13 @@ class IdempotencyPolicy(Policy): methods (``POST``/``PUT``/``PATCH`` by default) are stamped; idempotency keys on ``GET``/``DELETE`` are meaningless to most servers. - The policy is placed at :attr:`Stage.POST_REDIRECT`, which runs *outside* - the retry wrapper (:attr:`Stage.RETRY`). The key is therefore minted on the + The policy is placed at `Stage.POST_REDIRECT`, which runs *outside* + the retry wrapper (`Stage.RETRY`). The key is therefore minted on the first pass and reused on every retry re-send, rather than re-rolled per - attempt the way :class:`SetDatePolicy` re-stamps the ``Date`` header. + attempt the way `SetDatePolicy` re-stamps the ``Date`` header. Attributes: - STAGE: Pinned to :attr:`Stage.POST_REDIRECT` at the type level so + STAGE: Pinned to `Stage.POST_REDIRECT` at the type level so mis-slotting is caught by ``mypy``. Example: diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/redirect.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/redirect.py index 1fd40e1..1d68a37 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/redirect.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/redirect.py @@ -142,7 +142,14 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: location = response.headers.get("Location") if location is None or not location.strip(): return response - next_request = self._build_next_request(current_request, status, location) + try: + next_request = self._build_next_request(current_request, status, location) + except RuntimeError: + # A body-preserving redirect with a non-replayable body cannot + # be reissued. Close the in-hand 3xx response before the error + # escapes so the connection is not leaked. + response.close() + raise if next_request is None: return response next_key = str(next_request.url) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/retry.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/retry.py index a339425..fb2e758 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/retry.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/retry.py @@ -22,7 +22,6 @@ import logging import random import re -import time from collections.abc import Iterable from email.utils import parsedate_to_datetime from enum import StrEnum @@ -116,8 +115,10 @@ class RetryPolicy(Policy): retry_mode: ``EXPONENTIAL`` or ``FIXED`` (the latter sleeps ``backoff_factor`` between every attempt). timeout: Absolute time budget for the entire chain, in seconds. - method_allowlist: HTTP methods that get full retry semantics. - POST and PATCH are retried only on 500/503/504. + method_allowlist: HTTP methods that get full retry semantics on any + code in ``retry_on_status_codes``. POST and PATCH are special-cased + and unconditionally limited to status retries on 500/503/504 — they + cannot be widened by adding them to this allowlist. retry_on_status_codes: Status codes that trigger a retry. respect_retry_after: When ``True``, sleep for the server-supplied delay (``Retry-After`` header in seconds/HTTP-date, or an @@ -153,6 +154,25 @@ class RetryPolicy(Policy): STAGE = Stage.RETRY + __slots__ = ( + "_clock", + "_full_jitter", + "_jitter", + "_rand", + "backoff_factor", + "backoff_max", + "connect_retries", + "method_allowlist", + "read_retries", + "respect_retry_after", + "retry_after_max", + "retry_mode", + "retry_on_status_codes", + "status_retries", + "timeout", + "total_retries", + ) + def __init__( self, *, @@ -210,19 +230,6 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: tracer.attempt_started(len(history)) try: response = self.next.send(request, ctx) - if not self._is_retry(settings, request, response): - ctx.data["retry_history"] = tuple(history) - return response - history.append(RequestHistory(request=request, response=response)) - if not self._decrement_status(settings): - tracer.attempt_retries_exhausted() - ctx.data["retry_history"] = tuple(history) - return response - ctx.data["retry_count"] = len(history) - delay = self._delay_for(settings, response) - tracer.attempt_failed(_StatusRetryError(int(response.status)), delay) - self._sleep_bounded(delay, absolute_deadline) - continue except ClientAuthenticationError: raise except SdkError as err: @@ -234,9 +241,28 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: ctx.data["retry_count"] = len(history) delay = self._delay_for(settings, None) tracer.attempt_failed(err, delay) + # Sleep outside the try so a deadline crossed during the + # backoff raises ``ServiceResponseTimeoutError`` straight to + # the caller instead of being swallowed by ``except SdkError``. self._sleep_bounded(delay, absolute_deadline) _LOGGER.debug("retrying after %s: %s", type(err).__name__, err) continue + if not self._is_retry(settings, request, response): + ctx.data["retry_history"] = tuple(history) + return response + history.append(RequestHistory(request=request, response=response)) + if not self._decrement_status(settings): + tracer.attempt_retries_exhausted() + ctx.data["retry_history"] = tuple(history) + return response + ctx.data["retry_count"] = len(history) + delay = self._delay_for(settings, response) + tracer.attempt_failed(_StatusRetryError(int(response.status)), delay) + # The intermediate response is not handed back to the caller, so + # close it to release the pooled connection before sleeping. The + # return branches above keep the response open — the caller owns it. + response.close() + self._sleep_bounded(delay, absolute_deadline) # ----- configuration -------------------------------------------------- @@ -359,12 +385,13 @@ def _server_delay(self, response: _ResponseLike) -> float | None: Returns: Seconds to wait, or ``None`` when neither header is present. """ - retry_after = _parse_retry_after(response.headers.get("Retry-After")) + now = self._clock.now() + retry_after = _parse_retry_after(response.headers.get("Retry-After"), now) if retry_after is not None: return min(retry_after, self.retry_after_max) reset = _parse_rate_limit_reset( response.headers.get(_RATE_LIMIT_RESET_HEADER), - self._clock.now(), + now, ) if reset is None: return None @@ -435,11 +462,14 @@ def __init__(self, status: int) -> None: _RETRY_AFTER_DELTA_PATTERN = re.compile(r"^\s*\d+(\.\d+)?\s*$") -def _parse_retry_after(value: str | None) -> float | None: +def _parse_retry_after(value: str | None, now: float) -> float | None: """Parse a ``Retry-After`` header value (delta-seconds or HTTP-date). Args: value: Raw header value. ``None`` returns ``None`` directly. + now: Current wall-clock time, in seconds since the epoch, used to + convert an HTTP-date into a delay. Injected so the value is + deterministic in tests. Returns: Seconds to wait (>= 0), or ``None`` when ``value`` is missing or @@ -455,7 +485,7 @@ def _parse_retry_after(value: str | None) -> float | None: return None if when is None: return None - delta = when.timestamp() - time.time() + delta = when.timestamp() - now return max(0.0, delta) diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/set_date.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/set_date.py index f681ac6..7f6dc3a 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/set_date.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/policies/set_date.py @@ -26,14 +26,14 @@ class SetDatePolicy(Policy): shape. Any caller-supplied ``Date`` header is overwritten so the value on the wire always reflects the moment the request leaves this policy. - The policy is placed at :attr:`Stage.POST_RETRY` so each retry attempt + The policy is placed at `Stage.POST_RETRY` so each retry attempt receives a fresh timestamp. Stamping earlier — outside the retry wrapper — would cache the time across attempts and risk false signatures on services that bind the date into request signing (notably AWS SigV4 and similar). Attributes: - STAGE: Pinned to :attr:`Stage.POST_RETRY` at the type level so + STAGE: Pinned to `Stage.POST_RETRY` at the type level so mis-slotting is caught by ``mypy``. Example: @@ -50,7 +50,7 @@ def __init__(self, *, clock: Clock = SYSTEM_CLOCK) -> None: Args: clock: Source of wall-clock time. Defaults to the process-wide - :data:`SYSTEM_CLOCK`; tests substitute a deterministic fake. + `SYSTEM_CLOCK`; tests substitute a deterministic fake. """ self._clock = clock diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/staged_builder.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/staged_builder.py index 2449d3c..8aa37f2 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/staged_builder.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/staged_builder.py @@ -26,15 +26,15 @@ class StagedPipelineBuilder: - """Build a :class:`Pipeline` by stage rather than user-specified order. + """Build a `Pipeline` by stage rather than user-specified order. Surgical edits (``replace`` / ``insert_after`` / ``insert_before`` / ``remove``) take a policy *type* and operate on the first matching instance — useful for tweaking an already-built pipeline obtained via - :meth:`from_pipeline`. + `from_pipeline`. Thread-safety: not thread-safe. Construct on one thread, then call - :meth:`build` — the resulting :class:`Pipeline` is independent. + `build` — the resulting `Pipeline` is independent. """ __slots__ = ("_buckets", "_client", "_pillars") @@ -71,7 +71,7 @@ def append(self, policy: Policy, *, force: bool = False) -> Self: def prepend(self, policy: Policy, *, force: bool = False) -> Self: """Prepend ``policy`` to the head of its stage's bucket. - Pillar behaviour mirrors :meth:`append`. + Pillar behaviour mirrors `append`. """ stage = policy.STAGE if stage.is_pillar: @@ -133,12 +133,12 @@ def remove(self, target: type[Policy]) -> Self: return self def build(self) -> Pipeline: - """Flatten the builder's contents into a :class:`Pipeline`.""" + """Flatten the builder's contents into a `Pipeline`.""" return Pipeline(self._client, policies=self._flatten()) @classmethod def from_pipeline(cls, pipeline: Pipeline) -> Self: - """Seed a builder from an existing :class:`Pipeline`'s policies. + """Seed a builder from an existing `Pipeline`'s policies. Walks the pipeline's policy chain (skipping the internal transport runner) and re-slots each policy into its declared stage. Useful @@ -148,7 +148,9 @@ def from_pipeline(cls, pipeline: Pipeline) -> Self: Raises: ValueError: If the input pipeline's policies do not satisfy stage ordering — i.e. their declared stages do not appear - in non-decreasing order in the chain. + in non-decreasing order in the chain — or if the chain + contains a list-constructor SansIO step (a bare callable), + which carries no ``STAGE`` and so cannot be rehydrated. """ from ._transport_runner import _TransportRunner @@ -156,6 +158,13 @@ def from_pipeline(cls, pipeline: Pipeline) -> Self: chain: list[Policy] = [] node: Policy | None = pipeline._chain while node is not None and not isinstance(node, _TransportRunner): + if getattr(node, "STAGE", None) is None: + raise ValueError( + f"Pipeline node {type(node).__name__} carries no STAGE; " + f"it is a list-constructor SansIO step (a bare callable) " + f"that cannot be rehydrated into a staged builder. Rebuild " + f"the pipeline via the list constructor instead." + ) chain.append(node) node = getattr(node, "next", None) last_stage: Stage | None = None diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/step/config.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/step/config.py index d3edebd..190123e 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/step/config.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/pipeline/step/config.py @@ -13,7 +13,7 @@ class StepMetadata: """Human-readable identification for a pipeline step. Used by logging, tracing, and tooling that needs to identify a step at - runtime. Bump :attr:`version` when behavior changes in a non + runtime. Bump `version` when behavior changes in a non back-compatible way. """ @@ -23,24 +23,4 @@ class StepMetadata: tags: tuple[str, ...] = () -@dataclass(frozen=True, slots=True) -class RetryConfig: - """Configuration describing how a step should be retried on failure. - - Defaults provide a reasonable fixed-delay policy with exponential backoff - disabled; override individual fields to tune. ``retry_on`` defaults to - transient I/O failures (:class:`OSError`, :class:`TimeoutError`) — - programmer-error exceptions are intentionally excluded so retries don't - paper over bugs. - """ - - timeout_ms: int = 10_000 - exponential_backoff: bool = False - initial_backoff_ms: int = 1_000 - max_backoff_ms: int = 10_000 - multiplier: float = 2.0 - max_retries: int = 3 - retry_on: tuple[type[BaseException], ...] = (OSError, TimeoutError) - - -__all__ = ["RetryConfig", "StepMetadata"] +__all__ = ["StepMetadata"] diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/codec.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/codec.py index 11d0aba..836a9e3 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/codec.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/codec.py @@ -13,8 +13,8 @@ The codec never touches JSON (or any other) syntax, so it is format-agnostic and reusable with any ``Serde``. It is deliberately validation-free: it reconstructs declared types, handles aliases, ``Tristate`` fields, datetimes, -enums, containers and discriminated unions, but performs no schema checks or -scalar coercion. The model's own ``__post_init__`` invariants still run because +``UUID``s, enums, containers and discriminated unions, but performs no schema +checks or scalar coercion. The model's own ``__post_init__`` invariants still run because construction goes through the normal constructor. """ @@ -26,6 +26,7 @@ import enum import types import typing +import uuid from typing import Final, Union, cast, get_args, get_origin, get_type_hints from ..errors import DeserializationError, SerializationError @@ -233,7 +234,8 @@ def decode[T](self, data: object, target: type[T]) -> T: data: A plain document (``dict`` / ``list`` / scalar) as produced by ``Serde.deserializer``. target: The type to reconstruct — a dataclass, a discriminated base, - ``list[X]`` / ``dict[str, X]``, a datetime/enum, or a scalar. + ``list[X]`` / ``dict[str, X]``, a datetime/UUID/enum, or a + scalar. Returns: A fully constructed instance of ``target``. @@ -248,8 +250,8 @@ def encode(self, value: object) -> object: """Encode a typed value into a plain document. Args: - value: A dataclass, container, datetime, enum, ``Tristate`` field - value, or scalar. + value: A dataclass, container, datetime, ``UUID``, enum, + ``Tristate`` field value, or scalar. Returns: A plain document (``dict`` / ``list`` / scalar) ready for @@ -275,11 +277,11 @@ def _decode_value( """Decode ``data`` into ``target``, dispatching on the type's shape.""" if target is object or target is typing.Any: return data + if _is_tristate(target): + return _decode_tristate(data, target, path, tolerate_unknown) origin = get_origin(target) if origin is None: return _decode_atomic(data, target, path, tolerate_unknown) - if _is_tristate(target): - return _decode_tristate(data, target, path, tolerate_unknown) if origin in (Union, types.UnionType): return _decode_union(data, target, path, tolerate_unknown) return _decode_container(data, target, origin, path, tolerate_unknown) @@ -301,6 +303,8 @@ def _decode_atomic( return _decode_enum(data, target, path) if issubclass(target, (_dt.datetime, _dt.date, _dt.time)): return _decode_temporal(data, target, path) + if issubclass(target, uuid.UUID): + return _decode_uuid(data, target, path) return data @@ -475,9 +479,12 @@ def _decode_mapping( ) -> object: """Decode a mapping, recovering each key and value through its declared type. - Wire object keys are always strings, so a declared key type such as ``int``, - an enum, or ``UUID`` is recovered by recursing on the key the same way the - codec recurses on values; ``str`` and ``object`` key types pass through. + Wire object keys are always strings. A declared key type that has a + dedicated reconstruction branch — an enum, a datetime/date/time, or a + ``UUID`` — is recovered by recursing on the key the same way the codec + recurses on values. ``str`` and ``object`` keys pass through, and a bare + scalar key type (``int`` / ``float`` / ``bool``) follows the codec's + no-coercion rule, so its wire string is returned unchanged. """ if not isinstance(data, cabc.Mapping): raise CodecError(f"expected an object, got {type(data).__name__}", path=path) @@ -567,6 +574,25 @@ def _decode_temporal(data: object, target: type, path: tuple[str, ...]) -> objec ) from err +def _decode_uuid(data: object, target: type, path: tuple[str, ...]) -> object: + """Decode a string into a ``UUID`` (the canonical form ``_encode_value`` emits).""" + if not isinstance(data, str): + raise CodecError( + f"expected a UUID string, got {type(data).__name__}", + path=path, + target_name=target.__name__, + ) + try: + return target(data) + except ValueError as err: + raise CodecError( + f"{data!r} is not a valid {target.__name__}", + path=path, + target_name=target.__name__, + error=err, + ) from err + + def _dispatch_union( data: object, base: type, @@ -632,13 +658,15 @@ def _encode_value(value: object) -> object: return _encode_dataclass(value) if isinstance(value, (_dt.datetime, _dt.date, _dt.time)): return value.isoformat() + if isinstance(value, uuid.UUID): + return str(value) if isinstance(value, bytes): try: return value.decode("utf-8") except UnicodeDecodeError as err: raise SerializationError("cannot encode non-UTF-8 bytes value") from err if isinstance(value, cabc.Mapping): - return {k: _encode_value(v) for k, v in value.items()} + return {_encode_key(k): _encode_value(v) for k, v in value.items()} if isinstance(value, (list, tuple, set, frozenset)): return [_encode_value(v) for v in value] raise SerializationError(f"cannot encode value of type {type(value).__name__}") @@ -661,6 +689,42 @@ def _encode_dataclass(value: object) -> dict[str, object]: return out +def _encode_key(key: object) -> str | int | float | bool | None: + """Collapse a mapping key into a document-legal scalar key. + + Runs the key through ``_encode_value`` (which folds an enum to its value, a + datetime/date/time to its ISO string, and a ``UUID`` to its canonical + string) then ensures the result is a JSON object key type. A leftover + ``str`` / ``int`` / ``float`` / ``bool`` / ``None`` passes through; anything + else is coerced to ``str(...)``. + + Round-trip note: enum, temporal, and ``UUID`` keys reconstruct to their + declared type on decode because ``_decode_mapping`` recurses through a + branch that rebuilds them. A bare scalar key (``int`` / ``float`` / ``bool``) + follows the codec's no-coercion rule instead — JSON renders it as a string + and decode returns that string unchanged, so such a key does not survive a + full wire round-trip as its original type. + + Args: + key: The original mapping key. + + Returns: + A ``str`` / ``int`` / ``float`` / ``bool`` / ``None`` document key. + + Raises: + SerializationError: If the collapsed key is a container or other type + that has no meaningful scalar key form. + """ + encoded = _encode_value(key) + if encoded is None or isinstance(encoded, (str, int, float, bool)): + return encoded + if isinstance(encoded, (cabc.Mapping, list, tuple, set, frozenset)): + raise SerializationError( + f"cannot encode mapping key of type {type(key).__name__}", + ) + return str(encoded) + + # --------------------------------------------------------------------------- # # Type introspection helpers # # --------------------------------------------------------------------------- # @@ -689,8 +753,11 @@ def _is_tristate(target: object) -> bool: ``get_type_hints`` resolves a ``type`` alias such as ``Tristate[str]`` to a ``GenericAlias`` whose origin is the ``Tristate`` alias object itself, not a ``Union``. Older / expanded forms surface as a ``Present``-bearing union, so - both shapes are recognised. + both shapes are recognised. A bare, non-parametrised ``Tristate`` field is + treated as ``Tristate[object]`` (inner type ``object``). """ + if target is Tristate: + return True if get_origin(target) is Tristate: return True if get_origin(target) in (Union, types.UnionType): diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/json_serde.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/json_serde.py index fa4dc79..0ea77fc 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/json_serde.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/json_serde.py @@ -60,21 +60,43 @@ def __init__( def serialize(self, value: Any) -> str: """Serialise ``value`` to a JSON string. + ``json`` treats ``cls=`` and ``default=`` as mutually exclusive: passing + both makes the encoder class's ``default`` method unreachable. To keep + the built-in datetime / date / time / bytes handling alongside a + caller-supplied ``default``, the user callable is chained behind the + encoder class instead — the encoder's checks run first and the user + fallback only sees types the encoder rejects. + Raises: SerializationError: If encoding fails. """ + encoder = self._build_encoder() try: - return json.dumps( - value, - cls=self._encoder_cls, - default=self._default, - sort_keys=self._sort_keys, - allow_nan=self._allow_nan, - separators=(",", ":"), - ) + return encoder.encode(value) except (TypeError, ValueError, UnicodeDecodeError) as err: raise SerializationError(str(err), error=err) from err + def _build_encoder(self) -> json.JSONEncoder: + """Build an encoder chaining the built-in ``default`` to the user one.""" + encoder_cls = self._encoder_cls + kwargs: dict[str, Any] = { + "sort_keys": self._sort_keys, + "allow_nan": self._allow_nan, + "separators": (",", ":"), + } + if self._default is None: + return encoder_cls(**kwargs) + fallback: Callable[[Any], Any] = self._default + + class _ChainedEncoder(encoder_cls): # type: ignore[valid-type, misc] + def default(self, o: Any) -> Any: + try: + return super().default(o) + except TypeError: + return fallback(o) + + return _ChainedEncoder(**kwargs) + def serialize_to_bytes(self, value: Any) -> bytes: """Serialise ``value`` to UTF-8-encoded JSON bytes.""" return self.serialize(value).encode("utf-8") diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/serde.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/serde.py index 0b58df7..0e1d361 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/serde.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/serde/serde.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -""":class:`Serde`, :class:`Serializer`, :class:`Deserializer` Protocols.""" +"""`Serde`, `Serializer`, `Deserializer` Protocols.""" from __future__ import annotations @@ -45,7 +45,7 @@ class Serde(Protocol): """Bundle of serialization / deserialization strategies for a single wire format. Acts as a single injection point: components that need to round-trip values - pull a :class:`Serde` rather than separate serializer and deserializer + pull a `Serde` rather than separate serializer and deserializer references, which keeps the dependency surface flat and makes it easy to swap formats at the edge of the SDK. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/__init__.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/__init__.py index 8e8718b..7037ca5 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/__init__.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/__init__.py @@ -3,9 +3,9 @@ """Cross-cutting utilities used throughout the SDK core. -Exports the :class:`Clock` / :class:`AsyncClock` abstractions that let +Exports the `Clock` / `AsyncClock` abstractions that let time-dependent code (retry backoff, token expiry) be driven deterministically -in tests, and the :class:`ProxyOptions` value type used to describe outbound +in tests, and the `ProxyOptions` value type used to describe outbound HTTP / SOCKS proxies in a transport-agnostic way. """ diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/clock.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/clock.py index d9928ac..ff5be3e 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/clock.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/clock.py @@ -6,7 +6,7 @@ Time-dependent components (retry backoff, bearer-token refresh, deadline arithmetic) accept a ``Clock`` / ``AsyncClock`` rather than calling ``time.time`` / ``time.sleep`` / ``asyncio.sleep`` directly. Production -code wires in :data:`SYSTEM_CLOCK` or :data:`ASYNC_SYSTEM_CLOCK`; tests +code wires in `SYSTEM_CLOCK` or `ASYNC_SYSTEM_CLOCK`; tests substitute a deterministic fake (see ``tests/conftest.py::FakeClock``). Both protocols are ``@runtime_checkable`` so callers can assert @@ -61,7 +61,7 @@ def sleep(self, duration: float) -> None: @runtime_checkable class AsyncClock(Protocol): - """Async twin of :class:`Clock` for use inside ``async def`` callers. + """Async twin of `Clock` for use inside ``async def`` callers. ``now`` / ``monotonic`` remain synchronous — they are cheap reads against the operating system. Only ``sleep`` is awaitable. @@ -86,7 +86,7 @@ async def sleep(self, duration: float) -> None: class _SystemClock: - """Default :class:`Clock` backed by :mod:`time`.""" + """Default `Clock` backed by `time`.""" __slots__ = () @@ -105,7 +105,7 @@ def sleep(self, duration: float) -> None: class _AsyncSystemClock: - """Default :class:`AsyncClock` backed by :mod:`asyncio`.""" + """Default `AsyncClock` backed by `asyncio`.""" __slots__ = () @@ -124,7 +124,7 @@ async def sleep(self, duration: float) -> None: SYSTEM_CLOCK: Clock = _SystemClock() -"""Process-wide default :class:`Clock` backed by the standard library.""" +"""Process-wide default `Clock` backed by the standard library.""" ASYNC_SYSTEM_CLOCK: AsyncClock = _AsyncSystemClock() -"""Process-wide default :class:`AsyncClock` backed by :mod:`asyncio`.""" +"""Process-wide default `AsyncClock` backed by `asyncio`.""" diff --git a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/proxy.py b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/proxy.py index 7d8b12b..4e99740 100644 --- a/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/proxy.py +++ b/packages/dexpace-sdk-core/src/dexpace/sdk/core/util/proxy.py @@ -5,11 +5,19 @@ ``ProxyOptions`` describes an outbound HTTP / SOCKS proxy together with the list of hosts that should bypass it. Instances are immutable and the bypass -globs (``*.internal.example.com`` style) are compiled exactly once at -construction time so per-request matching is a plain regex check. - -The :meth:`ProxyOptions.from_configuration` factory bridges the proxy value -type to the layered :class:`Configuration` lookup: it reads ``HTTPS_PROXY`` +entries are compiled exactly once at construction time so per-request +matching is a plain comparison. + +Bypass entries follow the conventional ``NO_PROXY`` suffix semantics used by +curl, requests, and Go: a bare entry such as ``example.com`` bypasses the +host itself and any dot-delimited subdomain (``api.example.com``); a leading +dot (``.example.com``) is treated identically. A trailing ``:port`` on either +the entry or the candidate host is ignored, so matching is host-only. Entries +containing a glob metacharacter (``*`` / ``?`` / ``[``) keep their ``fnmatch`` +behaviour so existing ``*.example.com`` style patterns continue to work. + +The ``ProxyOptions.from_configuration`` factory bridges the proxy value +type to the layered ``Configuration`` lookup: it reads ``HTTPS_PROXY`` (preferred) or ``HTTP_PROXY`` as full URLs and ``NO_PROXY`` as a comma-separated bypass list. Parse failures degrade to ``None`` rather than raising — bad proxy configuration should never bring down the caller. @@ -20,6 +28,7 @@ import fnmatch import logging import re +from collections.abc import Callable from dataclasses import dataclass, field from enum import StrEnum from typing import Self @@ -32,6 +41,61 @@ _LOG = logging.getLogger(__name__) +# Glob metacharacters that switch an entry into ``fnmatch`` mode. +_GLOB_CHARS: frozenset[str] = frozenset("*?[") + + +def _strip_port(host: str) -> str: + """Drop a trailing ``:port`` (and IPv6 brackets) so matching is host-only. + + A bracketed IPv6 literal (``[::1]`` or ``[::1]:443``) yields its inner + address; a ``host:port`` carrying a single colon drops the port; a bare + IPv6 literal (multiple colons, no port) is returned unchanged. + + Args: + host: A candidate host or a bypass entry, possibly port-qualified. + + Returns: + The host with any port and IPv6 brackets removed. + """ + if host.startswith("["): + end = host.find("]") + return host[1:end] if end != -1 else host + if host.count(":") == 1: + name, _, port = host.partition(":") + if port.isdigit(): + return name + return host + + +def _compile_bypass(pattern: str) -> Callable[[str], bool]: + """Compile a single ``NO_PROXY`` entry into a case-insensitive matcher. + + Entries containing a glob metacharacter (``*`` / ``?`` / ``[``) keep + their ``fnmatch`` semantics. Every other (bare) entry uses conventional + suffix matching: a candidate matches when it equals the entry or ends + with ``"." + entry``. Leading dot(s) on the entry are stripped so + ``.example.com`` and ``example.com`` behave identically, and a trailing + ``:port`` is dropped so ``example.com:443`` matches on its host part. + + Args: + pattern: A raw ``NO_PROXY`` list entry (already stripped). + + Returns: + A predicate mapping a lower-cased candidate host to a bypass boolean. + """ + if any(char in pattern for char in _GLOB_CHARS): + regex = re.compile(fnmatch.translate(pattern), re.IGNORECASE) + return lambda host: regex.match(host) is not None + suffix = _strip_port(pattern).lstrip(".").lower() + dotted = "." + suffix + + def matches(host: str) -> bool: + candidate = host.lower() + return candidate == suffix or candidate.endswith(dotted) + + return matches + class ProxyType(StrEnum): """Supported proxy transport flavours. @@ -49,14 +113,17 @@ class ProxyType(StrEnum): @dataclass(frozen=True, slots=True) class ProxyOptions: - """Immutable proxy configuration with pre-compiled bypass globs. + """Immutable proxy configuration with pre-compiled bypass matchers. Attributes: type: Proxy transport flavour (HTTP / SOCKS4 / SOCKS5). host: Proxy host. Must be non-empty. port: Proxy port in the range ``0..65535``. - non_proxy_hosts: Glob patterns (``fnmatch`` syntax) that bypass the - proxy. Compiled once in ``__post_init__``. + non_proxy_hosts: Bypass entries. A bare entry (``example.com`` or + ``.example.com``) matches the host and its subdomains by suffix; + an entry with a glob metacharacter (``*.example.com``) keeps + ``fnmatch`` semantics. A trailing ``:port`` on a bare entry is + ignored. Compiled once in ``__post_init__``. username: Optional username for proxy auth. Masked in ``repr``. password: Optional password for proxy auth. Masked in ``repr``. """ @@ -67,15 +134,15 @@ class ProxyOptions: non_proxy_hosts: tuple[str, ...] = () username: str | None = None password: str | None = None - # Compiled bypass globs. Excluded from ``repr`` / equality / hashing so + # Compiled bypass matchers. Excluded from ``repr`` / equality / hashing so # two ``ProxyOptions`` with the same logical fields compare equal even - # though their compiled patterns are distinct objects. - _bypass_patterns: tuple[re.Pattern[str], ...] = field( + # though their compiled predicates are distinct objects. + _bypass_matchers: tuple[Callable[[str], bool], ...] = field( init=False, repr=False, compare=False, hash=False ) def __post_init__(self) -> None: - """Validate inputs and pre-compile bypass globs. + """Validate inputs and pre-compile bypass matchers. Raises: ValueError: If ``host`` is empty or ``port`` is outside 0..65535. @@ -84,26 +151,28 @@ def __post_init__(self) -> None: raise ValueError("host must not be empty") if not (0 <= self.port <= 65535): raise ValueError(f"port must be in 0..65535, got {self.port}") - compiled = tuple( - re.compile(fnmatch.translate(pattern), re.IGNORECASE) - for pattern in self.non_proxy_hosts - ) - object.__setattr__(self, "_bypass_patterns", compiled) + compiled = tuple(_compile_bypass(pattern) for pattern in self.non_proxy_hosts) + object.__setattr__(self, "_bypass_matchers", compiled) def bypasses_proxy(self, host: str) -> bool: - """Return ``True`` when ``host`` matches any bypass glob. + """Return ``True`` when ``host`` matches any bypass entry. - Matching is case-insensitive — hostnames in the wire are - case-insensitive per RFC 3986. + Matching is case-insensitive — hostnames on the wire are + case-insensitive per RFC 3986. Bare entries use suffix semantics + (``example.com`` bypasses ``api.example.com``); glob entries use + ``fnmatch``. A trailing ``:port`` on the candidate is stripped before + matching, so port-qualified hosts compare on their host part alone. Args: - host: Candidate hostname (no scheme, no port). + host: Candidate hostname, with an optional ``:port`` suffix + (stripped) and optional IPv6 brackets. No scheme. Returns: - ``True`` if at least one bypass pattern matches; ``False`` - otherwise (including when there are no bypass patterns). + ``True`` if at least one bypass entry matches; ``False`` + otherwise (including when there are no bypass entries). """ - return any(pattern.match(host) for pattern in self._bypass_patterns) + candidate = _strip_port(host) + return any(matcher(candidate) for matcher in self._bypass_matchers) def __repr__(self) -> str: """Render the proxy options with credentials masked. diff --git a/packages/dexpace-sdk-core/tests/auth/test_challenge_immutability.py b/packages/dexpace-sdk-core/tests/auth/test_challenge_immutability.py new file mode 100644 index 0000000..bae8169 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/auth/test_challenge_immutability.py @@ -0,0 +1,37 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Tests for the read-only parameter view on parsed challenges. + +``AuthenticateChallenge`` is a frozen dataclass; its ``parameters`` mapping +produced by ``parse_challenges`` must likewise be immutable so the object is +fully read-only, not a frozen shell wrapping a mutable dict. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from types import MappingProxyType + +import pytest + +from dexpace.sdk.core.http.auth import parse_challenges + + +class TestParameterImmutability: + def test_parameters_is_a_mapping(self) -> None: + challenge = parse_challenges('Digest realm="r", qop="auth"')[0] + assert isinstance(challenge.parameters, Mapping) + assert challenge.parameters["realm"] == "r" + assert challenge.parameters["qop"] == "auth" + + def test_parameters_view_rejects_mutation(self) -> None: + challenge = parse_challenges('Digest realm="r"')[0] + assert isinstance(challenge.parameters, MappingProxyType) + with pytest.raises(TypeError): + challenge.parameters["realm"] = "tampered" # type: ignore[index] + + def test_multiple_challenges_have_independent_params(self) -> None: + first, second = parse_challenges('Basic realm="a", Digest realm="b", qop="auth"') + assert first.parameters == {"realm": "a"} + assert second.parameters == {"realm": "b", "qop": "auth"} diff --git a/packages/dexpace-sdk-core/tests/auth/test_digest_edge_cases.py b/packages/dexpace-sdk-core/tests/auth/test_digest_edge_cases.py new file mode 100644 index 0000000..5b91975 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/auth/test_digest_edge_cases.py @@ -0,0 +1,190 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Edge-case tests for the Digest challenge handler. + +Covers the decline-on-unencodable-credentials path, the RFC 2069 (qop-less) +header shape, and the preference tuple acting as an algorithm allow-list. +""" + +from __future__ import annotations + +import re + +from dexpace.sdk.core.http.auth import ( + AuthenticateChallenge, + DigestChallengeHandler, +) +from dexpace.sdk.core.http.common.url import Url +from dexpace.sdk.core.http.request.method import Method + +_USERNAME = "Mufasa" +_PASSWORD = "Circle of Life" +_REALM = "http-auth@example.org" +_NONCE = "7ypf/xlj9XXwfDPEoM4URrv/xwf94BcCAzFZH4GiTo0v" +_CNONCE = "f2/wE4q74E6zIJEtWaHKaf5wv/H5QzzpXusqGemxURZJ" +_URL = Url(scheme="https", host="example.org", path="/dir/index.html") + + +def _parse_auth(value: str) -> dict[str, str]: + assert value.startswith("Digest ") + body = value[len("Digest ") :] + out: dict[str, str] = {} + parts = re.split(r",\s*(?=[a-zA-Z][a-zA-Z0-9_-]*=)", body) + for part in parts: + key, _, raw = part.partition("=") + raw = raw.strip() + if raw.startswith('"') and raw.endswith('"'): + raw = raw[1:-1] + out[key.strip().lower()] = raw + return out + + +class TestUnencodableCredentials: + def test_non_latin1_creds_against_charsetless_challenge_declines(self) -> None: + # No ``charset`` directive -> ISO-8859-1 default, which cannot encode + # a CJK password. ``handle`` must decline (return None), not raise. + handler = DigestChallengeHandler( + "ユーザー", + "パスワード", + preferred_algorithms=("MD5",), + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "qop": "auth", "nonce": _NONCE, "algorithm": "MD5"}, + ) + assert handler.handle(Method.GET, _URL, [challenge], is_proxy=False) is None + + def test_non_latin1_creds_succeed_when_utf8_advertised(self) -> None: + # With ``charset=UTF-8`` the same credentials encode fine. + handler = DigestChallengeHandler( + "ユーザー", + "パスワード", + preferred_algorithms=("MD5",), + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={ + "realm": _REALM, + "qop": "auth", + "nonce": _NONCE, + "algorithm": "MD5", + "charset": "UTF-8", + }, + ) + result = handler.handle(Method.GET, _URL, [challenge], is_proxy=False) + assert result is not None + + +class TestQopLessHeader: + def test_qopless_header_omits_cnonce_and_nc(self) -> None: + # RFC 2069: server omits ``qop``. The response header must not carry + # ``cnonce`` or ``nc`` per RFC 7616 §3.4. + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + preferred_algorithms=("MD5",), + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "nonce": _NONCE, "algorithm": "MD5"}, + ) + result = handler.handle(Method.GET, _URL, [challenge], is_proxy=False) + assert result is not None + _, value = result + params = _parse_auth(value) + assert "cnonce" not in params + assert "nc" not in params + assert "qop" not in params + # The RFC 2069 response digest is still emitted. + assert "response" in params + + def test_qop_present_still_emits_cnonce_and_nc(self) -> None: + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + preferred_algorithms=("MD5",), + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "qop": "auth", "nonce": _NONCE, "algorithm": "MD5"}, + ) + result = handler.handle(Method.GET, _URL, [challenge], is_proxy=False) + assert result is not None + params = _parse_auth(result[1]) + assert params["cnonce"] == _CNONCE + assert params["nc"] == "00000001" + assert params["qop"] == "auth" + + +class TestSessionVariantQopLess: + def test_sess_algorithm_without_qop_declines(self) -> None: + # A ``*-sess`` algorithm folds ``cnonce`` into HA1, but a qop-less + # response omits ``cnonce``/``nc`` — leaving the server unable to + # reconstruct HA1. The handler declines rather than emit an + # unverifiable header. + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "nonce": _NONCE, "algorithm": "SHA-256-sess"}, + ) + assert handler.handle(Method.GET, _URL, [challenge], is_proxy=False) is None + + def test_sess_algorithm_with_qop_still_works(self) -> None: + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + cnonce_factory=lambda: _CNONCE, + ) + challenge = AuthenticateChallenge( + scheme="Digest", + parameters={ + "realm": _REALM, + "qop": "auth", + "nonce": _NONCE, + "algorithm": "SHA-256-sess", + }, + ) + result = handler.handle(Method.GET, _URL, [challenge], is_proxy=False) + assert result is not None + params = _parse_auth(result[1]) + assert params["cnonce"] == _CNONCE + assert params["algorithm"] == "SHA-256-sess" + + +class TestPreferenceAllowList: + def test_sha256_preference_declines_md5_only_server(self) -> None: + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + preferred_algorithms=("SHA-256",), + cnonce_factory=lambda: _CNONCE, + ) + md5_only = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "qop": "auth", "nonce": _NONCE, "algorithm": "MD5"}, + ) + assert handler.handle(Method.GET, _URL, [md5_only], is_proxy=False) is None + assert handler.can_handle([md5_only]) is False + + def test_default_preference_still_reaches_md5(self) -> None: + handler = DigestChallengeHandler( + _USERNAME, + _PASSWORD, + cnonce_factory=lambda: _CNONCE, + ) + md5_only = AuthenticateChallenge( + scheme="Digest", + parameters={"realm": _REALM, "qop": "auth", "nonce": _NONCE, "algorithm": "MD5"}, + ) + result = handler.handle(Method.GET, _URL, [md5_only], is_proxy=False) + assert result is not None + assert _parse_auth(result[1])["algorithm"] == "MD5" diff --git a/packages/dexpace-sdk-core/tests/auth/test_policies.py b/packages/dexpace-sdk-core/tests/auth/test_policies.py index 8038472..5cc85a5 100644 --- a/packages/dexpace-sdk-core/tests/auth/test_policies.py +++ b/packages/dexpace-sdk-core/tests/auth/test_policies.py @@ -8,6 +8,7 @@ import asyncio import threading import time +from collections.abc import AsyncIterator, Iterator import pytest @@ -20,6 +21,7 @@ AuthenticateChallenge, BasicAuthCredential, BasicAuthPolicy, + BasicChallengeHandler, BearerTokenPolicy, DigestChallengeHandler, KeyCredential, @@ -29,6 +31,8 @@ from dexpace.sdk.core.http.context import DispatchContext from dexpace.sdk.core.http.request import Method, Request from dexpace.sdk.core.http.response import AsyncResponse, Response, Status +from dexpace.sdk.core.http.response.async_response_body import AsyncResponseBody +from dexpace.sdk.core.http.response.response_body import ResponseBody from dexpace.sdk.core.instrumentation import ( InstrumentationContext, SpanId, @@ -283,16 +287,28 @@ async def close(self) -> None: class _CapturingAsyncClient(AsyncHttpClient): """Async twin of ``_CapturingClient``.""" - def __init__(self, *, status: Status = Status.OK) -> None: + def __init__( + self, + *, + status: Status = Status.OK, + www_auth: bool = False, + ) -> None: self.status = status + self.www_auth = www_auth self.calls: list[Request] = [] async def execute(self, request: Request) -> AsyncResponse: + from dexpace.sdk.core.http.common import Headers + self.calls.append(request) + header_pairs: list[tuple[str, str]] = [] + if self.www_auth and self.status is Status.UNAUTHORIZED: + header_pairs.append(("WWW-Authenticate", 'Bearer realm="api"')) return AsyncResponse( request=request, protocol=Protocol.HTTP_1_1, status=self.status, + headers=Headers(header_pairs), ) @@ -421,3 +437,365 @@ def on_challenge(self, request: Request, response: Response) -> bool: assert handler.can_handle_calls == 1 assert handler.handle_calls == 0 assert on_challenge_calls == 1 + + +class _StaticAsyncCredential: + """Minimal AsyncTokenCredential — returns the same token unless told otherwise.""" + + def __init__( + self, + token: str = "abc", + expires_in: int = 3600, + clock: FakeClock | None = None, + ) -> None: + self.calls = 0 + self.token = token + self.expires_in = expires_in + self._clock = clock + + async def get_token_info( + self, + *scopes: str, + options: object = None, + ) -> AccessTokenInfo: + del scopes, options + self.calls += 1 + now = self._clock.now() if self._clock is not None else time.time() + return AccessTokenInfo( + token=self.token, + expires_on=int(now) + self.expires_in, + ) + + async def close(self) -> None: + return None + + +class _ScriptedAsyncClient(AsyncHttpClient): + """Async twin of ``_ScriptedClient``. + + Each call consumes the next ``(status, header_name, header_value)`` entry + from the script. The final entry is reused if the script runs dry. + """ + + def __init__(self, script: list[tuple[Status, str | None, str | None]]) -> None: + if not script: + raise ValueError("script must not be empty") + self._script = script + self.calls: list[Request] = [] + self._index = 0 + + async def execute(self, request: Request) -> AsyncResponse: + from dexpace.sdk.core.http.common import Headers + + self.calls.append(request) + idx = min(self._index, len(self._script) - 1) + self._index += 1 + status, header_name, header_value = self._script[idx] + header_pairs: list[tuple[str, str]] = [] + if header_name is not None and header_value is not None: + header_pairs.append((header_name, header_value)) + return AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=status, + headers=Headers(header_pairs), + ) + + +async def test_async_challenge_handler_wires_digest_authentication() -> None: + """Digest handler plugged into AsyncBearerTokenPolicy negotiates a 401 Digest.""" + + digest_challenge = ( + 'Digest realm="testrealm@host.com", ' + 'qop="auth", ' + 'nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", ' + 'opaque="5ccc069c403ebaf9f0171e9517f40e41", ' + "algorithm=MD5" + ) + client = _ScriptedAsyncClient( + [ + (Status.UNAUTHORIZED, "WWW-Authenticate", digest_challenge), + (Status.OK, None, None), + ] + ) + cred = _StaticAsyncCredential() + handler = DigestChallengeHandler( + "Mufasa", + "Circle Of Life", + cnonce_factory=lambda: "0a4f113b", + ) + policy = AsyncBearerTokenPolicy(cred, "scope-a", challenge_handler=handler) + async with AsyncPipeline(client, policies=[policy]) as p: + await p.run(_request(), DispatchContext(_instr("0" * 16 + "e"))) + + assert len(client.calls) == 2 + auth = client.calls[1].headers.get("authorization") + assert auth is not None + assert auth.startswith("Digest ") + assert 'username="Mufasa"' in auth + assert 'realm="testrealm@host.com"' in auth + assert "algorithm=MD5" in auth + + +async def test_async_challenge_handler_handles_407_proxy() -> None: + """A 407 with Proxy-Authenticate is satisfied by a Basic handler.""" + + client = _ScriptedAsyncClient( + [ + (Status.PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", 'Basic realm="proxy"'), + (Status.OK, None, None), + ] + ) + cred = _StaticAsyncCredential() + handler = BasicChallengeHandler("user", "pass") + policy = AsyncBearerTokenPolicy(cred, "scope-a", challenge_handler=handler) + async with AsyncPipeline(client, policies=[policy]) as p: + await p.run(_request(), DispatchContext(_instr("0" * 15 + "10"))) + + assert len(client.calls) == 2 + assert client.calls[1].headers.get("proxy-authorization") == "Basic dXNlcjpwYXNz" + + +async def test_async_407_without_handler_surfaces_response() -> None: + """A 407 with no challenge_handler returns the proxy response unchanged.""" + + client = _ScriptedAsyncClient( + [(Status.PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", 'Basic realm="proxy"')] + ) + cred = _StaticAsyncCredential() + policy = AsyncBearerTokenPolicy(cred, "scope-a") + async with AsyncPipeline(client, policies=[policy]) as p: + response = await p.run(_request(), DispatchContext(_instr("0" * 15 + "11"))) + + assert int(response.status) == 407 + assert len(client.calls) == 1 + + +async def test_async_407_does_not_invalidate_cached_token() -> None: + """A 407 leaves the cached origin token untouched (only 401 invalidates).""" + + client = _ScriptedAsyncClient( + [ + (Status.PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", 'Basic realm="proxy"'), + (Status.OK, None, None), + ] + ) + cred = _StaticAsyncCredential() + handler = BasicChallengeHandler("user", "pass") + policy = AsyncBearerTokenPolicy(cred, "scope-a", challenge_handler=handler) + async with AsyncPipeline(client, policies=[policy]) as p: + await p.run(_request(), DispatchContext(_instr("0" * 15 + "12"))) + await p.run(_request(), DispatchContext(_instr("0" * 15 + "13"))) + + # Token fetched once; the 407 must not have evicted it. + assert cred.calls == 1 + + +async def test_async_challenge_handler_can_handle_false_falls_through() -> None: + """Async handler that declines the challenge falls back to on_challenge.""" + + class _NoopHandler: + def __init__(self) -> None: + self.can_handle_calls = 0 + self.handle_calls = 0 + + def can_handle(self, challenges: list[AuthenticateChallenge]) -> bool: + del challenges + self.can_handle_calls += 1 + return False + + def handle( + self, + method: Method, + url: Url, + challenges: list[AuthenticateChallenge], + *, + is_proxy: bool, + ) -> tuple[str, str] | None: + del method, url, challenges, is_proxy + self.handle_calls += 1 + return None + + client = _CapturingAsyncClient(status=Status.UNAUTHORIZED, www_auth=True) + cred = _StaticAsyncCredential() + handler = _NoopHandler() + + on_challenge_calls = 0 + + class _Spy(AsyncBearerTokenPolicy): + async def on_challenge(self, request: Request, response: AsyncResponse) -> bool: + nonlocal on_challenge_calls + on_challenge_calls += 1 + return False + + policy = _Spy(cred, "scope-a", challenge_handler=handler) + async with AsyncPipeline(client, policies=[policy]) as p: + with pytest.raises(ClientAuthenticationError): + await p.run(_request(), DispatchContext(_instr("0" * 15 + "14"))) + + assert handler.can_handle_calls == 1 + assert handler.handle_calls == 0 + assert on_challenge_calls == 1 + + +# --------------------------------------------------------------------------- # +# A discarded 401/407 response is closed before the request is re-issued # +# --------------------------------------------------------------------------- # + +_DIGEST_CHALLENGE = ( + 'Digest realm="testrealm@host.com", ' + 'qop="auth", ' + 'nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", ' + 'opaque="5ccc069c403ebaf9f0171e9517f40e41", ' + "algorithm=MD5" +) + + +class _RecordingResponseBody(ResponseBody): + """Sync response body that records whether ``close`` was called.""" + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + def iter_bytes(self, chunk_size: int = 64 * 1024) -> Iterator[bytes]: + yield b"" + + def close(self) -> None: + self.closed = True + + +class _AsyncRecordingResponseBody(AsyncResponseBody): + """Async response body that records whether ``close`` was awaited.""" + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + yield b"" + + async def close(self) -> None: + self.closed = True + + +class _BodyScriptedClient(HttpClient): + """Scripted client whose responses each carry an inspectable recording body.""" + + def __init__(self, script: list[tuple[Status, str | None]]) -> None: + self._script = script + self._index = 0 + self.bodies: list[_RecordingResponseBody] = [] + + def execute(self, request: Request) -> Response: + from dexpace.sdk.core.http.common import Headers + + idx = min(self._index, len(self._script) - 1) + self._index += 1 + status, www_auth = self._script[idx] + pairs = [("WWW-Authenticate", www_auth)] if www_auth is not None else [] + body = _RecordingResponseBody() + self.bodies.append(body) + return Response( + request=request, + protocol=Protocol.HTTP_1_1, + status=status, + headers=Headers(pairs), + body=body, + ) + + +class _AsyncBodyScriptedClient(AsyncHttpClient): + """Async twin of ``_BodyScriptedClient``.""" + + def __init__(self, script: list[tuple[Status, str | None]]) -> None: + self._script = script + self._index = 0 + self.bodies: list[_AsyncRecordingResponseBody] = [] + + async def execute(self, request: Request) -> AsyncResponse: + from dexpace.sdk.core.http.common import Headers + + idx = min(self._index, len(self._script) - 1) + self._index += 1 + status, www_auth = self._script[idx] + pairs = [("WWW-Authenticate", www_auth)] if www_auth is not None else [] + body = _AsyncRecordingResponseBody() + self.bodies.append(body) + return AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=status, + headers=Headers(pairs), + body=body, + ) + + +def test_challenge_handler_closes_discarded_401_response() -> None: + """The 401 whose body the handler retries past is released before re-issue.""" + client = _BodyScriptedClient([(Status.UNAUTHORIZED, _DIGEST_CHALLENGE), (Status.OK, None)]) + handler = DigestChallengeHandler("Mufasa", "Circle Of Life", cnonce_factory=lambda: "0a4f113b") + policy = BearerTokenPolicy(_StaticCredential(), "scope-a", challenge_handler=handler) + with Pipeline(client, policies=[policy]) as p: + p.run(_request(), DispatchContext(_instr("0" * 15 + "2a"))) + + assert client.bodies[0].closed, "discarded 401 response leaked: body was not closed" + assert not client.bodies[1].closed, "final response handed to caller must stay open" + + +def test_on_challenge_closes_discarded_401_response() -> None: + """The on_challenge retry path also releases the rejected 401.""" + client = _BodyScriptedClient([(Status.UNAUTHORIZED, 'Bearer realm="x"'), (Status.OK, None)]) + + class _Retrying(BearerTokenPolicy): + def on_challenge(self, request: Request, response: Response) -> bool: + del request, response + return True + + policy = _Retrying(_StaticCredential(), "scope-a") + with Pipeline(client, policies=[policy]) as p: + p.run(_request(), DispatchContext(_instr("0" * 15 + "2b"))) + + assert client.bodies[0].closed, "discarded 401 response leaked: body was not closed" + assert not client.bodies[1].closed, "final response handed to caller must stay open" + + +async def test_async_challenge_handler_closes_discarded_401_response() -> None: + """Async handler retry releases the rejected 401 before re-issue.""" + client = _AsyncBodyScriptedClient([(Status.UNAUTHORIZED, _DIGEST_CHALLENGE), (Status.OK, None)]) + handler = DigestChallengeHandler("Mufasa", "Circle Of Life", cnonce_factory=lambda: "0a4f113b") + policy = AsyncBearerTokenPolicy(_StaticAsyncCredential(), "scope-a", challenge_handler=handler) + async with AsyncPipeline(client, policies=[policy]) as p: + await p.run(_request(), DispatchContext(_instr("0" * 15 + "2c"))) + + assert client.bodies[0].closed, "discarded 401 response leaked: body was not closed" + assert not client.bodies[1].closed, "final response handed to caller must stay open" + + +async def test_async_on_challenge_closes_discarded_401_response() -> None: + """Async on_challenge retry releases the rejected 401.""" + client = _AsyncBodyScriptedClient( + [(Status.UNAUTHORIZED, 'Bearer realm="x"'), (Status.OK, None)] + ) + + class _Retrying(AsyncBearerTokenPolicy): + async def on_challenge(self, request: Request, response: AsyncResponse) -> bool: + del request, response + return True + + policy = _Retrying(_StaticAsyncCredential(), "scope-a") + async with AsyncPipeline(client, policies=[policy]) as p: + await p.run(_request(), DispatchContext(_instr("0" * 15 + "2d"))) + + assert client.bodies[0].closed, "discarded 401 response leaked: body was not closed" + assert not client.bodies[1].closed, "final response handed to caller must stay open" diff --git a/packages/dexpace-sdk-core/tests/config/test_configuration.py b/packages/dexpace-sdk-core/tests/config/test_configuration.py index d641570..14c08c1 100644 --- a/packages/dexpace-sdk-core/tests/config/test_configuration.py +++ b/packages/dexpace-sdk-core/tests/config/test_configuration.py @@ -155,6 +155,14 @@ def test_get_duration_negative_returns_default() -> None: assert cfg.get_duration("T", 9.0) == 9.0 +@pytest.mark.parametrize("value", ["inf", "-inf", "Infinity", "nan", "NaN"]) +def test_get_duration_non_finite_returns_default(value: str) -> None: + # ``float('inf')`` / ``float('nan')`` parse successfully but a non-finite + # duration disables deadline enforcement; reject them in favour of the default. + cfg = Configuration(env=_env({"T": value})) + assert cfg.get_duration("T", 9.0) == 9.0 + + # --------------------------------------------------------------------------- # Builder diff --git a/packages/dexpace-sdk-core/tests/conftest.py b/packages/dexpace-sdk-core/tests/conftest.py index cbf603c..718e7c0 100644 --- a/packages/dexpace-sdk-core/tests/conftest.py +++ b/packages/dexpace-sdk-core/tests/conftest.py @@ -29,7 +29,7 @@ def _clean_context_store() -> Iterator[None]: class FakeClock: - """Deterministic :class:`~dexpace.sdk.core.util.Clock` for unit tests. + """Deterministic `Clock` for unit tests. Wall-clock (``now``) and monotonic readings are tracked independently so tests can model the real divergence between them (system clock jumps vs @@ -72,5 +72,5 @@ def advance(self, duration: float) -> None: @pytest.fixture def fake_clock() -> FakeClock: - """Provide a fresh :class:`FakeClock` starting at ``t=0``.""" + """Provide a fresh `FakeClock` starting at ``t=0``.""" return FakeClock() diff --git a/packages/dexpace-sdk-core/tests/errors/test_hierarchy.py b/packages/dexpace-sdk-core/tests/errors/test_hierarchy.py index 1076be0..18ffff4 100644 --- a/packages/dexpace-sdk-core/tests/errors/test_hierarchy.py +++ b/packages/dexpace-sdk-core/tests/errors/test_hierarchy.py @@ -55,6 +55,23 @@ def test_sdk_error_captures_sys_exc_info() -> None: assert "inner" in str(err.exc_value) +def test_sdk_error_traceback_falls_back_to_cause() -> None: + # Capture a caught exception, then construct the SdkError *outside* its + # except block so ``sys.exc_info()`` is the empty (None, None, None) + # triple. The traceback must still fall back to the cause's own + # ``__traceback__`` rather than leaving an incoherent (type, value, None). + cause: RuntimeError + try: + raise RuntimeError("inner") + except RuntimeError as exc: + cause = exc + assert cause.__traceback__ is not None + err = SdkError("outer", error=cause) + assert err.exc_type is RuntimeError + assert err.exc_value is cause + assert err.exc_traceback is cause.__traceback__ + + def test_continuation_token_propagates() -> None: err = SdkError("paging", continuation_token="next-page-3") assert err.continuation_token == "next-page-3" diff --git a/packages/dexpace-sdk-core/tests/http/test_async_bodies.py b/packages/dexpace-sdk-core/tests/http/test_async_bodies.py index 41870d1..f952668 100644 --- a/packages/dexpace-sdk-core/tests/http/test_async_bodies.py +++ b/packages/dexpace-sdk-core/tests/http/test_async_bodies.py @@ -40,6 +40,17 @@ async def test_from_form() -> None: assert body.media_type() == common_media_types.APPLICATION_FORM_URLENCODED +async def test_from_form_encoding_changes_percent_encoding() -> None: + # Async twin of the sync charset test: a non-ASCII field must percent-encode + # through the requested charset, so latin-1 and utf-8 differ byte-for-byte. + fields = {"name": "é"} + latin1 = await _adrain(AsyncRequestBody.from_form(fields, encoding="latin-1")) + utf8 = await _adrain(AsyncRequestBody.from_form(fields, encoding="utf-8")) + assert latin1 == b"name=%E9" + assert utf8 == b"name=%C3%A9" + assert latin1 != utf8 + + async def test_from_async_iter_single_use() -> None: async def chunks() -> AsyncIterator[bytes]: yield b"ab" @@ -111,3 +122,29 @@ async def test_aiter_bytes_rejects_invalid_chunk_size( with pytest.raises(ValueError, match="chunk_size"): async for _ in body.aiter_bytes(size): pass + + +async def _one_chunk() -> AsyncIterator[bytes]: + yield b"hi" + + +def _make_async_request_bodies() -> list[Callable[[], AsyncRequestBody]]: + return [ + lambda: AsyncRequestBody.from_bytes(b"hi"), + lambda: AsyncRequestBody.from_async_iter(_one_chunk()), + lambda: AsyncRequestBody.from_async_stream(_StubAsyncStream()), + ] + + +@pytest.mark.parametrize("factory", _make_async_request_bodies()) +@pytest.mark.parametrize("size", [0, -1]) +async def test_async_request_aiter_bytes_rejects_invalid_chunk_size( + factory: Callable[[], AsyncRequestBody], + size: int, +) -> None: + # All three AsyncRequestBody backings must reject a non-positive chunk_size + # up front, matching the sync and response-body guard. + body = factory() + with pytest.raises(ValueError, match="chunk_size"): + async for _ in body.aiter_bytes(size): + pass diff --git a/packages/dexpace-sdk-core/tests/http/test_async_cancellation.py b/packages/dexpace-sdk-core/tests/http/test_async_cancellation.py index ecf1353..12a0f27 100644 --- a/packages/dexpace-sdk-core/tests/http/test_async_cancellation.py +++ b/packages/dexpace-sdk-core/tests/http/test_async_cancellation.py @@ -18,6 +18,7 @@ import pytest +from dexpace.sdk.core.http.request import AsyncRequestBody from dexpace.sdk.core.http.response import AsyncResponse, AsyncResponseBody from dexpace.sdk.core.http.response.async_response_body import _shielded_cleanup from dexpace.sdk.core.http.sse.parser import parse_async_events @@ -69,6 +70,27 @@ async def consume() -> None: assert stream.close_completed is True +async def test_request_aiter_bytes_releases_stream_and_propagates_when_cancelled() -> None: + # The request-side stream body must shield its close just like the + # response side, so a cancel mid-read still releases the upstream stream. + stream = _SlowStream() + body = AsyncRequestBody.from_async_stream(stream) + + async def produce() -> None: + async for _ in body.aiter_bytes(): + pass + + task = asyncio.ensure_future(produce()) + await asyncio.sleep(0) # let the task reach the blocking read + task.cancel() + + with pytest.raises(asyncio.CancelledError): + await task + + assert stream.closed is True + assert stream.close_completed is True + + async def test_response_aexit_releases_body_when_cancelled() -> None: stream = _SlowStream() body = AsyncResponseBody.from_async_stream(stream) diff --git a/packages/dexpace-sdk-core/tests/http/test_etag.py b/packages/dexpace-sdk-core/tests/http/test_etag.py index 85e53b3..7b991d7 100644 --- a/packages/dexpace-sdk-core/tests/http/test_etag.py +++ b/packages/dexpace-sdk-core/tests/http/test_etag.py @@ -39,6 +39,25 @@ def test_parse_unquoted_raises() -> None: ETag.parse("no-quotes") +def test_parse_embedded_quote_raises() -> None: + # RFC 7232 forbids a DQUOTE inside the opaque tag; accepting it would + # re-emit a malformed wire form via __str__. + with pytest.raises(ValueError): + ETag.parse('"a"b"') + + +def test_parse_embedded_control_char_raises() -> None: + with pytest.raises(ValueError): + ETag.parse('"a\x01b"') + + +def test_parse_embedded_space_raises() -> None: + # RFC 7232 §2.3: etagc = %x21 / %x23-7E / obs-text, so SP (0x20) is not a + # valid entity-tag character. + with pytest.raises(ValueError): + ETag.parse('"a b"') + + def test_parse_empty_strong_etag_raises() -> None: with pytest.raises(ValueError, match="empty"): ETag.parse('""') diff --git a/packages/dexpace-sdk-core/tests/http/test_headers_injection.py b/packages/dexpace-sdk-core/tests/http/test_headers_injection.py new file mode 100644 index 0000000..a6b54d9 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/http/test_headers_injection.py @@ -0,0 +1,73 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Tests for header-value injection guards on mutators and hand-built names.""" + +from __future__ import annotations + +import pytest + +from dexpace.sdk.core.http.common import Headers, HttpHeaderName + +_BAD_VALUES = ["v\r\nLoc: bad", "v\nbad", "v\0bad"] + + +class TestMutatorValueValidation: + @pytest.mark.parametrize("value", _BAD_VALUES) + def test_with_added_rejects_control_chars(self, value: str) -> None: + h = Headers() + with pytest.raises(ValueError, match="invalid header value"): + h.with_added("X-Test", value) + + @pytest.mark.parametrize("value", _BAD_VALUES) + def test_with_added_rejects_control_chars_on_existing_name(self, value: str) -> None: + h = Headers([("X-Test", "ok")]) + with pytest.raises(ValueError, match="invalid header value"): + h.with_added("X-Test", value) + + @pytest.mark.parametrize("value", _BAD_VALUES) + def test_with_set_rejects_control_chars(self, value: str) -> None: + h = Headers() + with pytest.raises(ValueError, match="invalid header value"): + h.with_set("X-Test", value) + + @pytest.mark.parametrize("value", _BAD_VALUES) + def test_with_set_rejects_control_chars_in_later_value(self, value: str) -> None: + h = Headers() + with pytest.raises(ValueError, match="invalid header value"): + h.with_set("X-Test", "ok", value) + + @pytest.mark.parametrize("value", _BAD_VALUES) + def test_with_merged_rejects_control_chars(self, value: str) -> None: + base = Headers([("X-Base", "fine")]) + # Build the injected value via ``_construct`` fast-path so it bypasses + # ``__init__`` validation, mirroring how a sibling instance could carry it. + poisoned = object.__new__(Headers) + object.__setattr__(poisoned, "_data", (("x-test", (value,)),)) + object.__setattr__(poisoned, "_hash", None) + with pytest.raises(ValueError, match="invalid header value"): + base.with_merged(poisoned) + + def test_clean_value_round_trips_through_mutators(self) -> None: + h = Headers().with_added("X-Test", "fine").with_set("X-Other", "also-fine") + assert h.values("x-test") == ("fine",) + assert h.values("x-other") == ("also-fine",) + + +class TestHandBuiltHttpHeaderNameNormalised: + def test_non_lowercase_value_normalised_to_lower(self) -> None: + name = HttpHeaderName(value="Content-Type", canonical_name="Content-Type") + assert name.value == "content-type" + + def test_non_lowercase_name_matches_lookups(self) -> None: + name = HttpHeaderName(value="Content-Type", canonical_name="Content-Type") + h = Headers([("content-type", "application/json")]) + assert h.get(name) == "application/json" + assert name in h + assert h.values(name) == ("application/json",) + + def test_non_lowercase_name_equals_lowercase_twin(self) -> None: + upper = HttpHeaderName(value="Content-Type", canonical_name="Content-Type") + lower = HttpHeaderName(value="content-type", canonical_name="Content-Type") + assert upper == lower + assert hash(upper) == hash(lower) diff --git a/packages/dexpace-sdk-core/tests/http/test_http_range.py b/packages/dexpace-sdk-core/tests/http/test_http_range.py index 995de2e..232b1dc 100644 --- a/packages/dexpace-sdk-core/tests/http/test_http_range.py +++ b/packages/dexpace-sdk-core/tests/http/test_http_range.py @@ -27,6 +27,19 @@ def test_suffix_range() -> None: assert s.to_header_value() == "bytes=-20" +def test_suffix_range_is_public_httprange() -> None: + s = HttpRange.suffix(20) + assert isinstance(s, HttpRange) + assert s.is_suffix is True + assert s.format() == "-20" + + +def test_format_many_with_suffix_range() -> None: + # A suffix range is a single public HttpRange, so format_many accepts it. + ranges: list[HttpRange] = [HttpRange(0, 100), HttpRange.suffix(50)] + assert HttpRange.format_many(ranges) == "bytes=0-99,-50" + + def test_negative_start_raises() -> None: with pytest.raises(ValueError): HttpRange(-1, 100) diff --git a/packages/dexpace-sdk-core/tests/http/test_import_order.py b/packages/dexpace-sdk-core/tests/http/test_import_order.py new file mode 100644 index 0000000..8d5af17 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/http/test_import_order.py @@ -0,0 +1,30 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Regression test: importing the request package first must not deadlock. + +The request package re-exports ``AsyncRequestBody``, which historically pulled +in the async response body, which in turn did a runtime import of +``SupportsAsyncRead`` back from the still-initializing request module — a cycle +that broke ``from dexpace.sdk.core.http.request import Request`` as a *first* +import. The fix scopes that back-reference to ``TYPE_CHECKING`` so the request +package imports cleanly regardless of order. This test re-checks the property +in a fresh interpreter so import caching from sibling tests cannot mask a +regression. +""" + +from __future__ import annotations + +import subprocess +import sys + + +def test_request_package_imports_first_in_fresh_interpreter() -> None: + code = "from dexpace.sdk.core.http.request import Request; print(Request.__name__)" + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "Request" diff --git a/packages/dexpace-sdk-core/tests/http/test_loggable_body_fixes.py b/packages/dexpace-sdk-core/tests/http/test_loggable_body_fixes.py index b6becbe..ffdba97 100644 --- a/packages/dexpace-sdk-core/tests/http/test_loggable_body_fixes.py +++ b/packages/dexpace-sdk-core/tests/http/test_loggable_body_fixes.py @@ -122,6 +122,36 @@ def test_bounded_snapshot_caps_partial_bytes(self) -> None: assert body.snapshot(max_bytes=3) == b"abc" +class _CustomBaseError(BaseException): + """A non-``Exception`` ``BaseException`` raised mid-drain.""" + + +class TestResponseBaseExceptionPath: + def test_iter_bytes_reraises_base_exception_on_every_call(self) -> None: + # A bare ``BaseException`` (not an ``Exception``) must still be stored + # so a truncated read is never replayed as a complete success. + boom = _CustomBaseError("aborted") + inner = _FailingResponseBody([b"abc", b"def"], boom) + body = LoggableResponseBody(inner) + + with pytest.raises(_CustomBaseError) as first: + list(body.iter_bytes()) + assert first.value is boom + + # Second read must re-raise, not silently return the partial bytes. + with pytest.raises(_CustomBaseError) as second: + list(body.iter_bytes()) + assert second.value is boom + + def test_keyboard_interrupt_propagates_immediately(self) -> None: + # KeyboardInterrupt / SystemExit must escape the drain at once rather + # than being buffered and replayed by iter_bytes. + inner = _FailingResponseBody([b"abc"], KeyboardInterrupt()) + body = LoggableResponseBody(inner) + with pytest.raises(KeyboardInterrupt): + list(body.iter_bytes()) + + class TestResponseThreadSafety: def test_concurrent_first_read_drains_exactly_once(self) -> None: threads_count = 8 diff --git a/packages/dexpace-sdk-core/tests/http/test_pagination.py b/packages/dexpace-sdk-core/tests/http/test_pagination.py index 80cb6ad..bbec961 100644 --- a/packages/dexpace-sdk-core/tests/http/test_pagination.py +++ b/packages/dexpace-sdk-core/tests/http/test_pagination.py @@ -61,17 +61,28 @@ def test_flat_iteration(self) -> None: None: _Page(items=[1, 2], next_token="x"), "x": _Page(items=[3, 4], next_token=None), } - items: ItemPaged[int] = ItemPaged(_build_get_next(pages), _extract) + items: ItemPaged[int, _Page] = ItemPaged(_build_get_next(pages), _extract) assert list(items) == [1, 2, 3, 4] def test_by_page(self) -> None: pages: dict[str | None, _Page] = { None: _Page(items=[1], next_token=None), } - items: ItemPaged[int] = ItemPaged(_build_get_next(pages), _extract) + items: ItemPaged[int, _Page] = ItemPaged(_build_get_next(pages), _extract) result = [list(page) for page in items.by_page()] assert result == [[1]] + def test_by_page_with_continuation_token(self) -> None: + # Forwarding a continuation token to by_page must resume from that page + # without colliding with positional get_next/extract_data arguments. + pages: dict[str | None, _Page] = { + None: _Page(items=[1], next_token="page2"), + "page2": _Page(items=[2, 3], next_token=None), + } + items: ItemPaged[int, _Page] = ItemPaged(_build_get_next(pages), _extract) + result = [list(page) for page in items.by_page(continuation_token="page2")] + assert result == [[2, 3]] + class TestAsyncPager: async def test_iterates_pages(self) -> None: @@ -106,8 +117,26 @@ async def get_next(token: str | None) -> _Page: async def extract(page: _Page) -> tuple[str | None, Iterable[int]]: return page.next_token, page.items - items: AsyncItemPaged[int] = AsyncItemPaged(get_next, extract) + items: AsyncItemPaged[int, _Page] = AsyncItemPaged(get_next, extract) collected: list[int] = [] async for item in items: collected.append(item) assert collected == [1, 2, 3] + + async def test_by_page_with_continuation_token(self) -> None: + pages: dict[str | None, _Page] = { + None: _Page(items=[1], next_token="next"), + "next": _Page(items=[2, 3], next_token=None), + } + + async def get_next(token: str | None) -> _Page: + return pages[token] + + async def extract(page: _Page) -> tuple[str | None, Iterable[int]]: + return page.next_token, page.items + + items: AsyncItemPaged[int, _Page] = AsyncItemPaged(get_next, extract) + collected: list[list[int]] = [] + async for page in items.by_page(continuation_token="next"): + collected.append([item async for item in page]) + assert collected == [[2, 3]] diff --git a/packages/dexpace-sdk-core/tests/http/test_request_body.py b/packages/dexpace-sdk-core/tests/http/test_request_body.py index 14a60ea..02ce8e0 100644 --- a/packages/dexpace-sdk-core/tests/http/test_request_body.py +++ b/packages/dexpace-sdk-core/tests/http/test_request_body.py @@ -42,6 +42,16 @@ def test_from_form(self) -> None: assert "a=1" in text and "b=two%20words" in text assert body.media_type() == common_media_types.APPLICATION_FORM_URLENCODED + def test_from_form_encoding_changes_percent_encoding(self) -> None: + # A non-ASCII field must percent-encode through the requested charset, + # so latin-1 and utf-8 produce different bytes (one byte vs two for é). + fields = {"name": "é"} + latin1 = _drain(RequestBody.from_form(fields, encoding="latin-1")) + utf8 = _drain(RequestBody.from_form(fields, encoding="utf-8")) + assert latin1 == b"name=%E9" + assert utf8 == b"name=%C3%A9" + assert latin1 != utf8 + def test_from_stream_single_use(self) -> None: body = RequestBody.from_stream(BytesIO(b"once"), content_length=4) assert not body.is_replayable() @@ -118,6 +128,22 @@ def test_offset_and_count(self, tmp_path: Path) -> None: assert body.content_length() == 5 assert _drain(body) == b"23456" + def test_content_length_clamps_count_past_eof(self, tmp_path: Path) -> None: + # Requesting more bytes than the file holds must not over-report: + # iter_bytes stops at EOF, so content_length must match the drained size. + path = tmp_path / "short.bin" + path.write_bytes(b"0123456789") # 10 bytes + body = FileRequestBody(path, offset=4, count=1000) + drained = _drain(body) + assert drained == b"456789" # only 6 bytes available past the offset + assert body.content_length() == len(drained) == 6 + + def test_content_length_falls_back_to_count_when_stat_fails(self, tmp_path: Path) -> None: + # When stat raises (e.g. the file does not exist yet), fall back to the + # requested count rather than guessing zero. + body = FileRequestBody(tmp_path / "missing.bin", count=7) + assert body.content_length() == 7 + def test_from_file_factory(self, tmp_path: Path) -> None: path = tmp_path / "x.bin" path.write_bytes(b"abc") diff --git a/packages/dexpace-sdk-core/tests/http/test_request_conditions.py b/packages/dexpace-sdk-core/tests/http/test_request_conditions.py index ff372e1..cfcfe69 100644 --- a/packages/dexpace-sdk-core/tests/http/test_request_conditions.py +++ b/packages/dexpace-sdk-core/tests/http/test_request_conditions.py @@ -5,7 +5,7 @@ from __future__ import annotations -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta, timezone from dexpace.sdk.core.http.common import ETag, RequestConditions, Url from dexpace.sdk.core.http.request import Method, Request @@ -52,6 +52,18 @@ def test_naive_datetime_treated_as_utc() -> None: assert value is not None and "GMT" in value +def test_non_utc_offset_normalized_to_gmt() -> None: + # An aware datetime with a non-UTC offset must be normalized to UTC before + # formatting; format_datetime(usegmt=True) rejects any other offset outright. + plus_five = timezone(timedelta(hours=5)) + when = datetime(2024, 1, 1, 17, 0, 0, tzinfo=plus_five) + cond = RequestConditions(if_modified_since=when) + result = cond.apply_to(_request()) + value = result.headers.get("if-modified-since") + # 17:00 at +05:00 is 12:00 UTC. + assert value == "Mon, 01 Jan 2024 12:00:00 GMT" + + def test_apply_to_returns_new_instance() -> None: request = _request() cond = RequestConditions(if_match=[ETag(value="x")]) diff --git a/packages/dexpace-sdk-core/tests/instrumentation/test_url_redactor.py b/packages/dexpace-sdk-core/tests/instrumentation/test_url_redactor.py index dd957f5..1f6aefd 100644 --- a/packages/dexpace-sdk-core/tests/instrumentation/test_url_redactor.py +++ b/packages/dexpace-sdk-core/tests/instrumentation/test_url_redactor.py @@ -21,7 +21,10 @@ def test_allowlisted_query_unredacted() -> None: redactor = UrlRedactor() redacted = redactor.redact("https://api.example.com/v1?api-version=1.0&token=hunter2") assert "api-version=1.0" in redacted - assert "token=REDACTED" in redacted + # Non-allowlisted params collapse to a canonical REDACTED=REDACTED so the + # parameter name ("token") never leaks either. + assert "REDACTED=REDACTED" in redacted + assert "token" not in redacted assert "hunter2" not in redacted @@ -29,7 +32,9 @@ def test_accepts_parsed_url() -> None: redactor = UrlRedactor() parsed = Url.parse("https://api.example.com/v1?secret=value") out = redactor.redact(parsed) - assert "secret=REDACTED" in out + assert "REDACTED=REDACTED" in out + assert "secret" not in out + assert "value" not in out def test_unparseable_input_returns_input_unchanged() -> None: @@ -41,14 +46,26 @@ def test_custom_allowlist() -> None: redactor = UrlRedactor(allowed_query_keys={"plain"}) out = redactor.redact("https://example.com/?plain=ok&api-version=1.0") assert "plain=ok" in out - # api-version was not in the custom allow-list; should be redacted. - assert "api-version=REDACTED" in out + # api-version was not in the custom allow-list; key and value are redacted. + assert "REDACTED=REDACTED" in out + assert "api-version" not in out def test_multiple_values_per_key() -> None: redactor = UrlRedactor(allowed_query_keys=set()) out = redactor.redact("https://example.com/?a=1&a=2") - assert out.count("a=REDACTED") == 2 + assert out.count("REDACTED=REDACTED") == 2 + assert "a=" not in out + + +def test_bare_token_query_redacts_key_and_value() -> None: + # A bare token with no '=' parses as a key with an empty value. Without + # key redaction the secret would survive verbatim as the parameter name. + redactor = UrlRedactor() + token = "eyJhbGciOiJIUzI1NiJ9.payload.signature" + out = redactor.redact(f"https://api.example.com/v1?{token}") + assert token not in out + assert "REDACTED=REDACTED" in out def test_redactor_redacts_fragment_by_default() -> None: diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_async_pipeline.py b/packages/dexpace-sdk-core/tests/pipeline/test_async_pipeline.py index 3d94a6f..c28c1fa 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_async_pipeline.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_async_pipeline.py @@ -9,7 +9,7 @@ from dexpace.sdk.core.client.async_http_client import AsyncHttpClient from dexpace.sdk.core.errors import PipelineAbortedError, ServiceRequestError -from dexpace.sdk.core.http.common import Protocol, Url +from dexpace.sdk.core.http.common import Headers, Protocol, Url from dexpace.sdk.core.http.context import CallContext, DispatchContext from dexpace.sdk.core.http.request import Method, Request from dexpace.sdk.core.http.request.request_body import RequestBody @@ -62,7 +62,7 @@ async def execute(self, request: Request) -> AsyncResponse: class _AsyncFakeClock: - """Deterministic ``AsyncClock`` mirror of :class:`FakeClock`. + """Deterministic ``AsyncClock`` mirror of `FakeClock`. Advances an internal counter on ``sleep`` (clamped at zero) without yielding to the event loop or accruing real wall time. Used by every @@ -180,6 +180,36 @@ async def execute(self, request: Request) -> AsyncResponse: assert client.attempts == 2 +async def test_async_http_date_retry_after_uses_injected_clock() -> None: + # Parity with the sync test_http_date_uses_injected_clock_not_wall_clock: + # an HTTP-date ``Retry-After`` must be measured against the injected async + # clock, not real wall time. The fake clock starts 30s before the header's + # instant, so the policy sleeps exactly 30s and the clock lands on it. + epoch_2000 = 946_684_800.0 + + class _DatedClient(AsyncHttpClient): + def __init__(self) -> None: + self._statuses = iter([Status.SERVICE_UNAVAILABLE, Status.OK]) + + async def execute(self, request: Request) -> AsyncResponse: + return AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=next(self._statuses), + headers=Headers([("Retry-After", "Sat, 01 Jan 2000 00:00:00 GMT")]), + ) + + clock = _AsyncFakeClock(start=epoch_2000 - 30.0) + retry = AsyncRetryPolicy(clock=clock) + async with AsyncPipeline(_DatedClient(), policies=[retry]) as p: + response = await p.run(_request(), DispatchContext(_instr("0" * 16 + "d1"))) + assert response.is_success + # Absolute tolerance: without the injected clock the wall-clock-based delay + # would be ~0 and the clock would stay 30s short, which a relative approx + # (~946s band at this magnitude) would wrongly accept. + assert clock.monotonic() == pytest.approx(epoch_2000, abs=1.0) + + async def test_async_retry_with_single_use_body_auto_replays() -> None: consumed: list[bytes] = [] diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_async_redirect.py b/packages/dexpace-sdk-core/tests/pipeline/test_async_redirect.py index aae5652..c1685d2 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_async_redirect.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_async_redirect.py @@ -5,7 +5,7 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import AsyncIterator, Sequence import pytest @@ -14,7 +14,7 @@ from dexpace.sdk.core.http.context import DispatchContext from dexpace.sdk.core.http.request import Method, Request from dexpace.sdk.core.http.request.request_body import RequestBody -from dexpace.sdk.core.http.response import AsyncResponse, Status +from dexpace.sdk.core.http.response import AsyncResponse, AsyncResponseBody, Status from dexpace.sdk.core.instrumentation import ( InstrumentationContext, SpanId, @@ -52,18 +52,43 @@ def _request( return req +class _TrackingAsyncBody(AsyncResponseBody): + """Async response body that records whether ``close`` was called.""" + + __slots__ = ("closed",) + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + await self.close() + return + yield b"" # pragma: no cover - makes this an async generator + + async def close(self) -> None: + self.closed = True + + class _Hop: - __slots__ = ("extra_headers", "location", "status") + __slots__ = ("body", "extra_headers", "location", "status") def __init__( self, status: Status, location: str | None = None, extra_headers: tuple[tuple[str, str], ...] = (), + body: AsyncResponseBody | None = None, ) -> None: self.status = status self.location = location self.extra_headers = extra_headers + self.body = body class _ScriptedAsyncClient(AsyncHttpClient): @@ -77,7 +102,12 @@ async def execute(self, request: Request) -> AsyncResponse: idx = len(self.requests) self.requests.append(request) hop = self._hops[idx] - response = AsyncResponse(request=request, protocol=Protocol.HTTP_1_1, status=hop.status) + response = AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=hop.status, + body=hop.body, + ) if hop.location is not None: response = response.with_header("Location", hop.location) for name, value in hop.extra_headers: @@ -175,6 +205,28 @@ async def test_307_with_non_replayable_body_raises(self) -> None: with pytest.raises(RuntimeError): await _run(client, policy, request) + async def test_307_non_replayable_body_closes_intermediate_response(self) -> None: + # When the body-preserving rebuild raises on a single-use body, the + # in-hand 307 response must be closed before the RuntimeError escapes. + tracking_body = _TrackingAsyncBody() + body = RequestBody.from_iter(iter([b"chunk"])) + request = _request(method=Method.POST, body=body) + client = _ScriptedAsyncClient( + [ + _Hop( + Status.TEMPORARY_REDIRECT, + "https://example.com/new", + body=tracking_body, + ), + ], + ) + policy = AsyncRedirectPolicy( + allowed_methods=frozenset({Method.GET, Method.HEAD, Method.POST}), + ) + with pytest.raises(RuntimeError): + await _run(client, policy, request) + assert tracking_body.closed + async def test_308_follows_with_original_method_and_body(self) -> None: body = RequestBody.from_bytes(b"payload") request = _request(method=Method.PUT, body=body) diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_async_sansio_resolve.py b/packages/dexpace-sdk-core/tests/pipeline/test_async_sansio_resolve.py new file mode 100644 index 0000000..82a1ec3 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/pipeline/test_async_sansio_resolve.py @@ -0,0 +1,43 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Unit tests for the async SansIO runner's ``_resolve`` helper. + +``_resolve`` awaits awaitable step results and passes plain values through +unchanged. Both async-callable steps (which return coroutines) and sync +callable steps (which return plain values) flow through the same code path. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Generator + +from dexpace.sdk.core.pipeline._async_sansio_runner import _resolve + + +async def test_resolve_passes_plain_value_through() -> None: + assert await _resolve(42) == 42 + assert await _resolve(None) is None + + +async def test_resolve_awaits_a_coroutine() -> None: + async def produce() -> str: + return "awaited" + + assert await _resolve(produce()) == "awaited" + + +async def test_resolve_awaits_a_future() -> None: + future: asyncio.Future[int] = asyncio.get_running_loop().create_future() + future.set_result(7) + assert await _resolve(future) == 7 + + +async def test_resolve_awaits_a_custom_awaitable() -> None: + class _Awaitable: + def __await__(self) -> Generator[None, None, str]: + yield from () + return "custom" + + assert await _resolve(_Awaitable()) == "custom" diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_async_staged_builder.py b/packages/dexpace-sdk-core/tests/pipeline/test_async_staged_builder.py index 53064df..8ab8bba 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_async_staged_builder.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_async_staged_builder.py @@ -19,6 +19,7 @@ from dexpace.sdk.core.pipeline.policies.async_retry import AsyncRetryPolicy if TYPE_CHECKING: + from dexpace.sdk.core.http.context.call_context import CallContext from dexpace.sdk.core.http.request.request import Request from dexpace.sdk.core.http.response.async_response import AsyncResponse from dexpace.sdk.core.pipeline.context import PipelineContext @@ -83,3 +84,16 @@ def test_from_pipeline_round_trip(async_transport: _AsyncStubTransport) -> None: ) rebuilt = AsyncStagedPipelineBuilder.from_pipeline(original).build() assert isinstance(rebuilt, AsyncPipeline) + + +def test_from_pipeline_sansio_step_raises_value_error( + async_transport: _AsyncStubTransport, +) -> None: + # A bare callable becomes an internal SansIO runner with no STAGE. + # from_pipeline must surface a clear ValueError, not a raw AttributeError. + def stamp(request: Request, ctx: CallContext) -> Request: + return request + + original = AsyncPipeline(async_transport, policies=[stamp]) + with pytest.raises(ValueError, match="SansIO"): + AsyncStagedPipelineBuilder.from_pipeline(original) diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_context_eviction.py b/packages/dexpace-sdk-core/tests/pipeline/test_context_eviction.py new file mode 100644 index 0000000..f8e27eb --- /dev/null +++ b/packages/dexpace-sdk-core/tests/pipeline/test_context_eviction.py @@ -0,0 +1,160 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Pipeline must evict its ``ContextStore`` entry once a call completes. + +The promotion chain registers the call's context in the process-wide +``ContextStore`` keyed by trace id. Without eviction the store grows without +bound across calls with distinct trace ids. ``Pipeline.run`` / +``AsyncPipeline.run`` evict the entry after the chain has fully unwound — i.e. +after every in-chain observer has read the latest snapshot. +""" + +from __future__ import annotations + +import asyncio + +import pytest + +from dexpace.sdk.core.client.async_http_client import AsyncHttpClient +from dexpace.sdk.core.client.http_client import HttpClient +from dexpace.sdk.core.errors import ServiceRequestError +from dexpace.sdk.core.http.common import Protocol, Url +from dexpace.sdk.core.http.context import ( + CallContext, + ContextStore, + DispatchContext, + ExchangeContext, +) +from dexpace.sdk.core.http.request import Method, Request +from dexpace.sdk.core.http.response import AsyncResponse, Response, Status +from dexpace.sdk.core.instrumentation import ( + InstrumentationContext, + SpanId, + TraceFlags, + TraceId, + TraceIdType, + TraceState, +) +from dexpace.sdk.core.instrumentation.noop import NOOP_SPAN +from dexpace.sdk.core.pipeline import ( + AsyncPipeline, + AsyncPolicy, + Pipeline, + PipelineContext, + Policy, +) + + +def _instr(trace: str) -> InstrumentationContext: + return InstrumentationContext( + trace_id_type=TraceIdType.W3C, + trace_id=TraceId(trace), + span_id=SpanId("0" * 16), + span=NOOP_SPAN, + trace_flags=TraceFlags.NOOP, + trace_state=TraceState.NOOP, + ) + + +def _request() -> Request: + return Request(method=Method.GET, url=Url.parse("https://example.com/")) + + +class _StubClient(HttpClient): + def execute(self, request: Request) -> Response: + return Response(request=request, protocol=Protocol.HTTP_1_1, status=Status.OK) + + +class _FailingClient(HttpClient): + def execute(self, request: Request) -> Response: + raise ServiceRequestError("boom") + + +class _StubAsyncClient(AsyncHttpClient): + async def execute(self, request: Request) -> AsyncResponse: + return AsyncResponse(request=request, protocol=Protocol.HTTP_1_1, status=Status.OK) + + +class _FailingAsyncClient(AsyncHttpClient): + async def execute(self, request: Request) -> AsyncResponse: + raise ServiceRequestError("boom") + + +def test_run_evicts_context_store_entry() -> None: + instr = _instr("0" * 16 + "1") + with Pipeline(_StubClient()) as pipe: + pipe.run(_request(), DispatchContext(instr)) + assert ContextStore.get(instr.trace_id.value) is None + assert ContextStore._contexts == {} + + +def test_run_evicts_even_when_chain_raises() -> None: + instr = _instr("0" * 16 + "2") + with Pipeline(_FailingClient()) as pipe, pytest.raises(ServiceRequestError): + pipe.run(_request(), DispatchContext(instr)) + assert ContextStore.get(instr.trace_id.value) is None + + +def test_in_chain_observer_still_sees_exchange_context() -> None: + """A response-side observer reading the store mid-unwind sees the exchange.""" + instr = _instr("0" * 16 + "3") + seen: list[CallContext | None] = [] + + class _Observer(Policy): + def send(self, request: Request, ctx: PipelineContext) -> Response: + response = self.next.send(request, ctx) + seen.append(ContextStore.get(instr.trace_id.value)) + return response + + with Pipeline(_StubClient(), policies=[_Observer()]) as pipe: + pipe.run(_request(), DispatchContext(instr)) + + assert len(seen) == 1 + assert isinstance(seen[0], ExchangeContext) + # Once run() returns, the entry is gone. + assert ContextStore.get(instr.trace_id.value) is None + + +def test_async_run_evicts_context_store_entry() -> None: + instr = _instr("0" * 16 + "4") + + async def run() -> None: + async with AsyncPipeline(_StubAsyncClient()) as pipe: + await pipe.run(_request(), DispatchContext(instr)) + + asyncio.run(run()) + assert ContextStore.get(instr.trace_id.value) is None + assert ContextStore._contexts == {} + + +def test_async_run_evicts_even_when_chain_raises() -> None: + instr = _instr("0" * 16 + "6") + + async def run() -> None: + async with AsyncPipeline(_FailingAsyncClient()) as pipe: + await pipe.run(_request(), DispatchContext(instr)) + + with pytest.raises(ServiceRequestError): + asyncio.run(run()) + assert ContextStore.get(instr.trace_id.value) is None + + +def test_async_in_chain_observer_still_sees_exchange_context() -> None: + instr = _instr("0" * 16 + "5") + seen: list[CallContext | None] = [] + + class _Observer(AsyncPolicy): + async def send(self, request: Request, ctx: PipelineContext) -> AsyncResponse: + response = await self.next.send(request, ctx) + seen.append(ContextStore.get(instr.trace_id.value)) + return response + + async def run() -> None: + async with AsyncPipeline(_StubAsyncClient(), policies=[_Observer()]) as pipe: + await pipe.run(_request(), DispatchContext(instr)) + + asyncio.run(run()) + assert len(seen) == 1 + assert isinstance(seen[0], ExchangeContext) + assert ContextStore.get(instr.trace_id.value) is None diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_redirect.py b/packages/dexpace-sdk-core/tests/pipeline/test_redirect.py index dbf3689..3f9c797 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_redirect.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_redirect.py @@ -5,7 +5,7 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Iterator, Sequence import pytest @@ -14,7 +14,7 @@ from dexpace.sdk.core.http.context import DispatchContext from dexpace.sdk.core.http.request import Method, Request from dexpace.sdk.core.http.request.request_body import RequestBody -from dexpace.sdk.core.http.response import Response, Status +from dexpace.sdk.core.http.response import Response, ResponseBody, Status from dexpace.sdk.core.instrumentation import ( InstrumentationContext, SpanId, @@ -52,20 +52,44 @@ def _request( return req +class _TrackingBody(ResponseBody): + """Response body that records whether ``close`` was called.""" + + __slots__ = ("closed",) + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + def iter_bytes(self, chunk_size: int = 64 * 1024) -> Iterator[bytes]: + self.close() + return iter(()) + + def close(self) -> None: + self.closed = True + + class _Hop: """One scripted transport response: status + Location header.""" - __slots__ = ("extra_headers", "location", "status") + __slots__ = ("body", "extra_headers", "location", "status") def __init__( self, status: Status, location: str | None = None, extra_headers: tuple[tuple[str, str], ...] = (), + body: ResponseBody | None = None, ) -> None: self.status = status self.location = location self.extra_headers = extra_headers + self.body = body class _ScriptedClient(HttpClient): @@ -79,7 +103,12 @@ def execute(self, request: Request) -> Response: idx = len(self.requests) self.requests.append(request) hop = self._hops[idx] - response = Response(request=request, protocol=Protocol.HTTP_1_1, status=hop.status) + response = Response( + request=request, + protocol=Protocol.HTTP_1_1, + status=hop.status, + body=hop.body, + ) if hop.location is not None: response = response.with_header("Location", hop.location) for name, value in hop.extra_headers: @@ -179,6 +208,30 @@ def test_307_with_non_replayable_body_raises(self) -> None: with pytest.raises(RuntimeError): _run(client, policy, request) + def test_307_non_replayable_body_closes_intermediate_response(self) -> None: + # The 3xx hop carries a body; when the body-preserving rebuild raises + # because the request body is single-use, the in-hand 307 response must + # be closed before the RuntimeError propagates — otherwise the + # connection leaks. + tracking_body = _TrackingBody() + body = RequestBody.from_iter(iter([b"chunk"])) + request = _request(method=Method.POST, body=body) + client = _ScriptedClient( + [ + _Hop( + Status.TEMPORARY_REDIRECT, + "https://example.com/new", + body=tracking_body, + ), + ], + ) + policy = RedirectPolicy( + allowed_methods=frozenset({Method.GET, Method.HEAD, Method.POST}), + ) + with pytest.raises(RuntimeError): + _run(client, policy, request) + assert tracking_body.closed + def test_308_follows_with_original_method_and_body(self) -> None: body = RequestBody.from_bytes(b"payload") request = _request(method=Method.PUT, body=body) diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_retry.py b/packages/dexpace-sdk-core/tests/pipeline/test_retry.py index a2e27bc..5a2b0ab 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_retry.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_retry.py @@ -169,24 +169,37 @@ def test_connect_retry_budget_exhausted(self) -> None: class TestRetryAfterHeader: def test_parse_delta_seconds(self) -> None: - assert _parse_retry_after("5") == pytest.approx(5.0) - assert _parse_retry_after("0") == pytest.approx(0.0) - assert _parse_retry_after("0.5") == pytest.approx(0.5) + assert _parse_retry_after("5", now=0.0) == pytest.approx(5.0) + assert _parse_retry_after("0", now=0.0) == pytest.approx(0.0) + assert _parse_retry_after("0.5", now=0.0) == pytest.approx(0.5) def test_parse_http_date_future(self) -> None: # Far-future date should be a positive delta. - result = _parse_retry_after("Sun, 06 Nov 2099 08:49:37 GMT") + result = _parse_retry_after("Sun, 06 Nov 2099 08:49:37 GMT", now=0.0) assert result is not None assert result > 0 def test_parse_http_date_past_clamps_to_zero(self) -> None: - result = _parse_retry_after("Mon, 01 Jan 1990 00:00:00 GMT") + # ``now`` is well past the 1990 header instant, so the delta is + # negative and clamps to zero. + result = _parse_retry_after("Mon, 01 Jan 1990 00:00:00 GMT", now=2_000_000_000.0) assert result == pytest.approx(0.0) + def test_parse_http_date_uses_injected_now(self) -> None: + # The HTTP-date branch computes ``when.timestamp() - now`` from the + # injected clock, not the wall clock. 2000-01-01T00:00:00Z is epoch + # 946684800; with ``now`` set 60s before it, the delay is exactly 60s. + epoch_2000 = 946_684_800.0 + result = _parse_retry_after( + "Sat, 01 Jan 2000 00:00:00 GMT", + now=epoch_2000 - 60.0, + ) + assert result == pytest.approx(60.0) + def test_parse_invalid_returns_none(self) -> None: - assert _parse_retry_after("not-a-date") is None - assert _parse_retry_after("") is None - assert _parse_retry_after(None) is None + assert _parse_retry_after("not-a-date", now=0.0) is None + assert _parse_retry_after("", now=0.0) is None + assert _parse_retry_after(None, now=0.0) is None def test_respected_during_retry(self) -> None: sleeps: list[float] = [] @@ -205,6 +218,26 @@ def sleep(self, duration: float) -> None: p.run(_get(), DispatchContext(_instr("0" * 16 + "a"))) assert sleeps and sleeps[0] == pytest.approx(2.0) + def test_http_date_uses_injected_clock_not_wall_clock(self) -> None: + # An HTTP-date ``Retry-After`` must be measured against the injected + # clock's wall-time. The FakeClock starts 30s before the header's + # instant, so the policy sleeps exactly 30s — deterministic regardless + # of the real wall clock. + epoch_2000 = 946_684_800.0 + clock = FakeClock(start=epoch_2000 - 30.0) + client = _ScriptedClient( + [Status.SERVICE_UNAVAILABLE, Status.OK], + retry_after="Sat, 01 Jan 2000 00:00:00 GMT", + ) + retry = RetryPolicy(clock=clock) + with Pipeline(client, policies=[retry]) as p: + response = p.run(_get(), DispatchContext(_instr("0" * 16 + "f1"))) + assert response.status is Status.OK + # Wall clock advanced by the slept 30s (FakeClock.sleep moves both). + # Absolute tolerance: a relative approx at this magnitude (~946s band) + # would mask a clock that never advanced. + assert clock.monotonic() == pytest.approx(epoch_2000, abs=1.0) + class TestRetryHistory: def test_history_recorded_in_ctx_data(self) -> None: diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_retry_config_removed.py b/packages/dexpace-sdk-core/tests/pipeline/test_retry_config_removed.py new file mode 100644 index 0000000..13ed1e4 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/pipeline/test_retry_config_removed.py @@ -0,0 +1,39 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Guards that the dead ``RetryConfig`` surface stays removed. + +``RetryConfig`` was unread public config that contradicted ``RetryPolicy``; +the real retry surface is ``RetryPolicy.__init__``. These tests pin the +removal so it cannot silently reappear. +""" + +from __future__ import annotations + +import importlib + +import pytest + +import dexpace.sdk.core.pipeline as pipeline +import dexpace.sdk.core.pipeline.step.config as config + + +def test_retry_config_not_exported_from_pipeline() -> None: + assert "RetryConfig" not in pipeline.__all__ + assert not hasattr(pipeline, "RetryConfig") + + +def test_retry_config_not_exported_from_config_module() -> None: + assert "RetryConfig" not in config.__all__ + assert not hasattr(config, "RetryConfig") + + +def test_step_metadata_still_available() -> None: + assert "StepMetadata" in pipeline.__all__ + assert pipeline.StepMetadata is config.StepMetadata + + +def test_retry_config_cannot_be_imported() -> None: + module = importlib.import_module("dexpace.sdk.core.pipeline.step.config") + with pytest.raises(AttributeError): + getattr(module, "RetryConfig") # noqa: B009 diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_retry_resource_cleanup.py b/packages/dexpace-sdk-core/tests/pipeline/test_retry_resource_cleanup.py new file mode 100644 index 0000000..39d9789 --- /dev/null +++ b/packages/dexpace-sdk-core/tests/pipeline/test_retry_resource_cleanup.py @@ -0,0 +1,297 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Resource-cleanup and deadline-propagation contracts for the retry policies. + +Covers two invariants that the sync and async retry loops must share: + +- A status-driven retry closes the intermediate response before sleeping, so + the pooled connection is released instead of leaking (the success / return + branches keep the response open — the caller owns those). +- Crossing the absolute deadline during a status-path backoff propagates + ``ServiceResponseTimeoutError`` immediately, rather than being swallowed and + re-entering the loop. The sync loop must match the async twin attempt-for- + attempt. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import AsyncIterator, Iterator, Sequence + +import pytest + +from dexpace.sdk.core.client.async_http_client import AsyncHttpClient +from dexpace.sdk.core.client.http_client import HttpClient +from dexpace.sdk.core.errors import ServiceResponseTimeoutError +from dexpace.sdk.core.http.common import Protocol, Url +from dexpace.sdk.core.http.context import DispatchContext +from dexpace.sdk.core.http.request import Method, Request +from dexpace.sdk.core.http.response import AsyncResponse, Response, Status +from dexpace.sdk.core.http.response.async_response_body import AsyncResponseBody +from dexpace.sdk.core.http.response.response_body import ResponseBody +from dexpace.sdk.core.instrumentation import ( + InstrumentationContext, + SpanId, + TraceFlags, + TraceId, + TraceIdType, + TraceState, +) +from dexpace.sdk.core.instrumentation.noop import NOOP_SPAN +from dexpace.sdk.core.pipeline import AsyncPipeline, Pipeline +from dexpace.sdk.core.pipeline.policies import RetryPolicy +from dexpace.sdk.core.pipeline.policies.async_retry import AsyncRetryPolicy + +from ..conftest import FakeClock + + +def _instr(trace: str) -> InstrumentationContext: + return InstrumentationContext( + trace_id_type=TraceIdType.W3C, + trace_id=TraceId(trace), + span_id=SpanId("0" * 16), + span=NOOP_SPAN, + trace_flags=TraceFlags.NOOP, + trace_state=TraceState.NOOP, + ) + + +def _get() -> Request: + return Request(method=Method.GET, url=Url.parse("https://example.com/")) + + +class _AsyncFakeClock: + """Deterministic ``AsyncClock`` for tests; advances time on sleep.""" + + __slots__ = ("_t",) + + def __init__(self, start: float = 0.0) -> None: + self._t = start + + def now(self) -> float: + return self._t + + def monotonic(self) -> float: + return self._t + + async def sleep(self, duration: float) -> None: + self._t += max(0.0, duration) + + +class _RecordingBody(ResponseBody): + """Sync response body that records whether ``close`` was called.""" + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + def iter_bytes(self, chunk_size: int = 64 * 1024) -> Iterator[bytes]: + yield b"" + + def close(self) -> None: + self.closed = True + + +class _AsyncRecordingBody(AsyncResponseBody): + """Async response body that records whether ``close`` was awaited.""" + + def __init__(self) -> None: + self.closed = False + + def media_type(self) -> None: + return None + + def content_length(self) -> int: + return 0 + + async def aiter_bytes(self, chunk_size: int = 64 * 1024) -> AsyncIterator[bytes]: + yield b"" + + async def close(self) -> None: + self.closed = True + + +class _BodyTrackingClient(HttpClient): + """Returns responses carrying a recording body the test can inspect.""" + + def __init__(self, outcomes: Sequence[Status]) -> None: + self._outcomes = list(outcomes) + self.bodies: list[_RecordingBody] = [] + self.attempts = 0 + + def execute(self, request: Request) -> Response: + outcome = self._outcomes[self.attempts] + self.attempts += 1 + body = _RecordingBody() + self.bodies.append(body) + return Response( + request=request, + protocol=Protocol.HTTP_1_1, + status=outcome, + body=body, + ) + + +class _AsyncBodyTrackingClient(AsyncHttpClient): + """Async twin of ``_BodyTrackingClient``.""" + + def __init__(self, outcomes: Sequence[Status]) -> None: + self._outcomes = list(outcomes) + self.bodies: list[_AsyncRecordingBody] = [] + self.attempts = 0 + + async def execute(self, request: Request) -> AsyncResponse: + outcome = self._outcomes[self.attempts] + self.attempts += 1 + body = _AsyncRecordingBody() + self.bodies.append(body) + return AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=outcome, + body=body, + ) + + +class _DeadlineClient(HttpClient): + """Always returns a 503 carrying a large ``Retry-After`` header. + + The big header forces the status-path sleep to clamp to the remaining + budget, crossing the deadline mid-backoff. + """ + + def __init__(self, retry_after: str) -> None: + self._retry_after = retry_after + self.attempts = 0 + + def execute(self, request: Request) -> Response: + self.attempts += 1 + return Response( + request=request, + protocol=Protocol.HTTP_1_1, + status=Status.SERVICE_UNAVAILABLE, + ).with_header("Retry-After", self._retry_after) + + +class _AsyncDeadlineClient(AsyncHttpClient): + """Async twin of ``_DeadlineClient``.""" + + def __init__(self, retry_after: str) -> None: + self._retry_after = retry_after + self.attempts = 0 + + async def execute(self, request: Request) -> AsyncResponse: + self.attempts += 1 + return AsyncResponse( + request=request, + protocol=Protocol.HTTP_1_1, + status=Status.SERVICE_UNAVAILABLE, + ).with_header("Retry-After", self._retry_after) + + +class TestIntermediateResponseClosed: + def test_sync_status_retry_closes_intermediate_response(self) -> None: + client = _BodyTrackingClient([Status.SERVICE_UNAVAILABLE, Status.OK]) + retry = RetryPolicy(clock=FakeClock()) + with Pipeline(client, policies=[retry]) as p: + response = p.run(_get(), DispatchContext(_instr("0" * 16 + "1"))) + assert response.status is Status.OK + # The intermediate 503's body was closed by the retry loop before it + # slept, releasing the pooled connection. + assert client.bodies[0].closed is True + # The final response is handed back open — the caller owns it. The + # policy must not have closed the response it returned. + assert client.bodies[1].closed is False + # Closing it is the caller's responsibility. + response.close() + assert client.bodies[1].closed is True + + async def test_async_status_retry_closes_intermediate_response(self) -> None: + client = _AsyncBodyTrackingClient([Status.SERVICE_UNAVAILABLE, Status.OK]) + retry = AsyncRetryPolicy(clock=_AsyncFakeClock()) + async with AsyncPipeline(client, policies=[retry]) as p: + response = await p.run(_get(), DispatchContext(_instr("0" * 16 + "2"))) + assert response.status is Status.OK + assert client.bodies[0].closed is True + assert client.bodies[1].closed is False + await response.close() + assert client.bodies[1].closed is True + + +class TestStatusPathDeadlinePropagates: + """The status-path sleep must propagate a deadline timeout immediately. + + With the sleep inside the ``except SdkError`` try (the prior bug), the + raised ``ServiceResponseTimeoutError`` — itself a ``ServiceResponseError`` + — was re-classified as a read error and the loop kept issuing requests + until the read budget drained. Outside the try, it propagates after the + very first attempt, matching the async twin. + """ + + def test_sync_propagates_after_single_attempt(self) -> None: + # ``backoff_factor=0`` makes the buggy swallow-and-retry path sleep zero + # seconds (which skips the deadline check), so the regression would keep + # issuing requests until the read budget drained. The fix lifts the + # status sleep out of the ``except SdkError`` try, so the timeout from + # the very first attempt propagates straight through. + client = _DeadlineClient(retry_after="100") + retry = RetryPolicy(timeout=1.0, backoff_factor=0.0, clock=FakeClock()) + with ( + Pipeline(client, policies=[retry]) as p, + pytest.raises(ServiceResponseTimeoutError), + ): + p.run(_get(), DispatchContext(_instr("0" * 16 + "3"))) + assert client.attempts == 1 + + async def test_async_propagates_after_single_attempt(self) -> None: + client = _AsyncDeadlineClient(retry_after="100") + retry = AsyncRetryPolicy(timeout=1.0, backoff_factor=0.0, clock=_AsyncFakeClock()) + with pytest.raises(ServiceResponseTimeoutError): + async with AsyncPipeline(client, policies=[retry]) as p: + await p.run(_get(), DispatchContext(_instr("0" * 16 + "4"))) + assert client.attempts == 1 + + def test_sync_attempt_count_matches_async_twin(self) -> None: + sync_client = _DeadlineClient(retry_after="100") + sync_retry = RetryPolicy(timeout=1.0, backoff_factor=0.0, clock=FakeClock()) + with ( + Pipeline(sync_client, policies=[sync_retry]) as p, + pytest.raises(ServiceResponseTimeoutError), + ): + p.run(_get(), DispatchContext(_instr("0" * 16 + "5"))) + + async_client = _AsyncDeadlineClient(retry_after="100") + + async def _drive_async() -> None: + async_retry = AsyncRetryPolicy( + timeout=1.0, + backoff_factor=0.0, + clock=_AsyncFakeClock(), + ) + with pytest.raises(ServiceResponseTimeoutError): + async with AsyncPipeline(async_client, policies=[async_retry]) as p: + await p.run(_get(), DispatchContext(_instr("0" * 16 + "6"))) + + asyncio.run(_drive_async()) + assert sync_client.attempts == async_client.attempts == 1 + + +def test_returned_response_not_closed_on_no_retry() -> None: + """A non-retryable response is handed back open; the close fix is scoped + to the status-retry path only.""" + client = _BodyTrackingClient([Status.OK]) + retry = RetryPolicy(clock=FakeClock()) + with Pipeline(client, policies=[retry]) as p: + response = p.run(_get(), DispatchContext(_instr("0" * 16 + "7"))) + assert response.status is Status.OK + # The policy did not close the response it returned to the caller. + assert client.bodies[0].closed is False + # The caller owns it and closes it explicitly. + response.close() + assert client.bodies[0].closed is True diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_review_fixes.py b/packages/dexpace-sdk-core/tests/pipeline/test_review_fixes.py index 1297798..870e8d5 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_review_fixes.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_review_fixes.py @@ -94,12 +94,18 @@ def send(self, request: Request, ctx: PipelineContext) -> Response: def test_async_transport_runner_promotes_context() -> None: - """Fix #6: the async pipeline records the ExchangeContext after the call.""" + """Fix #6: the async transport runner records the ExchangeContext. + + The promotion is observed while the call is in flight — once + ``AsyncPipeline.run`` returns, the entry is evicted to keep the store from + growing without bound across calls with distinct trace ids. + """ import asyncio from dexpace.sdk.core.client.async_http_client import AsyncHttpClient + from dexpace.sdk.core.http.context import ExchangeContext from dexpace.sdk.core.http.response import AsyncResponse - from dexpace.sdk.core.pipeline import AsyncPipeline + from dexpace.sdk.core.pipeline import AsyncPipeline, AsyncPolicy, PipelineContext class _Client(AsyncHttpClient): async def execute(self, request: Request) -> AsyncResponse: @@ -110,14 +116,20 @@ async def execute(self, request: Request) -> AsyncResponse: ) instr = _instr("0" * 16 + "3") + seen: list[object] = [] + + class _Observer(AsyncPolicy): + async def send(self, request: Request, ctx: PipelineContext) -> AsyncResponse: + response = await self.next.send(request, ctx) + seen.append(ContextStore.get(instr.trace_id.value)) + return response async def run() -> None: - async with AsyncPipeline(_Client()) as pipe: + async with AsyncPipeline(_Client(), policies=[_Observer()]) as pipe: await pipe.run(_get(), DispatchContext(instr)) asyncio.run(run()) - stored = ContextStore.get(instr.trace_id.value) - # The exchange context is the latest snapshot in the store. - from dexpace.sdk.core.http.context import ExchangeContext - - assert isinstance(stored, ExchangeContext) + # The exchange context was the latest snapshot while the call was live. + assert isinstance(seen[0], ExchangeContext) + # Eviction-on-completion clears the entry afterwards. + assert ContextStore.get(instr.trace_id.value) is None diff --git a/packages/dexpace-sdk-core/tests/pipeline/test_staged_builder.py b/packages/dexpace-sdk-core/tests/pipeline/test_staged_builder.py index 514428e..6a65a26 100644 --- a/packages/dexpace-sdk-core/tests/pipeline/test_staged_builder.py +++ b/packages/dexpace-sdk-core/tests/pipeline/test_staged_builder.py @@ -15,6 +15,7 @@ from dexpace.sdk.core.pipeline.policies.tracing_policy import TracingPolicy if TYPE_CHECKING: + from dexpace.sdk.core.http.context.call_context import CallContext from dexpace.sdk.core.http.request.request import Request from dexpace.sdk.core.http.response.response import Response from dexpace.sdk.core.pipeline.context import PipelineContext @@ -196,6 +197,16 @@ def test_misordered_pipeline_raises(self, transport: _StubTransport) -> None: with pytest.raises(ValueError, match="non-decreasing stage order"): StagedPipelineBuilder.from_pipeline(original) + def test_sansio_step_raises_value_error(self, transport: _StubTransport) -> None: + # A bare callable becomes an internal SansIO runner with no STAGE. + # from_pipeline must surface a clear ValueError, not a raw AttributeError. + def stamp(request: Request, ctx: CallContext) -> Request: + return request + + original = Pipeline(transport, policies=[stamp]) + with pytest.raises(ValueError, match="SansIO"): + StagedPipelineBuilder.from_pipeline(original) + class TestPillarReplacementSafety: """Default raises; force=True is the explicit escape.""" diff --git a/packages/dexpace-sdk-core/tests/serde/test_codec.py b/packages/dexpace-sdk-core/tests/serde/test_codec.py index 503a487..980f7d3 100644 --- a/packages/dexpace-sdk-core/tests/serde/test_codec.py +++ b/packages/dexpace-sdk-core/tests/serde/test_codec.py @@ -13,6 +13,7 @@ import enum from dataclasses import dataclass, field from datetime import UTC, date, datetime, time +from uuid import UUID import pytest @@ -170,6 +171,24 @@ class Wrapped: assert model.when == Present(datetime(2026, 1, 2, 3, 4, 5, tzinfo=UTC)) +def test_decode_bare_tristate_value_wraps_in_present() -> None: + @dataclass(frozen=True, slots=True) + class Wrapped: + note: Tristate = ABSENT # type: ignore[type-arg] + + model = Codec().decode({"note": "hi"}, Wrapped) + assert model.note == Present("hi") + + +def test_decode_bare_tristate_null_becomes_null() -> None: + @dataclass(frozen=True, slots=True) + class Wrapped: + note: Tristate = ABSENT # type: ignore[type-arg] + + model = Codec().decode({"note": None}, Wrapped) + assert model.note is NULL + + # --------------------------------------------------------------------------- # # Encode round-trip # # --------------------------------------------------------------------------- # @@ -568,6 +587,99 @@ def test_decode_dict_recovers_enum_keys_and_model_values(codec: Codec) -> None: assert decoded == {_KeyKind.SECOND: _Inner(5)} +# --------------------------------------------------------------------------- # +# Mapping encode collapses non-string keys (decode/encode round-trip) # +# --------------------------------------------------------------------------- # + + +def test_encode_dict_with_enum_keys_collapses_to_value(codec: Codec) -> None: + encoded = codec.encode({_KeyKind.FIRST: 10}) + assert encoded == {"first": 10} + + +def test_dict_enum_key_field_round_trips(codec: Codec) -> None: + @dataclass(frozen=True, slots=True) + class Holder: + counts: dict[_KeyKind, int] + + model = Holder({_KeyKind.FIRST: 10, _KeyKind.SECOND: 20}) + encoded = codec.encode(model) + assert isinstance(encoded, dict) + assert encoded["counts"] == {"first": 10, "second": 20} + decoded = codec.decode(encoded, Holder) + assert decoded == model + + +def test_encode_dict_with_date_keys_collapses_to_iso(codec: Codec) -> None: + encoded = codec.encode({date(2026, 1, 2): 5}) + assert encoded == {"2026-01-02": 5} + + +def test_dict_date_key_field_round_trips(codec: Codec) -> None: + @dataclass(frozen=True, slots=True) + class Holder: + by_day: dict[date, int] + + model = Holder({date(2026, 1, 2): 5, date(2026, 3, 4): 6}) + encoded = codec.encode(model) + assert isinstance(encoded, dict) + assert encoded["by_day"] == {"2026-01-02": 5, "2026-03-04": 6} + decoded = codec.decode(encoded, Holder) + assert decoded == model + + +def test_encode_dict_rejects_non_collapsible_key(codec: Codec) -> None: + with pytest.raises(SerializationError): + codec.encode({object(): 1}) + + +# --------------------------------------------------------------------------- # +# UUID values and keys (symmetric encode/decode round-trip) # +# --------------------------------------------------------------------------- # + +_UUID = UUID("12345678-1234-5678-1234-567812345678") + + +def test_encode_uuid_value_collapses_to_str(codec: Codec) -> None: + assert codec.encode(_UUID) == "12345678-1234-5678-1234-567812345678" + assert type(codec.encode(_UUID)) is str + + +def test_uuid_value_field_round_trips(codec: Codec) -> None: + @dataclass(frozen=True, slots=True) + class Holder: + id: UUID + + model = Holder(_UUID) + decoded = codec.decode(codec.encode(model), Holder) + assert decoded == model + assert isinstance(decoded.id, UUID) + + +def test_encode_dict_with_uuid_keys_collapses_to_str(codec: Codec) -> None: + assert codec.encode({_UUID: 5}) == {"12345678-1234-5678-1234-567812345678": 5} + + +def test_dict_uuid_key_field_round_trips(codec: Codec) -> None: + @dataclass(frozen=True, slots=True) + class Holder: + by_id: dict[UUID, int] + + model = Holder({_UUID: 5}) + encoded = codec.encode(model) + assert isinstance(encoded, dict) + assert encoded["by_id"] == {"12345678-1234-5678-1234-567812345678": 5} + decoded = codec.decode(encoded, Holder) + assert decoded == model + assert all(isinstance(k, UUID) for k in decoded.by_id) + + +def test_decode_invalid_uuid_raises_with_path(codec: Codec) -> None: + with pytest.raises(CodecError) as info: + codec.decode("not-a-uuid", UUID) + assert "UUID" in str(info.value) + + # --------------------------------------------------------------------------- # # Union None-passthrough gating # # --------------------------------------------------------------------------- # diff --git a/packages/dexpace-sdk-core/tests/serde/test_json_serde.py b/packages/dexpace-sdk-core/tests/serde/test_json_serde.py index 241dea6..e14031a 100644 --- a/packages/dexpace-sdk-core/tests/serde/test_json_serde.py +++ b/packages/dexpace-sdk-core/tests/serde/test_json_serde.py @@ -116,3 +116,47 @@ def test_allow_nan_true_emits_nan() -> None: ser = JsonSerializer(allow_nan=True) text = ser.serialize({"v": math.nan}) assert "NaN" in text + + +def test_custom_default_still_encodes_builtin_datetime() -> None: + class _Custom: + pass + + def my_default(o: object) -> object: + if isinstance(o, _Custom): + return "custom" + raise TypeError(o) + + ser = JsonSerializer(default=my_default) + when = datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC) + text = ser.serialize({"at": when}) + assert "2024-01-01T12:00:00+00:00" in text + + +def test_custom_default_handles_its_own_types() -> None: + class _Custom: + pass + + def my_default(o: object) -> object: + if isinstance(o, _Custom): + return "custom" + raise TypeError(o) + + ser = JsonSerializer(default=my_default) + assert ser.serialize({"x": _Custom()}) == '{"x":"custom"}' + + +def test_custom_default_encodes_both_datetime_and_custom() -> None: + class _Custom: + pass + + def my_default(o: object) -> object: + if isinstance(o, _Custom): + return "custom" + raise TypeError(o) + + ser = JsonSerializer(default=my_default) + when = datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC) + text = ser.serialize({"at": when, "x": _Custom()}) + assert "2024-01-01T12:00:00+00:00" in text + assert '"x":"custom"' in text diff --git a/packages/dexpace-sdk-core/tests/sse/test_sse_crlf_boundary.py b/packages/dexpace-sdk-core/tests/sse/test_sse_crlf_boundary.py new file mode 100644 index 0000000..f6a659a --- /dev/null +++ b/packages/dexpace-sdk-core/tests/sse/test_sse_crlf_boundary.py @@ -0,0 +1,58 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Tests for CRLF terminators split across a chunk boundary in the SSE parser. + +A CR at the very end of one chunk followed by an LF at the start of the next +must be recognised as a single CRLF terminator, not two separate line endings. +Otherwise one multi-line event is split into several. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator + +from dexpace.sdk.core.http.sse import SseEvent, parse_async_events, parse_events +from dexpace.sdk.core.http.sse.parser import SseParser + + +def test_crlf_split_across_chunk_boundary_yields_single_event() -> None: + # CR ends the first chunk; LF begins the second. The two halves form one + # CRLF terminator, so the two data lines join into a single event. + chunks = [b"data: a\r", b"\ndata: b\r\n\r\n"] + events = list(parse_events(chunks)) + assert events == [SseEvent(data="a\nb")] + + +def test_cr_held_until_next_byte_disambiguates_lone_cr() -> None: + # A lone CR (next byte is not LF) is still a terminator once the next byte + # arrives, so the two data lines join into one event. + chunks = [b"data: a\r", b"data: b\r\n\r\n"] + events = list(parse_events(chunks)) + assert events == [SseEvent(data="a\nb")] + + +def test_trailing_cr_at_eos_is_a_terminator_not_literal_carriage_return() -> None: + # A CR that is the final byte of the whole stream terminates the line; it + # must not leak into the decoded field value as a literal '\r'. + parser = SseParser() + parser.feed(b"data: tail\r") + events = list(parser.end()) + assert events == [SseEvent(data="tail")] + + +def test_trailing_cr_at_eos_single_feed() -> None: + # Same property exercised through the convenience driver. + events = list(parse_events([b"data: tail\r"])) + assert events == [SseEvent(data="tail")] + + +async def test_crlf_split_across_chunk_boundary_async() -> None: + async def producer() -> AsyncIterator[bytes]: + yield b"data: a\r" + yield b"\ndata: b\r\n\r\n" + + events: list[SseEvent] = [] + async for event in parse_async_events(producer()): + events.append(event) + assert events == [SseEvent(data="a\nb")] diff --git a/packages/dexpace-sdk-core/tests/test_docstring_style.py b/packages/dexpace-sdk-core/tests/test_docstring_style.py new file mode 100644 index 0000000..63ceeeb --- /dev/null +++ b/packages/dexpace-sdk-core/tests/test_docstring_style.py @@ -0,0 +1,56 @@ +# Copyright (c) 2026 dexpace and Omar Aljarrah. +# Licensed under the MIT License. See LICENSE.md in the repository root for details. + +"""Docstring-style guardrail: no Sphinx cross-reference roles. + +The project convention (CLAUDE.md) mandates Google-style docstrings with plain +backticks for code/type references — never Sphinx roles such as +``:class:`Foo```. Neither ``ruff`` nor ``mypy`` inspects docstring prose, so a +one-time conversion sweep would silently rot without a mechanical guard. This +test scans every package source and test file and fails if any role reappears, +keeping the convention enforceable rather than aspirational. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[3] +_SELF = Path(__file__).resolve() + +# Assembled from parts (and an explicit backtick via ``chr(96)``) so this file +# contains no literal Sphinx role and never flags itself. +_ROLE_NAMES = ("class", "meth", "func", "data", "attr", "mod", "exc", "obj", "paramref", "term") +_ROLE_RE = re.compile(r":(?:" + "|".join(_ROLE_NAMES) + r"):" + chr(96)) + + +def _python_sources() -> list[Path]: + """Return every ``.py`` file under each package's ``src`` and ``tests``.""" + files: list[Path] = [] + packages = _REPO_ROOT / "packages" + for pkg in sorted(packages.iterdir()): + for sub in ("src", "tests"): + root = pkg / sub + if root.is_dir(): + files.extend(p for p in root.rglob("*.py") if p.resolve() != _SELF) + return files + + +def test_no_sphinx_roles_in_docstrings() -> None: + """No source or test file may use a Sphinx cross-reference role.""" + offenders: list[str] = [] + for path in _python_sources(): + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + if _ROLE_RE.search(line): + offenders.append(f"{path.relative_to(_REPO_ROOT)}:{lineno}: {line.strip()}") + assert not offenders, ( + "Sphinx cross-reference roles are forbidden; use plain backticks " + "(e.g. ':class:`Foo`' becomes '`Foo`'). Offenders:\n" + "\n".join(offenders) + ) + + +def test_guard_detects_a_role() -> None: + """The matcher itself works (so a green run means real coverage).""" + assert _ROLE_RE.search(":meth:" + chr(96) + "Foo.bar" + chr(96)) is not None + assert _ROLE_RE.search("plain `Foo` reference") is None diff --git a/packages/dexpace-sdk-core/tests/util/test_clock.py b/packages/dexpace-sdk-core/tests/util/test_clock.py index 0bd7c14..8cf2fbd 100644 --- a/packages/dexpace-sdk-core/tests/util/test_clock.py +++ b/packages/dexpace-sdk-core/tests/util/test_clock.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Tests for the :mod:`dexpace.sdk.core.util.clock` module.""" +"""Tests for the `dexpace.sdk.core.util.clock` module.""" from __future__ import annotations @@ -93,13 +93,13 @@ def test_async_system_clock_now_and_monotonic() -> None: def test_clock_protocol_satisfied_by_system_clock() -> None: - """``SYSTEM_CLOCK`` structurally satisfies the :class:`Clock` protocol.""" + """``SYSTEM_CLOCK`` structurally satisfies the `Clock` protocol.""" assert isinstance(SYSTEM_CLOCK, Clock) assert isinstance(_SystemClock(), Clock) def test_async_clock_protocol_satisfied_by_async_system_clock() -> None: - """``ASYNC_SYSTEM_CLOCK`` structurally satisfies :class:`AsyncClock`.""" + """``ASYNC_SYSTEM_CLOCK`` structurally satisfies `AsyncClock`.""" assert isinstance(ASYNC_SYSTEM_CLOCK, AsyncClock) assert isinstance(_AsyncSystemClock(), AsyncClock) @@ -128,7 +128,7 @@ def test_fake_clock_advance(fake_clock: FakeClock) -> None: def test_fake_clock_satisfies_clock_protocol(fake_clock: FakeClock) -> None: - """``FakeClock`` is structurally a :class:`Clock`.""" + """``FakeClock`` is structurally a `Clock`.""" assert isinstance(fake_clock, Clock) diff --git a/packages/dexpace-sdk-core/tests/util/test_proxy.py b/packages/dexpace-sdk-core/tests/util/test_proxy.py index bba27fd..236ec83 100644 --- a/packages/dexpace-sdk-core/tests/util/test_proxy.py +++ b/packages/dexpace-sdk-core/tests/util/test_proxy.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Tests for the :mod:`dexpace.sdk.core.util.proxy` module.""" +"""Tests for the `dexpace.sdk.core.util.proxy` module.""" from __future__ import annotations @@ -69,6 +69,84 @@ def test_bypasses_proxy_no_match() -> None: assert options.bypasses_proxy("api.example.org") is False +def test_bypasses_proxy_bare_entry_matches_subdomains() -> None: + """A bare ``example.com`` entry follows curl/Go suffix semantics.""" + options = ProxyOptions( + type=ProxyType.HTTP, + host="proxy.corp", + port=8080, + non_proxy_hosts=("example.com",), + ) + # Suffix match: any subdomain is bypassed. + assert options.bypasses_proxy("api.example.com") is True + # Exact match of the entry itself. + assert options.bypasses_proxy("example.com") is True + # Case-insensitive. + assert options.bypasses_proxy("API.EXAMPLE.COM") is True + # A host that merely ends with the same characters but is not a + # dot-delimited suffix must NOT be bypassed. + assert options.bypasses_proxy("notexample.com") is False + assert options.bypasses_proxy("example.org") is False + + +def test_bypasses_proxy_leading_dot_entry() -> None: + """A leading-dot ``.example.com`` entry strips the dot and matches the suffix.""" + options = ProxyOptions( + type=ProxyType.HTTP, + host="proxy.corp", + port=8080, + non_proxy_hosts=(".example.com",), + ) + assert options.bypasses_proxy("api.example.com") is True + assert options.bypasses_proxy("example.com") is True + assert options.bypasses_proxy("notexample.com") is False + + +def test_bypasses_proxy_ignores_port_on_candidate_and_entry() -> None: + """Conventional NO_PROXY parity: ports on the candidate or entry are ignored.""" + options = ProxyOptions( + type=ProxyType.HTTP, + host="proxy.corp", + port=8080, + non_proxy_hosts=("example.com:443",), + ) + # A port on the candidate host is stripped before matching. + assert options.bypasses_proxy("api.example.com:443") is True + assert options.bypasses_proxy("example.com:8443") is True + # A port on the entry itself is ignored (host-only suffix match). + assert options.bypasses_proxy("api.example.com") is True + # No spurious match through the port handling. + assert options.bypasses_proxy("notexample.com:443") is False + + +def test_bypasses_proxy_ipv6_literal_with_port() -> None: + """A bracketed IPv6 literal still matches once a port is appended.""" + options = ProxyOptions( + type=ProxyType.HTTP, + host="proxy.corp", + port=8080, + non_proxy_hosts=("::1",), + ) + # Bare IPv6 (multiple colons, no port) is left intact and matches exactly. + assert options.bypasses_proxy("::1") is True + # Bracketed form with a port drops both brackets and port before matching. + assert options.bypasses_proxy("[::1]:443") is True + + +def test_bypasses_proxy_explicit_glob_still_works() -> None: + """Explicit ``*`` globs keep their fnmatch semantics.""" + options = ProxyOptions( + type=ProxyType.HTTP, + host="proxy.corp", + port=8080, + non_proxy_hosts=("*.example.com",), + ) + assert options.bypasses_proxy("api.example.com") is True + assert options.bypasses_proxy("a.b.example.com") is True + # A bare ``*.example.com`` glob does not match the apex domain. + assert options.bypasses_proxy("example.com") is False + + def test_repr_masks_credentials() -> None: """``repr`` masks both username and password when present.""" options = ProxyOptions( diff --git a/packages/dexpace-sdk-core/tests/webhooks/test_verification.py b/packages/dexpace-sdk-core/tests/webhooks/test_verification.py index 207d3c0..211d577 100644 --- a/packages/dexpace-sdk-core/tests/webhooks/test_verification.py +++ b/packages/dexpace-sdk-core/tests/webhooks/test_verification.py @@ -185,6 +185,49 @@ def test_verify_rejects_a_non_integer_timestamp() -> None: _verifier().verify(headers, _BODY) +def test_verify_rejects_an_underscore_timestamp() -> None: + # PEP 515 underscore digit grouping is accepted by int() but is not a valid + # Standard Webhooks timestamp and must be rejected. + signature = _sign(_RAW_KEY, _WEBHOOK_ID, _TIMESTAMP, _BODY) + headers = _headers(signature, timestamp="1_690_000_000") + with pytest.raises(InvalidWebhookSignatureError, match="malformed webhook-timestamp"): + _verifier().verify(headers, _BODY) + + +def test_verify_skips_a_non_ascii_signature_token() -> None: + # A v1 token carrying a non-ASCII character must be skipped, not raised out + # of verify() as a UnicodeEncodeError (a one-header denial-of-service). + good = _sign(_RAW_KEY, _WEBHOOK_ID, _TIMESTAMP, _BODY) + tampered = good.replace(good[0], "é", 1) if good[0] != "é" else "é" + good[1:] + headers = { + "webhook-id": _WEBHOOK_ID, + "webhook-timestamp": _TIMESTAMP, + "webhook-signature": f"v1,{tampered}", + } + with pytest.raises(InvalidWebhookSignatureError, match="no matching signature"): + _verifier().verify(headers, _BODY) + + +def test_verify_skips_non_ascii_token_but_still_matches_a_valid_one() -> None: + good = _sign(_RAW_KEY, _WEBHOOK_ID, _TIMESTAMP, _BODY) + headers = { + "webhook-id": _WEBHOOK_ID, + "webhook-timestamp": _TIMESTAMP, + "webhook-signature": f"v1,nonéascii v1,{good}", + } + _verifier().verify(headers, _BODY) + + +def test_empty_secret_is_rejected_at_construction() -> None: + with pytest.raises(InvalidWebhookSignatureError, match="empty webhook secret"): + WebhookVerifier("") + + +def test_bare_whsec_prefix_secret_is_rejected_at_construction() -> None: + with pytest.raises(InvalidWebhookSignatureError, match="empty webhook secret"): + WebhookVerifier("whsec_") + + def test_whsec_prefix_is_stripped_and_base64_decoded() -> None: # A verifier built from the whsec_-prefixed secret produces the same result # as signing with the raw decoded key — i.e. the prefix was stripped and diff --git a/packages/dexpace-sdk-http-aiohttp/src/dexpace/sdk/http/aiohttp/client.py b/packages/dexpace-sdk-http-aiohttp/src/dexpace/sdk/http/aiohttp/client.py index 1558015..72fdf56 100644 --- a/packages/dexpace-sdk-http-aiohttp/src/dexpace/sdk/http/aiohttp/client.py +++ b/packages/dexpace-sdk-http-aiohttp/src/dexpace/sdk/http/aiohttp/client.py @@ -1,16 +1,16 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""``AsyncHttpClient`` implementation backed by :mod:`aiohttp`. +"""``AsyncHttpClient`` implementation backed by `aiohttp`. ``aiohttp`` exposes only an async API; this package therefore ships an async transport without a sync twin. The adapter is a thin pass-through: - Request bodies are forwarded to ``aiohttp`` via an async-iterable shim - over :meth:`RequestBody.iter_bytes`, so uploads stream without buffering + over `RequestBody.iter_bytes`, so uploads stream without buffering the full payload into memory. - Response content streams through ``aiohttp.StreamReader``; we wrap it as - an :class:`AsyncResponseBody` so the SDK's body lifecycle (deferred + an `AsyncResponseBody` so the SDK's body lifecycle (deferred read, deterministic close) is preserved. - Transport exceptions are mapped to the SDK's typed error hierarchy. @@ -27,6 +27,7 @@ import aiohttp from dexpace.sdk.core.errors import ( ServiceRequestError, + ServiceRequestTimeoutError, ServiceResponseError, ServiceResponseTimeoutError, ) @@ -45,19 +46,21 @@ class AiohttpHttpClient: - """Async ``HttpClient`` over an :class:`aiohttp.ClientSession`. + """Async ``HttpClient`` over an `aiohttp.ClientSession`. The client owns the session by default and releases it on ``aclose``. Pass an existing ``session`` to share connection pooling with other components; the caller is then responsible for closing it. Attributes: - timeout: Total request timeout in seconds. Applied via - ``aiohttp.ClientTimeout(total=...)``. ``None`` disables the + timeout: Per-phase request timeout in seconds, applied to both the + connect and the socket-read phases via + ``aiohttp.ClientTimeout(sock_connect=..., sock_read=...)`` so the + two phases raise distinguishable exceptions. ``None`` disables the timeout entirely (not recommended). """ - __slots__ = ("_owns_session", "_session", "_session_factory", "timeout") + __slots__ = ("_closed", "_owns_session", "_session", "timeout") def __init__( self, @@ -70,12 +73,21 @@ def __init__( self.timeout = timeout self._session = session self._owns_session = session is None - self._session_factory = aiohttp.ClientSession + self._closed = False async def execute(self, request: Request) -> AsyncResponse: + if self._closed: + raise ServiceRequestError("AiohttpHttpClient is closed") session = await self._ensure_session() + # Per-phase budgets (not a single ``total=``) so aiohttp raises the + # distinguishable ``ConnectionTimeoutError`` for a connect-phase stall + # and ``SocketTimeoutError`` for a read-phase stall. This keeps the + # connect -> request-error / read -> response-error split consistent + # with the other transports, which all use per-operation timeouts. timeout_cfg = ( - aiohttp.ClientTimeout(total=self.timeout) if self.timeout is not None else None + aiohttp.ClientTimeout(sock_connect=self.timeout, sock_read=self.timeout) + if self.timeout is not None + else None ) data = _payload(request.body) try: @@ -90,6 +102,10 @@ async def execute(self, request: Request) -> AsyncResponse: aio_response = await ctx except aiohttp.ClientConnectorError as err: raise ServiceRequestError(f"Connect failed: {err}", error=err) from err + except aiohttp.ConnectionTimeoutError as err: + raise ServiceRequestTimeoutError( + f"Connection to {request.url} timed out", error=err + ) from err except TimeoutError as err: raise ServiceResponseTimeoutError( f"Request to {request.url} timed out", error=err @@ -101,6 +117,9 @@ async def execute(self, request: Request) -> AsyncResponse: return _wrap_response(request, aio_response) async def aclose(self) -> None: + if self._closed: + return + self._closed = True if self._session is not None and self._owns_session: await self._session.close() self._session = None @@ -119,7 +138,8 @@ async def __aexit__( async def _ensure_session(self) -> aiohttp.ClientSession: if self._session is None: - self._session = self._session_factory() + # Construct lazily so the session binds the running event loop. + self._session = aiohttp.ClientSession() self._owns_session = True return self._session @@ -159,6 +179,9 @@ def _wrap_response(request: Request, aio_response: aiohttp.ClientResponse) -> As try: status = Status(aio_response.status) except ValueError as err: + # Release the handle before bailing so the connection returns to the + # pool instead of leaking (aiohttp's release() is synchronous). + aio_response.release() raise ServiceResponseError( f"Unknown status code: {aio_response.status}", error=err ) from err @@ -179,7 +202,7 @@ def _wrap_response(request: Request, aio_response: aiohttp.ClientResponse) -> As class _StreamReaderAdapter: - """``SupportsAsyncRead`` adapter over an :class:`aiohttp.ClientResponse`. + """``SupportsAsyncRead`` adapter over an `aiohttp.ClientResponse`. Owns the response handle; closing the adapter releases the connection back to the pool. diff --git a/packages/dexpace-sdk-http-aiohttp/tests/test_aiohttp_client.py b/packages/dexpace-sdk-http-aiohttp/tests/test_aiohttp_client.py index 044470b..a8f705b 100644 --- a/packages/dexpace-sdk-http-aiohttp/tests/test_aiohttp_client.py +++ b/packages/dexpace-sdk-http-aiohttp/tests/test_aiohttp_client.py @@ -13,6 +13,8 @@ from dexpace.sdk.core.errors import ( ServiceRequestError, + ServiceRequestTimeoutError, + ServiceResponseError, ServiceResponseTimeoutError, ) from dexpace.sdk.core.http.common import Url @@ -20,6 +22,7 @@ from dexpace.sdk.core.http.request.request_body import RequestBody from dexpace.sdk.core.http.response import Status from dexpace.sdk.http.aiohttp import AiohttpHttpClient +from dexpace.sdk.http.aiohttp.client import _wrap_response # ---------------------------------------------------------------------- handlers @@ -170,3 +173,119 @@ async def test_content_length_extracted_from_response(base_url: str) -> None: assert response.body is not None # /ok returns a small JSON; Content-Length should be set by aiohttp. assert response.body.content_length() > 0 + + +# ----------------------------------------------------------------- unknown status + + +class _FakeAioResponse: + """Minimal stand-in for an ``aiohttp.ClientResponse`` carrying a raw status.""" + + def __init__(self, status: int) -> None: + self.status = status + self.released = False + + def release(self) -> None: # aiohttp's release() is synchronous + self.released = True + + +def test_unknown_status_releases_connection_and_raises() -> None: + """An unregistered status code must release the response before raising.""" + request = Request(method=Method.GET, url=Url.parse("http://example.test/")) + fake = _FakeAioResponse(520) # Cloudflare 'Web Server Returned an Unknown Error' + + with pytest.raises(ServiceResponseError) as exc_info: + _wrap_response(request, fake) # type: ignore[arg-type] + + assert fake.released, "connection leaked: release() was not called" + assert "520" in str(exc_info.value) + + +# ----------------------------------------------------------------- post-close + + +async def test_execute_after_aclose_raises() -> None: + """A closed client must not resurrect; execute() raises ServiceRequestError.""" + client = AiohttpHttpClient(timeout=5.0) + await client.aclose() + with pytest.raises(ServiceRequestError, match="closed"): + await client.execute(Request(method=Method.GET, url=Url.parse("http://example.test/"))) + + +async def test_aclose_is_idempotent() -> None: + """Calling aclose() twice is a no-op and stays in the closed state.""" + client = AiohttpHttpClient(timeout=5.0) + await client.aclose() + await client.aclose() # must not raise + with pytest.raises(ServiceRequestError, match="closed"): + await client.execute(Request(method=Method.GET, url=Url.parse("http://example.test/"))) + + +# ----------------------------------------------------------- connect timeout + + +class _ConnectTimeoutSession: + """Stub session whose ``request`` raises aiohttp's connect-phase timeout. + + aiohttp raises ``ConnectionTimeoutError`` for a connect-scoped timeout + (``connect=`` / ``sock_connect=``) and ``SocketTimeoutError`` for a read + timeout (``sock_read=``); the client configures both so the two phases stay + distinguishable. We drive the connect branch directly with the exception + aiohttp raises so the test stays hermetic (no real unreachable-host connect). + """ + + def request(self, **_kwargs: object) -> _ConnectTimeoutSession: + return self + + def __await__(self) -> object: + raise aiohttp.ConnectionTimeoutError("connect timed out") + yield # pragma: no cover - makes this an awaitable generator + + +async def test_connect_timeout_maps_to_request_timeout() -> None: + """A connect-phase timeout maps to ServiceRequestTimeoutError, not a response timeout.""" + client = AiohttpHttpClient(timeout=5.0, session=_ConnectTimeoutSession()) # type: ignore[arg-type] + with pytest.raises(ServiceRequestTimeoutError): + await client.execute(Request(method=Method.GET, url=Url.parse("http://example.test/"))) + + +class _CaptureTimeoutSession: + """Captures the ``ClientTimeout`` passed to ``request`` then fails the connect.""" + + def __init__(self) -> None: + self.captured: aiohttp.ClientTimeout | None = None + + def request( + self, *, timeout: aiohttp.ClientTimeout, **_kwargs: object + ) -> _CaptureTimeoutSession: + self.captured = timeout + return self + + def __await__(self) -> object: + raise aiohttp.ConnectionTimeoutError("connect timed out") + yield # pragma: no cover - makes this an awaitable generator + + +async def test_timeout_configured_per_phase_so_connect_is_distinguishable() -> None: + """The client asks aiohttp for per-phase sock_connect/sock_read, not a total budget. + + A total-only budget makes a connect-phase timeout raise a bare + ``TimeoutError`` indistinguishable from a read timeout; per-phase config + makes connect raise ``ConnectionTimeoutError`` so it maps to a request error. + """ + session = _CaptureTimeoutSession() + client = AiohttpHttpClient(timeout=5.0, session=session) # type: ignore[arg-type] + with pytest.raises(ServiceRequestTimeoutError): + await client.execute(Request(method=Method.GET, url=Url.parse("http://example.test/"))) + cfg = session.captured + assert cfg is not None + assert cfg.sock_connect == 5.0 + assert cfg.sock_read == 5.0 + assert cfg.total is None + + +async def test_read_timeout_maps_to_response_timeout(base_url: str) -> None: + """A read-phase (sock_read) timeout maps to ServiceResponseTimeoutError.""" + async with AiohttpHttpClient(timeout=0.25) as client: + with pytest.raises(ServiceResponseTimeoutError): + await client.execute(Request(method=Method.GET, url=Url.parse(f"{base_url}/slow"))) diff --git a/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/async_.py b/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/async_.py index b85d220..eff2f1b 100644 --- a/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/async_.py +++ b/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/async_.py @@ -1,15 +1,15 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Async ``HttpClient`` implementation built on :class:`httpx.AsyncClient`. +"""Async ``HttpClient`` implementation built on `httpx.AsyncClient`. Supports streaming uploads and downloads, per-phase timeouts (connect / -read / write / pool), HTTP/2 (opt-in via :mod:`httpx`), and proxies. +read / write / pool), HTTP/2 (opt-in via `httpx`), and proxies. Production-grade alternative to the asyncio reference client shipped in ``dexpace-sdk-http-stdlib``. -The transport delegates to :class:`httpx.AsyncClient`, streaming the -request body via :meth:`AsyncRequestBody.aiter_bytes` (when provided) or +The transport delegates to `httpx.AsyncClient`, streaming the +request body via `AsyncRequestBody.aiter_bytes` (when provided) or the sync ``RequestBody.iter_bytes`` (drained eagerly into bytes; httpx accepts both). The response exposes ``AsyncResponseBody.from_async_stream`` wrapping httpx's ``aiter_bytes`` iterator. @@ -40,11 +40,11 @@ class AsyncHttpxHttpClient: - """Async transport over :class:`httpx.AsyncClient`. + """Async transport over `httpx.AsyncClient`. Per-phase timeouts (``connect_timeout``, ``read_timeout``, ``write_timeout``, ``pool_timeout``) are forwarded to - :class:`httpx.Timeout`. ``None`` disables the phase's timeout. + `httpx.Timeout`. ``None`` disables the phase's timeout. Args: connect_timeout: Seconds allowed for connection establishment. @@ -54,9 +54,9 @@ class AsyncHttpxHttpClient: request body. pool_timeout: Seconds allowed to acquire a connection from the pool. - transport: Optional :class:`httpx.AsyncBaseTransport`; the primary + transport: Optional `httpx.AsyncBaseTransport`; the primary extension hook for tests (``httpx.MockTransport``). - client: Optional pre-built :class:`httpx.AsyncClient` — overrides + client: Optional pre-built `httpx.AsyncClient` — overrides the timeout / transport kwargs entirely. Ownership transfers to this transport. """ diff --git a/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/sync.py b/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/sync.py index 8c3d9b4..348f5c9 100644 --- a/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/sync.py +++ b/packages/dexpace-sdk-http-httpx/src/dexpace/sdk/http/httpx/sync.py @@ -1,16 +1,16 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Synchronous ``HttpClient`` implementation built on :mod:`httpx`. +"""Synchronous ``HttpClient`` implementation built on `httpx`. Supports streaming uploads and downloads, per-phase timeouts (connect / -read / write / pool), HTTP/2 (opt-in via :mod:`httpx`), and proxies. +read / write / pool), HTTP/2 (opt-in via `httpx`), and proxies. Production-grade alternative to the urllib reference client shipped in ``dexpace-sdk-http-stdlib``. -The transport delegates to :class:`httpx.Client`, streaming the request -body via :meth:`RequestBody.iter_bytes` and exposing the response as a -:class:`ResponseBody` whose ``iter_bytes`` walks the httpx response's own +The transport delegates to `httpx.Client`, streaming the request +body via `RequestBody.iter_bytes` and exposing the response as a +`ResponseBody` whose ``iter_bytes`` walks the httpx response's own ``iter_bytes`` iterator. Closing the SDK response closes the underlying httpx response and releases the connection back to the pool. """ @@ -40,7 +40,7 @@ class HttpxHttpClient: - """Synchronous transport over :class:`httpx.Client`. + """Synchronous transport over `httpx.Client`. Behaves as a structural ``HttpClient`` (single ``execute`` method) plus a context-manager surface so a ``Pipeline`` can take ownership and call @@ -48,7 +48,7 @@ class HttpxHttpClient: Per-phase timeouts (``connect_timeout``, ``read_timeout``, ``write_timeout``, ``pool_timeout``) are forwarded to - :class:`httpx.Timeout`. ``None`` disables the phase's timeout. + `httpx.Timeout`. ``None`` disables the phase's timeout. Args: connect_timeout: Seconds allowed for connection establishment. @@ -58,9 +58,9 @@ class HttpxHttpClient: request body. pool_timeout: Seconds allowed to acquire a connection from the pool. - transport: Optional :class:`httpx.BaseTransport`; the primary + transport: Optional `httpx.BaseTransport`; the primary extension hook for tests (``httpx.MockTransport``). - client: Optional pre-built :class:`httpx.Client` — overrides the + client: Optional pre-built `httpx.Client` — overrides the timeout / transport kwargs entirely. Ownership transfers to this transport. """ diff --git a/packages/dexpace-sdk-http-httpx/tests/test_async_httpx_client.py b/packages/dexpace-sdk-http-httpx/tests/test_async_httpx_client.py index b38862f..77eb43b 100644 --- a/packages/dexpace-sdk-http-httpx/tests/test_async_httpx_client.py +++ b/packages/dexpace-sdk-http-httpx/tests/test_async_httpx_client.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Tests for ``AsyncHttpxHttpClient`` using :class:`httpx.MockTransport`.""" +"""Tests for ``AsyncHttpxHttpClient`` using `httpx.MockTransport`.""" from __future__ import annotations diff --git a/packages/dexpace-sdk-http-httpx/tests/test_httpx_client.py b/packages/dexpace-sdk-http-httpx/tests/test_httpx_client.py index 82fc6c9..6bc8718 100644 --- a/packages/dexpace-sdk-http-httpx/tests/test_httpx_client.py +++ b/packages/dexpace-sdk-http-httpx/tests/test_httpx_client.py @@ -1,7 +1,7 @@ # Copyright (c) 2026 dexpace and Omar Aljarrah. # Licensed under the MIT License. See LICENSE.md in the repository root for details. -"""Tests for ``HttpxHttpClient`` using :class:`httpx.MockTransport`.""" +"""Tests for ``HttpxHttpClient`` using `httpx.MockTransport`.""" from __future__ import annotations diff --git a/packages/dexpace-sdk-http-requests/src/dexpace/sdk/http/requests/client.py b/packages/dexpace-sdk-http-requests/src/dexpace/sdk/http/requests/client.py index e18fd6a..fa598a5 100644 --- a/packages/dexpace-sdk-http-requests/src/dexpace/sdk/http/requests/client.py +++ b/packages/dexpace-sdk-http-requests/src/dexpace/sdk/http/requests/client.py @@ -5,15 +5,20 @@ ``RequestsHttpClient`` wraps a ``requests.Session`` configured with ``stream=True`` so response bodies are read lazily and surfaced through the -SDK's :class:`ResponseBody` streaming API. Request bodies are produced via -:meth:`RequestBody.iter_bytes` in 8 KiB chunks. +SDK's `ResponseBody` streaming API. Request bodies are produced via +`RequestBody.iter_bytes` in 8 KiB chunks. Exception mapping (``requests`` -> SDK): -- ``requests.ConnectTimeout`` -> :class:`ServiceRequestTimeoutError` -- ``requests.ReadTimeout`` -> :class:`ServiceResponseTimeoutError` -- ``requests.ConnectionError`` -> :class:`ServiceRequestError` -- ``requests.RequestException`` (catch-all) -> :class:`ServiceRequestError` +- ``requests.ConnectTimeout`` -> `ServiceRequestTimeoutError` +- ``requests.ReadTimeout`` -> `ServiceResponseTimeoutError` +- ``requests.ConnectionError`` -> `ServiceRequestError` +- ``requests.RequestException`` (catch-all) -> `ServiceRequestError` + +Failures that surface later, while the response body is being streamed, are +classified as response-side errors (the request was already sent): a +``requests.Timeout`` -> `ServiceResponseTimeoutError` and any other +``requests.RequestException`` -> `ServiceResponseError`. """ from __future__ import annotations @@ -50,7 +55,7 @@ class RequestsHttpClient: a context-manager surface so a ``Pipeline`` can take ownership and call ``close`` on exit. Each call sends one ``requests`` request with ``stream=True`` and wraps the streamed response into a - :class:`ResponseBody`. + `ResponseBody`. Attributes: timeout: Single timeout in seconds applied to ``Session.request``; @@ -145,6 +150,7 @@ def _build_response(request: Request, raw: requests.Response) -> Response: try: status = Status(raw.status_code) except ValueError as err: + raw.close() raise ServiceResponseError(f"Unknown status code: {raw.status_code}", error=err) from err headers = Headers(list(raw.headers.items())) body = ResponseBody.from_stream(_IterContentStream(raw)) # type: ignore[arg-type] @@ -162,7 +168,7 @@ def _build_response(request: Request, raw: requests.Response) -> Response: class _IterContentStream: """Adapter that exposes ``requests.Response.iter_content`` as a stream. - :class:`ResponseBody.from_stream` calls ``read(chunk_size)`` and + `ResponseBody.from_stream` calls ``read(chunk_size)`` and ``close()`` on its argument. ``requests`` doesn't expose a file-like object that honours chunk-size hints once decoded, but ``iter_content`` does — this adapter buffers what the iterator yields and serves it in @@ -182,17 +188,16 @@ def read(self, size: int = -1) -> bytes: return b"" if self._iter is None: self._iter = self._response.iter_content(chunk_size=_CHUNK_SIZE) - if size is None or size < 0: - for chunk in self._iter: + if size < 0: + while (chunk := self._next_chunk()) is not None: if chunk: self._buf.extend(chunk) out = bytes(self._buf) self._buf.clear() return out while len(self._buf) < size: - try: - chunk = next(self._iter) - except StopIteration: + chunk = self._next_chunk() + if chunk is None: break if chunk: self._buf.extend(chunk) @@ -203,6 +208,24 @@ def read(self, size: int = -1) -> bytes: del self._buf[:take] return out + def _next_chunk(self) -> bytes | None: + """Pull the next body chunk, mapping read-phase failures to SDK errors. + + Returns the chunk, or ``None`` at end of stream. The request is already + on the wire, so a read-phase failure is a response-side error: a + ``requests`` read timeout becomes ``ServiceResponseTimeoutError`` and + any other transport failure mid-body becomes ``ServiceResponseError``. + """ + assert self._iter is not None + try: + return next(self._iter) + except StopIteration: + return None + except requests.Timeout as err: + raise ServiceResponseTimeoutError("Response body read timed out", error=err) from err + except requests.RequestException as err: + raise ServiceResponseError(f"Response body read failed: {err}", error=err) from err + def close(self) -> None: if self._closed: return diff --git a/packages/dexpace-sdk-http-requests/tests/test_requests_client.py b/packages/dexpace-sdk-http-requests/tests/test_requests_client.py index d0822cd..d5f77d9 100644 --- a/packages/dexpace-sdk-http-requests/tests/test_requests_client.py +++ b/packages/dexpace-sdk-http-requests/tests/test_requests_client.py @@ -5,15 +5,18 @@ from __future__ import annotations +import io import socketserver import threading import time from collections.abc import Iterator import pytest +import requests from dexpace.sdk.core.errors import ( ServiceRequestError, + ServiceResponseError, ServiceResponseTimeoutError, ) from dexpace.sdk.core.http.common import Url @@ -187,3 +190,109 @@ def test_closed_client_raises() -> None: def test_invalid_timeout_raises() -> None: with pytest.raises(ValueError): RequestsHttpClient(timeout=0) + + +class _UnknownStatusAdapter(requests.adapters.BaseAdapter): + """Transport adapter that returns a 999 response with a tracked close.""" + + def __init__(self, closed: dict[str, bool]) -> None: + super().__init__() + self._closed = closed + + def send( # signature mirrors BaseAdapter.send + self, + request: requests.PreparedRequest, + stream: bool = False, + timeout: float | tuple[float | None, float | None] | None = None, + verify: bool | str = True, + cert: str | tuple[str, str] | None = None, + proxies: dict[str, str] | None = None, + ) -> requests.Response: + response = requests.Response() + response.status_code = 999 + response.reason = "Bonkers" + response.url = request.url or "" + response.raw = io.BytesIO(b"") + tracker = self._closed + + def _close() -> None: + tracker["yes"] = True + + # ``requests.Response.close`` releases ``raw``; record the call. + response.close = _close # type: ignore[method-assign] + return response + + def close(self) -> None: # pragma: no cover - nothing to release + pass + + +def test_unknown_status_closes_response() -> None: + """When Status() rejects an unknown code, the response must be released.""" + closed = {"yes": False} + session = requests.Session() + session.mount("http://", _UnknownStatusAdapter(closed)) + client = RequestsHttpClient(session=session) + request = Request(method=Method.GET, url=Url.parse("http://example.test/")) + with client, pytest.raises(ServiceResponseError): + client.execute(request) + assert closed["yes"], "Response should be closed when status mapping fails" + + +class _BodyFailureAdapter(requests.adapters.BaseAdapter): + """Returns a 200 whose body stream raises ``exc`` partway through the read.""" + + def __init__(self, exc: Exception) -> None: + super().__init__() + self._exc = exc + + def send( # signature mirrors BaseAdapter.send + self, + request: requests.PreparedRequest, + stream: bool = False, + timeout: float | tuple[float | None, float | None] | None = None, + verify: bool | str = True, + cert: str | tuple[str, str] | None = None, + proxies: dict[str, str] | None = None, + ) -> requests.Response: + response = requests.Response() + response.status_code = 200 + response.reason = "OK" + response.url = request.url or "" + response.raw = io.BytesIO(b"") + exc = self._exc + + def _raising_iter(chunk_size: int = 1, decode_unicode: bool = False) -> Iterator[bytes]: + yield b"partial" + raise exc + + response.iter_content = _raising_iter # type: ignore[method-assign, assignment] + return response + + def close(self) -> None: # pragma: no cover - nothing to release + pass + + +def test_body_read_chunked_error_maps_to_service_response_error() -> None: + """A mid-body ``ChunkedEncodingError`` surfaces as ServiceResponseError.""" + session = requests.Session() + session.mount("http://", _BodyFailureAdapter(requests.exceptions.ChunkedEncodingError("boom"))) + client = RequestsHttpClient(session=session) + request = Request(method=Method.GET, url=Url.parse("http://example.test/")) + with client: + response = client.execute(request) + assert response.body is not None + with pytest.raises(ServiceResponseError): + response.body.bytes() + + +def test_body_read_timeout_maps_to_service_response_timeout_error() -> None: + """A mid-body ``ReadTimeout`` surfaces as ServiceResponseTimeoutError.""" + session = requests.Session() + session.mount("http://", _BodyFailureAdapter(requests.exceptions.ReadTimeout("slow"))) + client = RequestsHttpClient(session=session) + request = Request(method=Method.GET, url=Url.parse("http://example.test/")) + with client: + response = client.execute(request) + assert response.body is not None + with pytest.raises(ServiceResponseTimeoutError): + response.body.bytes() diff --git a/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/asyncio_http_client.py b/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/asyncio_http_client.py index 1f09e6a..aeee8b9 100644 --- a/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/asyncio_http_client.py +++ b/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/asyncio_http_client.py @@ -15,14 +15,17 @@ - ``Content-Length``-framed responses (no chunked transfer-encoding) - Connection: close (one request per connection) -Additional limitations that mirror :mod:`urllib_http_client`: +Additional limitations that mirror `urllib_http_client`: - **No streaming uploads.** The request body is fully buffered into memory via ``b"".join(request.body.iter_bytes())`` before send. For streaming uploads, plug in an alternative transport (``httpx``, ``aiohttp``). - **Coarse timeouts.** A single ``timeout`` value is applied to connect, status-line read, header read, and body read. Production transports - expose per-phase granularity. + expose per-phase granularity. A connect-phase timeout surfaces as + ``ServiceRequestTimeoutError``; a timeout during any read phase + (status line, headers, or body) surfaces as + ``ServiceResponseTimeoutError``. - **Multi-value request headers are emitted as repeated lines** (one ``Name: Value`` line per value), which is the wire-correct form on the async side — note this differs from the urllib reference client, which @@ -45,6 +48,7 @@ ServiceRequestError, ServiceRequestTimeoutError, ServiceResponseError, + ServiceResponseTimeoutError, ) from dexpace.sdk.core.http.common.headers import Headers from dexpace.sdk.core.http.common.protocol import Protocol @@ -94,6 +98,31 @@ async def execute(self, request: Request) -> AsyncResponse: try: await self._send(writer, request, host, path) return await self._read(reader, request) + except TimeoutError as err: + # The connection was established (``_open`` already maps connect + # timeouts), so a timeout here is a read/write-phase stall. + raise ServiceResponseTimeoutError( + f"Read from {host}:{port} timed out", error=err + ) from err + except (asyncio.IncompleteReadError, ValueError) as err: + # Mid-body drop (``readexactly``) or an over-limit line + # (``readline`` re-raises ``LimitOverrunError`` as ``ValueError``). + raise ServiceResponseError( + f"Reading response from {host}:{port} failed: {err}", error=err + ) from err + except ServiceResponseError: + raise + except OSError as err: + # A mid-exchange socket error after a successful connect is a + # response-side failure, not a connect failure: the request may + # already be on the wire, so it is deliberately classified as a + # response error (not retry-safe) rather than the request error the + # httpx / aiohttp / requests adapters use for their generic + # transport bucket. The conservative choice avoids blind-retrying a + # potentially-received non-idempotent request. + raise ServiceResponseError( + f"Exchange with {host}:{port} failed: {err}", error=err + ) from err finally: writer.close() with contextlib.suppress(ConnectionError, OSError): @@ -142,7 +171,11 @@ async def _send( body_bytes = b"" if request.body is not None: body_bytes = b"".join(request.body.iter_bytes()) - headers = Headers(request.headers.items()).with_set("Host", host) + headers = Headers(request.headers.items()) + if "host" not in headers: + # Preserve a caller-supplied Host (virtual hosting); only derive + # it from the URL when the request did not set one. + headers = headers.with_set("Host", host) if "content-length" not in headers and body_bytes: headers = headers.with_set("Content-Length", str(len(body_bytes))) headers = headers.with_set("Connection", "close") diff --git a/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/urllib_http_client.py b/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/urllib_http_client.py index 8766102..b244c6c 100644 --- a/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/urllib_http_client.py +++ b/packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/urllib_http_client.py @@ -26,14 +26,14 @@ outbound) and, in proxy/forwarder scenarios, ``WWW-Authenticate``. Use a production transport if you need to emit repeated outbound headers. -The response body is exposed as a :class:`ResponseBody.from_stream` wrapper +The response body is exposed as a `ResponseBody.from_stream` wrapper so streaming reads on the response side are possible. Maps urllib's exception types into the SDK error hierarchy. """ from __future__ import annotations -from socket import timeout as _SocketTimeout # noqa: N812 — re-exporting the stdlib lowercase name +import contextlib from types import TracebackType from typing import Final, Self from urllib.error import HTTPError, URLError @@ -87,23 +87,34 @@ def execute(self, request: Request) -> Response: Raises: ServiceRequestError: When the connection cannot be established. + ServiceRequestTimeoutError: When the connection times out before + the request is transmitted (connect phase). ServiceResponseError: When reading the response fails. + ServiceResponseTimeoutError: When the request was transmitted but + the response read times out (read phase). """ if self._closed: raise ServiceRequestError("UrllibHttpClient is closed") raw = _build_urllib_request(request) try: opened = urlopen(raw, timeout=self.timeout) - except TimeoutError as err: - raise ServiceRequestTimeoutError(str(err), error=err) from err except HTTPError as err: # urllib raises HTTPError for 4xx/5xx; surface them via the # normal Response path so policies (retry, error_map) can react. return _build_response(request, err) except URLError as err: - if isinstance(err.reason, _SocketTimeout): - raise ServiceResponseTimeoutError(str(err), error=err) from err + # Since 3.10 ``socket.timeout`` is ``TimeoutError``. A connect + # timeout reaches us wrapped as ``URLError(reason=TimeoutError)`` + # — the request never left, so it is a *request* timeout. + if isinstance(err.reason, TimeoutError): + raise ServiceRequestTimeoutError(str(err), error=err) from err raise ServiceRequestError(str(err), error=err) from err + except TimeoutError as err: + # A *bare* ``TimeoutError`` (not wrapped in ``URLError``) is a + # read-phase timeout: the request was transmitted but the response + # stalled. Classify as a *response* timeout so non-idempotent + # reads are not auto-retried as if they never reached the service. + raise ServiceResponseTimeoutError(str(err), error=err) from err return _build_response(request, opened) def close(self) -> None: @@ -146,6 +157,10 @@ def _build_response(request: Request, opened: object) -> Response: try: status = Status(status_code) except ValueError as err: + # A valid-but-unregistered code raises here before the body wraps the + # stream. Release the underlying response first so the connection is + # not leaked (parity with the aiohttp/httpx adapters). + _close_quietly(opened) raise ServiceResponseError(f"Unknown status code: {status_code}", error=err) from err raw_headers = getattr(opened, "headers", None) headers = _convert_headers(raw_headers) @@ -161,6 +176,21 @@ def _build_response(request: Request, opened: object) -> Response: ) +def _close_quietly(opened: object) -> None: + """Close ``opened`` if it exposes ``close``, swallowing any error. + + Used on the failure path before raising so a partially constructed + response does not leak its underlying connection. + + Args: + opened: The urllib response (or ``HTTPError``) to release. + """ + close = getattr(opened, "close", None) + if callable(close): + with contextlib.suppress(Exception): + close() + + def _convert_headers(raw: object) -> Headers: if raw is None: return Headers() diff --git a/packages/dexpace-sdk-http-stdlib/tests/test_asyncio_http_client.py b/packages/dexpace-sdk-http-stdlib/tests/test_asyncio_http_client.py index 0fdd449..c43543a 100644 --- a/packages/dexpace-sdk-http-stdlib/tests/test_asyncio_http_client.py +++ b/packages/dexpace-sdk-http-stdlib/tests/test_asyncio_http_client.py @@ -6,16 +6,22 @@ from __future__ import annotations import asyncio -from collections.abc import AsyncIterator +from collections.abc import AsyncIterator, Awaitable, Callable import pytest -from dexpace.sdk.core.errors import ServiceRequestError +from dexpace.sdk.core.errors import ( + ServiceRequestError, + ServiceResponseError, + ServiceResponseTimeoutError, +) from dexpace.sdk.core.http.common import Url from dexpace.sdk.core.http.request import Method, Request from dexpace.sdk.core.http.response import Status from dexpace.sdk.http.stdlib import AsyncioHttpClient +_Handler = Callable[[asyncio.StreamReader, asyncio.StreamWriter], Awaitable[None]] + async def _handle_ok( reader: asyncio.StreamReader, @@ -72,3 +78,118 @@ async def test_connect_failure() -> None: def test_invalid_timeout_raises() -> None: with pytest.raises(ValueError): AsyncioHttpClient(timeout=0) + + +async def _serve(handler: _Handler) -> AsyncIterator[str]: + srv = await asyncio.start_server(handler, "127.0.0.1", 0) + socks = srv.sockets + assert socks, "server has no listening sockets" + port = int(socks[0].getsockname()[1]) + try: + yield f"http://127.0.0.1:{port}" + finally: + srv.close() + await srv.wait_closed() + + +async def _read_request_head(reader: asyncio.StreamReader) -> list[str]: + """Drain the request line and headers, returning the decoded lines.""" + lines: list[str] = [] + while True: + line = await reader.readline() + if not line or line == b"\r\n": + break + lines.append(line.decode("latin-1").rstrip("\r\n")) + return lines + + +async def test_caller_host_header_is_preserved() -> None: + # A caller-supplied Host must survive (virtual hosting); the transport + # must not overwrite it with the bare URL host. + seen: dict[str, str] = {} + + async def echo_host(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + for header in await _read_request_head(reader): + name, _, value = header.partition(":") + if name.strip().lower() == "host": + seen["host"] = value.strip() + body = b'{"ok":true}' + writer.write( + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: " + str(len(body)).encode() + b"\r\n" + b"Connection: close\r\n\r\n" + body + ) + await writer.drain() + writer.close() + + gen = _serve(echo_host) + base = await anext(gen) + try: + request = Request(method=Method.GET, url=Url.parse(f"{base}/")) + request = request.with_header("Host", "virtual.example.com") + async with AsyncioHttpClient(timeout=5.0) as client: + await client.execute(request) + finally: + with pytest.raises(StopAsyncIteration): + await anext(gen) + assert seen.get("host") == "virtual.example.com" + + +async def test_partial_body_maps_to_service_response_error() -> None: + # Server promises more bytes than it sends, then drops the connection. + async def short_body(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await _read_request_head(reader) + writer.write( + b"HTTP/1.1 200 OK\r\nContent-Length: 100\r\nConnection: close\r\n\r\nonly-a-few-bytes" + ) + await writer.drain() + writer.close() + + gen = _serve(short_body) + base = await anext(gen) + try: + async with AsyncioHttpClient(timeout=5.0) as client: + with pytest.raises(ServiceResponseError): + await client.execute(Request(method=Method.GET, url=Url.parse(f"{base}/"))) + finally: + with pytest.raises(StopAsyncIteration): + await anext(gen) + + +async def test_oversized_header_maps_to_service_response_error() -> None: + # A header line longer than the reader limit makes readline raise a bare + # ValueError; it must surface as a ServiceResponseError, not leak. + async def oversized(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await _read_request_head(reader) + giant = b"X-Big: " + b"a" * (128 * 1024) + writer.write(b"HTTP/1.1 200 OK\r\n" + giant + b"\r\n\r\n") + await writer.drain() + writer.close() + + gen = _serve(oversized) + base = await anext(gen) + try: + async with AsyncioHttpClient(timeout=5.0) as client: + with pytest.raises(ServiceResponseError): + await client.execute(Request(method=Method.GET, url=Url.parse(f"{base}/"))) + finally: + with pytest.raises(StopAsyncIteration): + await anext(gen) + + +async def test_read_timeout_maps_to_service_response_timeout() -> None: + # Server accepts and reads the request, then stalls before replying. + async def stall(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await _read_request_head(reader) + await asyncio.sleep(5.0) + writer.close() + + gen = _serve(stall) + base = await anext(gen) + try: + async with AsyncioHttpClient(timeout=0.3) as client: + with pytest.raises(ServiceResponseTimeoutError): + await client.execute(Request(method=Method.GET, url=Url.parse(f"{base}/"))) + finally: + with pytest.raises(StopAsyncIteration): + await anext(gen) diff --git a/packages/dexpace-sdk-http-stdlib/tests/test_urllib_http_client.py b/packages/dexpace-sdk-http-stdlib/tests/test_urllib_http_client.py index c6ca44d..4526d27 100644 --- a/packages/dexpace-sdk-http-stdlib/tests/test_urllib_http_client.py +++ b/packages/dexpace-sdk-http-stdlib/tests/test_urllib_http_client.py @@ -5,17 +5,25 @@ from __future__ import annotations +import contextlib import socketserver import threading +import time from collections.abc import Iterator import pytest -from dexpace.sdk.core.errors import ServiceRequestError +from dexpace.sdk.core.errors import ( + ServiceRequestError, + ServiceRequestTimeoutError, + ServiceResponseError, + ServiceResponseTimeoutError, +) from dexpace.sdk.core.http.common import Url from dexpace.sdk.core.http.request import Method, Request, RequestBody from dexpace.sdk.core.http.response import Status from dexpace.sdk.http.stdlib import UrllibHttpClient +from dexpace.sdk.http.stdlib import urllib_http_client as _urllib_mod def _build_response(method: str, body_len: int) -> bytes: @@ -124,3 +132,76 @@ def test_connect_error_maps_to_service_request_error() -> None: request = Request(method=Method.GET, url=Url.parse("http://127.0.0.1:1/")) with pytest.raises(ServiceRequestError): client.execute(request) + + +class _StallHandler(socketserver.BaseRequestHandler): + """Accepts the connection, reads the request, then never replies. + + Models a read-phase timeout: the TCP connection is established (so the + request is fully transmitted) but the server stalls before sending a + status line, so ``urlopen`` times out during the read phase. + """ + + def handle(self) -> None: + with contextlib.suppress(OSError): + self.request.recv(65536) + time.sleep(5.0) + + +@pytest.fixture +def stall_server() -> Iterator[str]: + """Start a server that accepts then stalls; yield its base URL.""" + server = _TestServer(("127.0.0.1", 0), _StallHandler) + port = int(server.server_address[1]) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{port}" + finally: + server.shutdown() + server.server_close() + + +def test_read_timeout_maps_to_service_response_timeout(stall_server: str) -> None: + # Accept-then-stall: the request was sent but the response never arrives. + # Since 3.10 this surfaces as a bare TimeoutError, which must classify as + # a *response* timeout (not request) so non-idempotent reads are not + # silently retried. + client = UrllibHttpClient(timeout=0.5) + request = Request(method=Method.GET, url=Url.parse(f"{stall_server}/")) + with pytest.raises(ServiceResponseTimeoutError): + client.execute(request) + + +def test_connect_timeout_maps_to_service_request_timeout() -> None: + # Blackhole connect: 10.255.255.1 is non-routable, so the connection is + # never established. urllib surfaces this as URLError(reason=TimeoutError), + # which must classify as a *request* timeout (retry-safe). + client = UrllibHttpClient(timeout=0.5) + request = Request(method=Method.GET, url=Url.parse("http://10.255.255.1:81/")) + with pytest.raises(ServiceRequestTimeoutError): + client.execute(request) + + +class _TrackingResponse: + """Stand-in for urllib's HTTPResponse that records whether it was closed.""" + + def __init__(self, status_code: int) -> None: + self.status = status_code + self.headers = None + self.reason = "Weird" + self.closed = False + + def close(self) -> None: + self.closed = True + + +def test_unregistered_status_closes_response_before_raising() -> None: + # A valid-but-unregistered status code makes Status(code) raise. The + # underlying response must be released first so the connection is not + # leaked. + opened = _TrackingResponse(599) + request = Request(method=Method.GET, url=Url.parse("http://127.0.0.1/")) + with pytest.raises(ServiceResponseError): + _urllib_mod._build_response(request, opened) + assert opened.closed is True diff --git a/tools/surface_baseline.json b/tools/surface_baseline.json index 39101ea..30317eb 100644 --- a/tools/surface_baseline.json +++ b/tools/surface_baseline.json @@ -368,7 +368,7 @@ "end": "end(self) -> int | None", "format": "format(self) -> str", "format_many": "format_many(cls, ranges: Sequence[HttpRange]) -> str", - "suffix": "suffix(cls, count: int) -> _SuffixHttpRange", + "suffix": "suffix(cls, count: int) -> HttpRange", "to_header_value": "to_header_value(self) -> str" } } @@ -469,7 +469,9 @@ }, "dexpace.sdk.core.http.context.call_context": { "CallContext": { - "bases": [], + "bases": [ + "abc.ABC" + ], "methods": { "close": "close(self) -> None" } @@ -1268,10 +1270,6 @@ } }, "dexpace.sdk.core.pipeline.step.config": { - "RetryConfig": { - "bases": [], - "methods": {} - }, "StepMetadata": { "bases": [], "methods": {} @@ -1586,7 +1584,6 @@ "Policy", "RequestPipelineStep", "ResponsePipelineStep", - "RetryConfig", "Stage", "StagedPipelineBuilder", "StepMetadata",