Skip to content

Transport adapters re-derive framing, redirect, status, and error-classification independently — consider shared SPI helpers plus a transport conformance suite #60

@OmarAlJarrah

Description

@OmarAlJarrah

Current design

core exposes the transport seam as a single-method HttpClient / AsyncHttpClient Protocol (execute(request) -> Response) and ships no scaffolding beneath it. Each of the four transport packages implements the same cross-cutting obligations from scratch, and they have reached different answers on several of them:

  • Redirect disabling. stdlib needs a custom handler (_NoRedirectHandler in packages/dexpace-sdk-http-stdlib/src/dexpace/sdk/http/stdlib/urllib_http_client.py:76), while requests and aiohttp pass allow_redirects=False (packages/dexpace-sdk-http-requests/src/dexpace/sdk/http/requests/client.py:143, packages/dexpace-sdk-http-aiohttp/src/dexpace/sdk/http/aiohttp/client.py:108).
  • Request-body framing (Content-Length vs chunked). The same "known length -> length-frame, unknown -> chunked, never carry both" rule is re-derived four times: _frame_known_length (.../httpx/sync.py:274), _frame_length (.../aiohttp/client.py:155), _SizedBody (.../requests/client.py:199), and a full-buffer path in urllib (.../urllib_http_client.py:192).
  • Multi-value request headers. urllib and requests flatten to ", "-joined values (urllib_http_client.py:202, requests/client.py:128); asyncio emits repeated Name: Value lines (.../stdlib/asyncio_http_client.py:215); aiohttp emits repeated pairs (.../aiohttp/client.py:180). Each adapter documents this as deliberate, but the upshot is that the same outbound Headers value goes on the wire differently depending on which transport is plugged in.
  • Connect-vs-read error classification. This is bespoke per library, and the references disagree. The asyncio adapter explicitly documents that it classifies a generic mid-exchange OSError as a response error, "not the request error the httpx / aiohttp / requests adapters use for their generic transport bucket" (asyncio_http_client.py:132). That is a real behavioural fork: a generic transport failure makes a non-idempotent request retry-safe under three transports and not under the fourth.
  • Out-of-range status + handle release. The "synthesize in-range Status, release the handle and raise on out-of-range" logic is copy-pasted into all four _build_response / _wrap_response helpers (e.g. urllib_http_client.py:211, httpx/sync.py:219, aiohttp/client.py:239, requests/client.py:222).

Trade-off / concern

A minimal Protocol keeps the public seam tiny, but it pushes the real transport contract entirely into prose and reviewer discipline. There is no executable statement of what a conforming transport must do: disable redirects, never carry both framing headers, classify a connect timeout as a request error and a read timeout as a response error, preserve in-range unnamed status codes, release the handle before raising. Two consequences follow from the current code:

  1. The error-classification fork is a retry-safety surface, not cosmetics. Identical SDK code retries a non-idempotent request differently depending on the plugged-in transport. CHANGELOG.md (lines 14-17) lists "consistent connect- vs read-phase timeout classification" as something the adapters were given this round, yet the generic-error bucket is documented as intentionally inconsistent — worth deciding whether that inconsistency is the contract or a gap.
  2. Every future third-party transport must rediscover all of the above by reading four existing adapters, and the four already disagree on two points (header flattening, generic-error classification), so "match the others" is ambiguous guidance.

Proposed direction

Add a module-private helper layer in core (SPI-only, not part of the public surface) for the pieces that are genuinely hard to get right and identical in intent across transports:

  • a framing helper that decides Content-Length-vs-chunked from body.content_length() and guarantees the two framing headers are mutually exclusive;
  • a status mapper that preserves in-range codes and signals out-of-range so the adapter can release its handle and raise;
  • a documented error-classification helper (connect -> request error, read -> response error, and a single agreed answer for the generic mid-exchange case).

Pair it with a published transport conformance suite in core that each transport package runs against its own client. core already ships packages/dexpace-sdk-core/tests/test_conformance_fixtures.py, which pins core's internal seams ("one observable check per seam") but stops exactly at the transport boundary and is imported by no transport package. Extending that philosophy to the transport contract would replace four prose contracts that already diverge with one machine-checkable one, and give a new adapter a green/red target instead of four references to imitate.

Trade-off: helpers in core risk leaking library-specific assumptions back into the toolkit, and any new symbol widens the surface the narrow-API convention guards. Keeping them module-private / SPI-only contains that, and the conformance suite is test-only so it adds no runtime surface at all. The framing/classification logic would then have one place to fix rather than four.

Acknowledging the current rationale

CLAUDE.md is explicit that "Structural duck-typed seams (HttpClient, Serde, PipelineStep) are typing.Protocol" and that "the public API is narrow... concrete adapter classes are module-private" — a deliberate choice to keep the seam minimal and avoid a base-class hierarchy. This proposal does not argue against that: the Protocol stays the public contract and stays single-method. The suggestion is that "no Protocol method beyond execute" need not imply "no shared, non-public scaffolding" — the hard-to-get-right obligations can live in core as SPI helpers and an executable conformance suite without growing the public API or reintroducing a mandatory base class. Given that the existing four adapters already disagree on a retry-safety-relevant behaviour, it seems worth deciding whether the cross-transport contract should be specified in code rather than left to per-adapter prose.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestquestionFurther information is requested

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions