Skip to content

Drive sync/async policy parity from a shared sans-io core instead of hand-written twins #51

@OmarAlJarrah

Description

@OmarAlJarrah

Current design

Async support is delivered by hand-writing a parallel twin for nearly every async-touching policy and pipeline type, rather than driving both surfaces from one shared description of the control flow.

  • pipeline/pipeline.py and pipeline/async_pipeline.py carry near-identical __init__, _wire_chain, and run/async run. The _wire_chain helper is duplicated verbatim across the two modules.
  • Each stateful policy has a twin: policies/{retry,redirect,idempotency,set_date,client_identity}.py plus tracing_policy.py, each with an async_*.py counterpart.

The sharing discipline across those twins is non-uniform:

  • AsyncRetryPolicy (policies/async_retry.py) wraps a sync RetryPolicy as self.config and delegates the pure decision helpers to it — but re-copies the entire dispatch loop. Its send (roughly lines 155–194) is a line-for-line transcription of RetryPolicy.send (policies/retry.py, roughly lines 232–268), with await inserted. To do so it reaches into five underscore-private methods on the wrapped instance: cfg._configure_settings, cfg._decrement_for_error, cfg._is_retry, cfg._decrement_status, cfg._delay_for. Module-private methods have become a de-facto cross-module contract.
  • AsyncRedirectPolicy (policies/async_redirect.py) follows the same pattern — delegates per-hop construction to cfg._build_next_request but re-copies the hop loop.
  • AsyncSetDatePolicy and AsyncOperationTracingPolicy copy the whole body, not just the loop.

The duplicated surface is exactly the part most prone to silent divergence: control flow, resource-cleanup ordering (close-before-raise, close-before-sleep), and error classification.

A second, narrower issue rides along: AsyncRetryPolicy.__init__ re-lists all sixteen retry parameters with their own default literals (total_retries=10, backoff_factor=0.8, timeout=604_800, backoff_max=120.0, ...), kept in agreement with RetryPolicy.__init__ only by hand. Nothing — no test, no type — ties the two default sets together. (The idempotency and client-identity twins avoid this by sharing module-level constant defaults; retry and, to a lesser extent, redirect do not.)

Trade-off / concern

The codebase pays the full duplication cost — two copies of every control-flow path that can drift — while only partially banking the de-duplication benefit, since the decision and classification helpers are shared but the loop around them is not.

There is direct evidence that this drift is a recurring maintenance reality rather than a hypothetical: the history of paired sync/async fixes (the retry "sleep 0s before first retry" correction, the redirect close-before-raise leak, the per-operation tracing lifecycle correction in #7) each had to be reasoned about and applied across both twins. When a control-flow or cleanup-ordering fix lands in the sync loop, the async transcription has to be patched in lockstep by hand.

The duplicated default literals are the most user-visible risk: a future edit that bumps, say, the sync backoff_max without touching the async twin would leave sync and async clients hitting the same service with silently different retry budgets, and nothing would flag the divergence.

Proposed direction

Express each stateful loop (retry, redirect) once as a generator/coroutine that yields I/O intents rather than performing I/O directly — for example send this request, sleep this long, close this response. A thin sync runner drives the generator by calling self.next.send / clock.sleep / response.close; a thin async runner drives the same generator by awaiting those operations. The seam that makes this viable already exists and is documented: the async SansIO runner's _resolve (pipeline/_async_sansio_runner.py) awaits any Awaitable return and passes plain values through, which is the same "describe once, await on the async side" pattern these stateful loops need (docs/architecture.md: "the async pipeline auto-awaits any coroutine return").

Concretely this would:

  • collapse the two dispatch loops into one authoritative description, so cleanup ordering and error classification live in a single place;
  • eliminate the cross-module reach into _configure_settings / _is_retry / _build_next_request and friends, since the loop and its helpers would sit together;
  • leave only the genuinely-different awaiting layer hand-written (the sync vs async runner shim).

For configuration, make the sync policy the single source of truth — the async twin holds a config (as AsyncRetryPolicy already partly does) or otherwise derives its signature/defaults from the sync one — so each default literal lives in exactly one place.

Trade-off of the proposal: a generator-driven loop is less linear to read than a straight while loop and adds one layer of indirection. That cost is weighed against permanently removing the burden of keeping two transcriptions in step on every control-flow change.

Acknowledging the current rationale

The async surface is a deliberate, first-class part of the SDK, and some twins are genuinely thin (AsyncSetDatePolicy differs from its sync version only by one await) — for those, a shared core would add more indirection than it saves, and leaving them as twins is reasonable. The duplication that matters is the stateful loops (retry, redirect) where the copied body is long, branch-heavy, and cleanup-sensitive. Notably, hand-transcribed sync/async parity is not listed among the documented "Honest scope boundaries" in CHANGELOG.md (which call out the deferred default error map, no sendfile, sync-only OTel/logging, no MCP, no codegen), so this is an open structural question rather than a settled trade-off — which is why it seems worth revisiting now, while there are only a handful of twins, rather than after the policy set grows.

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