From e8523438fb7c22be5acda8e47748a02c7d5d6d17 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:34:56 +0000 Subject: [PATCH 1/6] docs(net): Timeout layer design spec (Slice 1 PR 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Response-future-only timeout (bounds the send, not the permit wait, per ADR-0031 §1); RequestTimeout per-request override with layer default; body-transparent; TimeoutBody deferred (inert on IBKR's buffered traffic). Co-Authored-By: Claude Opus 4.8 (1M context) --- ...026-07-04-net-http-timeout-layer-design.md | 234 ++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md diff --git a/docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md b/docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md new file mode 100644 index 0000000..2e0a14b --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md @@ -0,0 +1,234 @@ +# net-http `Timeout` layer — design (Slice 1, PR 2) + +## Context + +Slice 0 landed the net-http **construction surface** (transport contract ADR-0030; +`AuthSource`/`Auth`/`Guarded` in #66; the boot-time pacing config `RateKey`/ +`RateLimitConfig`/`validate_coverage` in #72). Slice 1 implements the resilience +*layers* of [ADR-0031](../../adr/0031-http-resilience-venue-pacing.md) — `Timeout`, +`RateLimit`, `Retry`, `CircuitBreaker`, `Tracing` — each a standalone, composable +`Service` generic over [`net-api::Timer`](../../adr/0029-network-adapter-stack-transport-split-compile-time-composition.md), +tested over inline service doubles + `MockTimer`. Assembly (`stack()`/`build()`) is +Slice 2. + +**PR 1 landed the `RateLimit` layer** (#76: `RateLimit` + `RateLimitLayer`, +the `RateScope`/`Scope` directive, the token-bucket + concurrency acquire, `Guarded` +permit lifetime). This spec covers **PR 2: `Timeout`** — the simplest timing layer, +and a clean template for the remaining ones. It reuses every seam PR 1 established: +the `Layer`/`Service` contracts, `net-api::Timer`, `futures_util::future::select` for +the race, and the inline-double + `MockTimer` test pattern (net-http-api **cannot** +dev-depend on `net-http-mock`'s `MockClient` — that closes a crate cycle and the two +builds' `Service` impls do not unify; `rate_limit.rs`/`body.rs` use inline doubles for +exactly this reason). + +### Governing ADRs + +- **ADR-0031 §1** — the default stack `Tracing → CircuitBreaker → Retry → RateLimit → + Timeout → BufferOrStream → Auth → leaf`. `Timeout` sits **inside `Retry`** (each + attempt re-times cleanly) and **outside `BufferOrStream`/`Auth`/leaf**, and it + *"bounds the send, not the permit wait"* — which is precisely why `RateLimit` is + outside it. +- **ADR-0029** — `Timer` (`now()` + `sleep()`), compile-time composition, no `dyn`. +- **ADR-0030 §4 / ADR-0034 §2** — `ResponseBody` is `Buffered { Full }` *xor* + `Streaming { B }`; the buffered branch is fully in memory before the response future + resolves. This is the fact that makes a body-level timeout unnecessary for v1 (see + Decision 4). + +## Goal + +A `Timeout` `Service` (+ its `TimeoutLayer` factory) that bounds how long the +inner stack may take to **produce a response**, returning `HttpError::Timeout` when a +per-layer (or per-request-overridden) deadline elapses first — runtime-neutral +(`Timer`-generic, `futures-util` race, **no** `tokio`), body-transparent, and mockable +with a fake clock. + +## Scope (in) + +- The `Timeout` service + `TimeoutLayer` factory (impl'ing `net-api::Layer`), + in `oath-adapter-net-http-api`. +- The **response-future race**: `select(inner.call(req), timer.sleep(dur))` — inner + wins → its `Result` verbatim; deadline wins → `HttpError::Timeout` (inner future + dropped/cancelled). +- The per-request **`RequestTimeout(Duration)`** `http::Request` extension overriding + the layer default; **absent → layer default** (not fail-closed — see Decision 2). +- **Body-transparency:** `Response = http::Response` passed through untouched (no + `Guarded`-style wrapper, no body clone). +- `MockTimer`-driven tests with inline service doubles. + +## Non-goals (deferred — each its own PR/slice) + +| Deferred | Why | Where | +| --- | --- | --- | +| `TimeoutBody` — a deadline-carrying body bounding a **streaming** transfer's mid-stream stall | Inert on IBKR's all-buffered traffic (a `Buffered` body is already in memory when `call` returns, so a per-poll deadline can never trip); unspecified by the ADRs. A clean **additive** follow-up when a streaming venue first lands (Decision 4). | future PR | +| `Retry`, `CircuitBreaker`, `Tracing` layers | Independent `Service`s; each its own PR | Slice 1 PRs 3–5 | +| `stack()`/`build()` assembly, `HttpConfig`, default layer order | Construction/wiring | Slice 2 | +| Tokio `Timer` impl, hyper backend | Runtime-specific | Slice 2 (`net-http-hyper`) | +| A separate idle-timeout vs total-deadline distinction | YAGNI — one deadline models every IBKR endpoint | when a venue needs it | + +## Decisions + +### 1. Layer shape & construction + +```rust +pub struct TimeoutLayer { default: Duration, timer: T } +pub struct Timeout { inner: S, default: Duration, timer: T } +``` + +`TimeoutLayer::new(default: Duration, timer: T) -> Self` is **infallible** — every +`Duration` is a valid deadline, so there is nothing to validate and no `Result`/ +`BuildError` (contrast `RateLimitLayer::new`, which validates a config map). `Clone` +and `Debug` are **hand-written** (not derived): `Debug` uses `finish_non_exhaustive` +showing only `default`; `Clone` bounds `T: Clone` (and, for `Timeout`, `S: Clone`) — +the same reason `RateLimit` hand-rolls them, so the derives don't demand `Debug`/`Clone` +on the inner service. `impl Layer for TimeoutLayer { type Service = +Timeout; … }` clones the `timer` and copies `default` into each produced service. + +### 2. The per-request directive — `RequestTimeout` + +```rust +#[derive(Debug, Clone, Copy)] +pub struct RequestTimeout(pub Duration); // http::Request extension +``` + +`Copy` so it survives the per-attempt request clone `Retry` performs (matching +`Retryability`). The adapter stamps it when it knows an endpoint warrants a +non-default bound (e.g. a heavier fetch). Resolution in `call`: + +```rust +let dur = req.extensions().get::().map_or(self.default, |t| t.0); +``` + +- **Present → the override.** **Absent → the layer default.** +- **Absent is *not* fail-closed** (unlike `RateScope`, ADR-0034 Amendment #1). A missing + `RateScope` could silently skip an endpoint's own rate limit — a pacing hole into + IBKR's 429 box — so it is rejected. A missing `RequestTimeout` has no such hazard: the + layer default still bounds the request. There is no gate to bypass, so the safe + default is simply "use the global deadline". This asymmetry is deliberate and + recorded (see §Amendment). + +### 3. Data flow — the race + +```rust +impl Service> for Timeout +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, +{ + type Response = http::Response; + type Error = HttpError; + + #[allow(clippy::manual_async_fn)] + fn call(&self, req: http::Request) + -> impl Future> + Send + { + async move { + let dur = req.extensions().get::().map_or(self.default, |t| t.0); + let call = std::pin::pin!(self.inner.call(req)); + let nap = std::pin::pin!(self.timer.sleep(dur)); + match futures_util::future::select(call, nap).await { + Either::Left((res, _)) => res, // inner first → verbatim + Either::Right(((), _)) => Err(HttpError::Timeout), // deadline → Timeout + } + } + } +} +``` + +- **`select` polls the inner call first**, so a ready inner is never spuriously + preempted by a `Duration::ZERO` deadline (the ordering `rate_limit`'s `acquire_conc` + already relies on). +- **`S: Sync`** because the returned `Send` future borrows `&self` (`&S` is `Send` only + if `S: Sync`; `T: Sync` holds via `Timer: Sync`). Same bound `RateLimit` carries. +- On timeout the inner future is **dropped** — cancellation is the runtime-neutral way + to abandon the send; no `tokio::time::timeout`. +- Not `async fn`: the trait requires the returned future be `Send` (only the desugared + `impl Future + Send` form can state it), matching every other layer. + +### 4. Why no `TimeoutBody` in v1 + +The race ends the instant `inner.call()` yields a `Response`. What that bounds depends +on **where the body-read sits relative to that instant**: + +- **Buffered response (every real IBKR endpoint — `/history`, `/snapshot`, …).** The + (future) `BufferOrStream` layer — inside `Timeout` — reads the wire body into + `Full` **before** returning the `Response`, so the response future already + covers the entire fetch. Afterward `ResponseBody::Buffered::poll_frame` is a + synchronous in-memory replay (`Poll::Ready` at once); it **cannot** stall, so a + body-level deadline could never fire. +- **Streaming response (no current venue).** `inner.call()` returns at headers; the wire + body is pulled frame-by-frame by the consumer *after* the `Timeout` future has already + resolved. A mid-stream stall here is the **only** thing a response-future race misses, + and bounding it is the **only** thing a `TimeoutBody` (a body carrying a `Timer` + + deadline, checked each `poll_frame`) would buy. + +Since IBKR is all-buffered and the ADRs mandate nothing here, `TimeoutBody` is inert +weight on 100% of current traffic → deferred. It is a clean additive follow-up (wrap the +`Streaming` arm, leave `Buffered` untouched) when a streaming venue lands. + +### 5. Error handling + +- Deadline wins → **`HttpError::Timeout`** (existing variant, `→ ErrorKind::Timeout`) — + **no new variant**. +- Inner `Err(_)` is propagated **unchanged**: a `Connection`/`Auth`/`Throttled` error + keeps its identity and is never masked as `Timeout`. (`HttpError` has no `PartialEq`, + so tests assert with `matches!`.) + +### 6. Stack interaction (ADR-0031 §1) + +`… → Retry → RateLimit → Timeout → BufferOrStream → Auth → leaf`. Inside `Retry` so each +attempt gets a fresh deadline; outside `RateLimit` so the deadline bounds the **send**, +not the pacing-permit wait (a request throttled by `RateLimit` returns `Throttled` +before `Timeout` is even entered). `Timeout` is body-transparent, so it composes with +`RateLimit`'s `Guarded` output without disturbing the permit lifetime. + +## Testing (MockTimer-driven, inline doubles) + +Time is driven by `MockTimer::advance()`; the leaf is an **inline** `Service` double (no +`MockClient` — cycle). `#[tokio::test]` provides the executor; the timeout-firing tests +spawn the call, `yield_now`, then `advance` so the layer's `sleep` resolves while the +inner future is pending (the shape `rate_limit`'s concurrency tests use). + +- **Fast inner passes, body transparent:** a leaf returning immediately → `Ok`; the + response body `collect()`s to the expected bytes (proves `Response` is untouched). +- **Slow inner times out:** a leaf that `await`s `timer.sleep(long)`; layer default `d` + → spawn, `yield_now`, `advance(d)` → `Err(HttpError::Timeout)`; the leaf's future is + dropped (assert it did not "complete"). +- **Per-request override:** `RequestTimeout(short)` fires before the (longer) default; + `RequestTimeout(long)` outlives a `default`-length advance — both vs the same slow + leaf. Absent extension → the default applies. +- **Inner error passes through:** a leaf returning `Err(HttpError::connection(...))` → + the call returns `Connection`, **not** `Timeout`. +- **`select` ordering:** an immediately-ready leaf with `default = Duration::ZERO` still + returns `Ok` (inner polled first), not `Timeout`. + +## Dependencies + +**No new dependency, no `Cargo.toml` change.** `futures-util` (the `select` race) and +`http`/`bytes`/`http-body` are crate deps; `oath-adapter-net-mock` (`MockTimer`) and +`tokio` are dev-deps — all present since #76. Still **no** `tokio`/`hyper`/`reqwest`/ +`serde` in the layer. + +## Definition of done + +- `Timeout` + `TimeoutLayer` implemented as specified; the `RequestTimeout` + extension defined; `lib.rs` gains `pub mod timeout;` + re-exports + a module-doc + bullet; all with the tests above. +- ADR-0034 gains an append-only amendment for the `RequestTimeout` per-request override, + the response-future-only scope, and the deferred `TimeoutBody`. +- `just ci` green (fmt, lint = deny, test + doctests, doc, deny, typos, machete); no new + warnings; no `unsafe`/`unwrap`/`expect`/indexing in non-test code. +- `CHANGELOG.md` `[Unreleased]` updated. +- Delivered as one issue → one branch (worktree) → one PR (`Closes #N`). + +## Open questions (for the implementation plan) + +1. **ADR placement** — record the Timeout refinements as an append-only ADR-0034 + amendment (the living 2026-07-04 list, where #76 recorded RateLimit as #5), i.e. + Amendment #6? Leaning yes, for trail-completeness parity with RateLimit. (Note: the + primary checkout carries an *unmerged* local ADR-0034 edit numbered #5 for unrelated + WS-auth work; this branch is off `origin/main` where #5 is the RateLimit note, so the + Timeout amendment is #6 here — any renumbering of the WS-auth edit is that PR's + concern.) +2. **Test executor** — `#[tokio::test]` + spawn/`yield_now`/`advance` (as `rate_limit`), + or a hand-polled `Waker::noop()` executor for a stricter runtime-neutrality + demonstration? Leaning `#[tokio::test]` for parity with the shipped layer's tests. From 88b23fdbd211cfd8718a521c14ab2fe83f18646b Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:46:26 +0000 Subject: [PATCH 2/6] docs(net): Timeout layer implementation plan (Slice 1 PR 2) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-07-04-net-http-timeout-layer.md | 600 ++++++++++++++++++ 1 file changed, 600 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-04-net-http-timeout-layer.md diff --git a/docs/superpowers/plans/2026-07-04-net-http-timeout-layer.md b/docs/superpowers/plans/2026-07-04-net-http-timeout-layer.md new file mode 100644 index 0000000..7b4f7d9 --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-net-http-timeout-layer.md @@ -0,0 +1,600 @@ +# net-http `Timeout` Layer (Slice 1, PR 2) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Build the `Timeout` HTTP middleware layer that bounds how long the inner stack may take to **produce a response** — the *send*, not the pacing-permit wait — returning `HttpError::Timeout` when a per-layer (or per-request-overridden) deadline elapses first. + +**Architecture:** A `Timer`-generic, runtime-neutral `Service` wrapper in `oath-adapter-net-http-api`. It reads an optional per-request `RequestTimeout(Duration)` extension (absent → the layer default), then races `inner.call(req)` against `Timer::sleep(dur)` via `futures_util::future::select`: the inner future winning yields its `Result` verbatim, the deadline winning yields `HttpError::Timeout` (inner future dropped). **Body-transparent** — `http::Response` is returned untouched (no `Guarded`-style carrier, no `B` bound). + +**Tech Stack:** Rust (edition 2024, MSRV 1.90), `just`, `futures-util` (the race — already a crate dep since #76), `http`/`bytes`, `std::time::Duration`, `net-api::Timer`. Tests use inline service doubles + `MockTimer` (`oath-adapter-net-mock`), driven on `tokio` (dev-only). + +## Global Constraints + +Every task implicitly includes these: + +- **Edition 2024, MSRV 1.90.** No `unsafe` — the crate is `#![forbid(unsafe_code)]`. +- **No `unwrap`/`expect`/indexing/panic in non-test code** — return `Result`. Test code is exempt for `unwrap`/`expect`/indexing. +- **`just lint` = clippy `-D warnings` + `pedantic`/`nursery`** — `#[must_use]` where asked, document all public items (`missing_docs`), `Debug` on all **public** types (`missing_debug_implementations` — hand-impl where a derive would demand `Debug`/`Clone` on `S`/`T`), `const fn` where `missing_const_for_fn` asks. +- **`net-http-api` charter:** no async *runtime* — no `tokio`/`hyper`/`reqwest`/`serde` in non-dev deps. **This PR adds no dependency** (`futures-util`, `http`, `bytes` are crate deps; `oath-adapter-net-mock` + `tokio` are dev-deps — all present since #76), so `cargo-deny`/`machete` are unaffected. +- **net-http-api tests must NOT dev-depend on `oath-adapter-net-http-mock` (`MockClient`)** — it normal-depends on this crate, so the dev-dep closes a cycle that recompiles a second, non-unifying copy of `net-http-api` (E0599: `MockClient` does not satisfy *this* crate's `Service`). Use **inline** service doubles + `oath-adapter-net-mock`'s `MockTimer`, exactly as `rate_limit.rs`/`body.rs` do. +- **DoD per PR:** `just ci` green (fmt, lint, test + doctests, doc, deny, typos, machete). Update `CHANGELOG.md` `[Unreleased]`. One issue → one branch → worktree → one PR (`Closes #`). + +## Source spec + +[docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md](../specs/2026-07-04-net-http-timeout-layer-design.md), governed by [ADR-0031 §1](../../adr/0031-http-resilience-venue-pacing.md) and [ADR-0034](../../adr/0034-http-construction-surface-auth-guarded-boot-coverage.md). This is **Slice 1, PR 2** — the second of the resilience-layer PRs (RateLimit #76 landed PR 1; then Timeout, Retry, CircuitBreaker, Tracing). + +## File Structure + +- `crates/adapter/net/http/api/src/timeout.rs` — **new** (Tasks 1–2). `RequestTimeout`, `TimeoutLayer`, `Timeout`, the `Layer`/`Service` impls, and their tests. +- `crates/adapter/net/http/api/src/lib.rs` — **modify** (Tasks 1–2). `pub mod timeout;` + re-exports + module-doc bullet. +- `docs/adr/0034-...md`, `CHANGELOG.md` — **modify** (Task 3). + +No `Cargo.toml` change. Each task is one or more commits; the tasks together are one PR/issue. + +--- + +## Setup: issue (worktree already exists) + +> The isolated worktree **already exists** at `.claude/worktrees/net-http-timeout` (branch `feat/net-http-timeout`, branched off `origin/main` = #76). All tasks run inside it. Only the GitHub issue remains to be created. + +- [ ] **Create the issue** + +```bash +gh issue create \ + --title "feat(net): Timeout resilience layer (Slice 1, PR 2)" \ + --label enhancement \ + --body "Slice 1 PR 2 of the net-http resilience layers (spec: docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md; ADR-0031 §1). + +- \`Timeout\` + \`TimeoutLayer\` (impl \`net-api::Layer\`): bounds the send (inner call -> response), not the pacing-permit wait — a response-future race against \`Timer::sleep\`, \`HttpError::Timeout\` on the deadline +- \`RequestTimeout(Duration)\` per-request override extension; absent -> layer default (not fail-closed — the global deadline still applies) +- Body-transparent (\`Response\` untouched); a streaming-body \`TimeoutBody\` is deferred (inert on IBKR's buffered traffic). No new dependency." +``` + +Note the issue number `#` for the PR body. + +--- + +## Task 1: `RequestTimeout` directive + module scaffold + +**Files:** +- Create: `crates/adapter/net/http/api/src/timeout.rs` +- Modify: `crates/adapter/net/http/api/src/lib.rs` + +**Interfaces:** +- Consumes: nothing (only `std::time::Duration`). +- Produces: + - `oath_adapter_net_http_api::RequestTimeout` — `struct RequestTimeout(pub Duration)` (`Debug`, `Clone`, `Copy`), an `http::Request` extension. + - Task 2 adds `TimeoutLayer`/`Timeout` to this module and the `Service` race. + +- [ ] **Step 1: Write the failing test** + +Create `crates/adapter/net/http/api/src/timeout.rs` with the module doc + only the directive test below: + +```rust +//! The `Timeout` resilience layer (ADR-0031 §1). +//! +//! Bounds how long the inner stack may take to **produce a response** — the +//! *send*, not the pacing-permit wait (`RateLimit` sits outside it, so a +//! throttled request never enters `Timeout`). Races `inner.call(req)` against +//! [`Timer::sleep`](oath_adapter_net_api::Timer::sleep); the deadline winning +//! yields [`HttpError`]`::Timeout` with the inner future dropped, the inner +//! finishing first yields its `Result` verbatim. **Body-transparent:** the +//! response body is returned untouched. The per-request [`RequestTimeout`] +//! extension overrides the layer default; an absent extension uses the default. +//! Runtime-neutral: generic over [`Timer`](oath_adapter_net_api::Timer), race +//! via `futures-util`. + +use std::time::Duration; + +/// A per-request timeout override, carried as an `http::Request` extension. +/// +/// The adapter stamps it for an endpoint that needs a non-default bound. `Copy` +/// so it survives the per-attempt request clone `Retry` performs (Slice 1). An +/// **absent** extension uses the layer default — a missing override has no +/// fail-open hazard (the global deadline still applies), so it is not rejected +/// (contrast `RateScope`, ADR-0034 Amendment #1). +#[derive(Debug, Clone, Copy)] +pub struct RequestTimeout(pub Duration); + +#[cfg(test)] +mod tests { + use super::RequestTimeout; + use std::time::Duration; + + #[test] + fn request_timeout_round_trips_through_request_extensions() { + let mut req = http::Request::new(bytes::Bytes::new()); + req.extensions_mut().insert(RequestTimeout(Duration::from_secs(3))); + let got = req + .extensions() + .get::() + .copied() + .expect("override present"); + assert_eq!(got.0, Duration::from_secs(3)); + } +} +``` + +In `lib.rs`, add the module-doc bullet (after the `rate_limit` bullet, line 14), the `pub mod`, and the re-export (keep alphabetical ordering — `timeout` sits after `service`): + +Module-doc bullet (insert after line 14): + +```rust +//! - [`timeout`] — the `Timeout` layer, its `TimeoutLayer` factory, and the +//! `RequestTimeout` per-request override +``` + +Module declaration (after `pub mod service;`): + +```rust +pub mod timeout; +``` + +Re-export (after `pub use service::Service;`): + +```rust +pub use timeout::RequestTimeout; +``` + +(Task 2 extends this to `pub use timeout::{RequestTimeout, Timeout, TimeoutLayer};`.) + +- [ ] **Step 2: Run it to verify it fails** + +Run: `just check` +Expected: FAIL — before adding the code, `cannot find type RequestTimeout in module timeout` / unresolved module `timeout`. After adding the code it should compile; the step exists to confirm the wiring, so if it already passes, proceed. + +- [ ] **Step 3: Confirm the test passes** + +Run: `cargo test -p oath-adapter-net-http-api timeout && just lint` +Expected: PASS, warning-free. (`RequestTimeout` is fully implemented in Step 1; this task has no separate implementation step.) + +- [ ] **Step 4: Commit** + +```bash +git add crates/adapter/net/http/api/src/timeout.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): RequestTimeout per-request timeout override extension" +``` + +--- + +## Task 2: `Timeout` layer — the response-future race + +**Files:** +- Modify: `crates/adapter/net/http/api/src/timeout.rs` +- Modify: `crates/adapter/net/http/api/src/lib.rs` + +**Interfaces:** +- Consumes: `RequestTimeout` (Task 1); `HttpError`, `Service` (crate); `Layer`, `Timer` (`oath_adapter_net_api`); `futures_util::future::{Either, select}`. +- Produces: + - `oath_adapter_net_http_api::TimeoutLayer` — `impl Layer` factory; `pub const fn new(default: Duration, timer: T) -> Self`. + - `oath_adapter_net_http_api::Timeout` — the wrapping `Service`; for an inner `S: Service, Response = http::Response, Error = HttpError> + Sync` and `T: Timer`, it is `Service, Response = http::Response, Error = HttpError>` (body-transparent — same `B`). + +- [ ] **Step 1: Write the failing tests** + +Add the imports + inline doubles + tests to the `tests` module in `timeout.rs` (replace the `use super::RequestTimeout;` line): + +```rust + use super::{RequestTimeout, Timeout, TimeoutLayer}; + use crate::{HttpError, Service}; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_api::{Layer, Timer}; + use oath_adapter_net_mock::MockTimer; + use std::future::Future; + use std::pin::Pin; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::task::{Context, Poll}; + use std::time::Duration; + + // A canned one-frame response body (`Data = Bytes`, `Error = HttpError`) — + // enough to prove `Timeout` returns the body untouched. `Debug` so + // `Result::unwrap_err` can render an unexpected `Ok`. + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(body: &'static [u8]) -> Self { + Self { data: Some(Bytes::from_static(body)) } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data.as_ref().map_or_else( + || SizeHint::with_exact(0), + |d| SizeHint::with_exact(d.len() as u64), + ) + } + } + + // An inline leaf returning `200` immediately — the fast path. Inline (not + // `MockClient`) to avoid the net-http-mock -> net-http-api dev-dep cycle. + #[derive(Clone)] + struct FastLeaf { + body: &'static [u8], + } + impl Service> for FastLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let body = self.body; + async move { Ok(http::Response::new(StubBody::new(body))) } + } + } + + // An inline leaf that sleeps `delay` on the shared clock before returning — + // lets a test hold the inner future pending while the layer deadline fires, + // or (with a finite delay) complete after the deadline would have. + #[derive(Clone)] + struct DelayLeaf { + timer: T, + delay: Duration, + completed: Arc, + } + impl DelayLeaf { + fn new(timer: T, delay: Duration) -> Self { + Self { timer, delay, completed: Arc::new(AtomicBool::new(false)) } + } + fn completed(&self) -> bool { + self.completed.load(Ordering::Relaxed) + } + } + impl Service> for DelayLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let timer = self.timer.clone(); + let delay = self.delay; + let completed = self.completed.clone(); + async move { + timer.sleep(delay).await; + completed.store(true, Ordering::Relaxed); + Ok(http::Response::new(StubBody::new(b"slow"))) + } + } + } + + // An inline leaf returning a `Connection` error immediately. + #[derive(Clone)] + struct ErrLeaf; + impl Service> for ErrLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Err(HttpError::connection("reset")) } + } + } + + fn req(override_to: Option) -> http::Request { + let mut r = http::Request::new(Bytes::new()); + if let Some(d) = override_to { + r.extensions_mut().insert(RequestTimeout(d)); + } + r + } + + #[tokio::test] + async fn fast_inner_passes_and_body_is_transparent() { + let svc = TimeoutLayer::new(Duration::from_secs(1), MockTimer::new()).layer(FastLeaf { body: b"ok" }); + let resp = svc.call(req(None)).await.expect("inner within deadline"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // Response passed straight through + } + + #[tokio::test] + async fn slow_inner_times_out_at_default() { + let timer = MockTimer::new(); + let leaf = DelayLeaf::new(timer.clone(), Duration::from_secs(3600)); + let leaf_probe = leaf.clone(); + let svc = TimeoutLayer::new(Duration::from_secs(1), timer.clone()).layer(leaf); + let waiter = tokio::spawn(async move { svc.call(req(None)).await }); + tokio::task::yield_now().await; // task registers inner sleep(3600s) + deadline sleep(1s) + timer.advance(Duration::from_secs(1)); // fire the layer deadline + let err = waiter.await.unwrap().unwrap_err(); + assert!(matches!(err, HttpError::Timeout)); // HttpError has no PartialEq + assert!(!leaf_probe.completed(), "inner future must be dropped, not run to completion"); + } + + #[tokio::test] + async fn request_timeout_override_shortens_deadline() { + // Layer default is huge; a per-request 1s override fires first. + let timer = MockTimer::new(); + let svc = TimeoutLayer::new(Duration::from_secs(3600), timer.clone()) + .layer(DelayLeaf::new(timer.clone(), Duration::from_secs(3600))); + let waiter = tokio::spawn(async move { svc.call(req(Some(Duration::from_secs(1)))).await }); + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // fires the override, not the default + let err = waiter.await.unwrap().unwrap_err(); + assert!(matches!(err, HttpError::Timeout)); + } + + #[tokio::test] + async fn request_timeout_override_lengthens_deadline() { + // Default 1s would time out; a 5s override lets the 2s inner complete. + let timer = MockTimer::new(); + let svc = TimeoutLayer::new(Duration::from_secs(1), timer.clone()) + .layer(DelayLeaf::new(timer.clone(), Duration::from_secs(2))); + let waiter = tokio::spawn(async move { svc.call(req(Some(Duration::from_secs(5)))).await }); + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // now=1s: neither the 2s inner nor the 5s override is due + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // now=2s: the inner completes first + let resp = waiter.await.unwrap().expect("override outlived the default; inner completed"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"slow")); + } + + #[tokio::test] + async fn inner_error_passes_through_not_masked_as_timeout() { + let svc = TimeoutLayer::new(Duration::from_secs(1), MockTimer::new()).layer(ErrLeaf); + let err = svc.call(req(None)).await.unwrap_err(); + assert!(matches!(err, HttpError::Connection(_))); // its own error, never Timeout + } + + #[tokio::test] + async fn zero_default_still_returns_ready_inner() { + // `select` polls the inner call first, so a ready inner is never + // preempted by a Duration::ZERO deadline. + let svc = TimeoutLayer::new(Duration::ZERO, MockTimer::new()).layer(FastLeaf { body: b"ok" }); + svc.call(req(None)).await.expect("inner polled first, not the zero deadline"); + } +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `just check` +Expected: FAIL — `cannot find type Timeout`/`TimeoutLayer` in module `timeout`. + +- [ ] **Step 3: Implement the layer** + +Insert the imports + types between the `RequestTimeout` definition and the `tests` module in `timeout.rs`. Extend the top-of-file `use` block: + +```rust +use crate::{HttpError, Service}; +use bytes::Bytes; +use futures_util::future::{Either, select}; +use oath_adapter_net_api::{Layer, Timer}; +use std::fmt; +use std::future::Future; +use std::time::Duration; +``` + +(The existing `use std::time::Duration;` from Task 1 is now covered by this block — keep a single `Duration` import.) + +Add below `RequestTimeout`: + +```rust +/// The `Timeout` [`Layer`] factory: holds the default deadline + clock and +/// produces a [`Timeout`] around any inner service. +pub struct TimeoutLayer { + default: Duration, + timer: T, +} + +impl TimeoutLayer { + /// Build the layer with a default deadline and a [`Timer`] clock. + /// + /// The default bounds every request lacking a [`RequestTimeout`] extension. + /// Infallible — every [`Duration`] is a valid deadline (no config to check). + #[must_use] + pub const fn new(default: Duration, timer: T) -> Self { + Self { default, timer } + } +} + +impl Clone for TimeoutLayer { + fn clone(&self) -> Self { + Self { default: self.default, timer: self.timer.clone() } + } +} + +impl fmt::Debug for TimeoutLayer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TimeoutLayer").field("default", &self.default).finish_non_exhaustive() + } +} + +impl Layer for TimeoutLayer { + type Service = Timeout; + + fn layer(&self, inner: S) -> Timeout { + Timeout { inner, default: self.default, timer: self.timer.clone() } + } +} + +/// The `Timeout` middleware: races the inner call against a deadline, returning +/// [`HttpError`]`::Timeout` if the deadline wins. Body-transparent — the inner +/// `http::Response` is returned untouched. +pub struct Timeout { + inner: S, + default: Duration, + timer: T, +} + +impl Clone for Timeout { + fn clone(&self) -> Self { + Self { inner: self.inner.clone(), default: self.default, timer: self.timer.clone() } + } +} + +impl fmt::Debug for Timeout { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Timeout").field("default", &self.default).finish_non_exhaustive() + } +} + +impl Service> for Timeout +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, +{ + type Response = http::Response; + type Error = HttpError; + + // Not `async fn`: the trait requires the returned future to be `Send`. + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + async move { + let dur = req + .extensions() + .get::() + .map_or(self.default, |t| t.0); + // `select` polls `call` first, so a ready inner beats a zero deadline; + // pinning to the stack makes both futures `Unpin` for `select`. + let call = std::pin::pin!(self.inner.call(req)); + let nap = std::pin::pin!(self.timer.sleep(dur)); + match select(call, nap).await { + Either::Left((res, _)) => res, // inner finished first -> its Result verbatim + Either::Right(((), _)) => Err(HttpError::Timeout), // deadline won -> inner dropped + } + } + } +} +``` + +In `lib.rs`, extend the Task 1 re-export: + +```rust +pub use timeout::{RequestTimeout, Timeout, TimeoutLayer}; +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `just check && cargo test -p oath-adapter-net-http-api timeout && just lint` +Expected: PASS, warning-free. + +> Known risks (from the spec's implementation notes): +> - `select` needs `Unpin` futures — `std::pin::pin!` both (shown). It polls the left (`call`) first, so an immediately-ready inner is never preempted by a `Duration::ZERO` deadline. +> - `S: Sync` is required because the returned `Send` future borrows `&self` (`&S: Send` ⇒ `S: Sync`; `T: Sync` holds via `Timer`). +> - `B` carries **no** `http_body::Body` bound — `Timeout` never touches the body, so it stays fully generic (contrast `RateLimit`, which builds `Guarded`). +> - If clippy's `missing_const_for_fn` rejects `const fn new` for the generic `T` (it should not — construction-only), drop `const`. + +- [ ] **Step 5: Commit** + +```bash +git add crates/adapter/net/http/api/src/timeout.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): Timeout layer — response-future race, body-transparent" +``` + +--- + +## Task 3: ADR amendment, CHANGELOG, full gate, PR + +**Files:** +- Modify: `docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md` +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: ADR-0034 append-only amendment** + +Append to ADR-0034's **Amendments (2026-07-04)** numbered list (after item 5, the RateLimit note #76 added) a new item 6: + +```markdown +6. **`Timeout` layer (Slice 1 PR 2).** The `Timeout` layer + `TimeoutLayer` + factory bound the **send** (`inner.call` → response), not the pacing-permit wait + (ADR-0031 §1) — a response-future race against `Timer::sleep`, `HttpError::Timeout` + on the deadline (inner future dropped). Body-transparent: `http::Response` is + returned untouched (no `Guarded`-style carrier, no `B: Body` bound). A per-request + `RequestTimeout(Duration)` extension overrides the layer default; an **absent** + extension uses the default (not fail-closed, unlike `RateScope` — a missing override + has no fail-open pacing hazard, the global deadline still applies). A `TimeoutBody` + bounding a *streaming* transfer's mid-stream stall is **deferred**: it is inert on + IBKR's all-buffered responses (a `Buffered` body is already in memory when `call` + returns) and lands additively when a streaming venue first needs it. +``` + +- [ ] **Step 2: CHANGELOG** + +Add to `CHANGELOG.md` `[Unreleased] → Added` (after the RateLimit resilience-layer entry #76): + +```markdown +- `oath-adapter-net-http-api` `Timeout` resilience layer (Slice 1 PR 2) — the + `Timeout` service + `TimeoutLayer` factory (`net-api::Layer`): bounds the + send (inner call → response) against a `net-api::Timer` deadline, returning + `HttpError::Timeout` when it elapses first (inner future dropped); body-transparent. + Adds the `RequestTimeout(Duration)` per-request override extension (absent → the + layer default). Response-future-only (ADR-0031 §1's "bounds the send, not the permit + wait"); a streaming-body timeout is deferred. No new dependency. (ADR-0031 §1, + ADR-0034.) +``` + +- [ ] **Step 3: Full local gate** + +Run: `just ci` +Expected: green (fmt, lint, test + doctests, doc, deny, typos, machete — no new dep, so `deny`/`machete` are unaffected). + +- [ ] **Step 4: Commit, push, PR** + +```bash +git add docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md CHANGELOG.md +git commit -m "docs(net): record Timeout layer amendment (ADR-0034) + changelog" +git push -u origin feat/net-http-timeout +gh pr create \ + --title "feat(net): Timeout resilience layer (Slice 1, PR 2)" \ + --body "Closes # + +Slice 1 **PR 2** of the net-http resilience layers (spec: docs/superpowers/specs/2026-07-04-net-http-timeout-layer-design.md; ADR-0031 §1). Builds on the RateLimit layer (#76). + +- **\`Timeout\`** + **\`TimeoutLayer\`** (\`net-api::Layer\`) — bounds the **send** (inner call → response), not the pacing-permit wait (ADR-0031 §1): a response-future race against \`Timer::sleep\`, returning \`HttpError::Timeout\` when the deadline wins (inner future dropped). Body-transparent — \`http::Response\` returned untouched. +- **\`RequestTimeout(Duration)\`** per-request override extension — absent → the layer default (not fail-closed: a missing override has no fail-open pacing hazard, unlike \`RateScope\`). +- A streaming-body \`TimeoutBody\` is **deferred** — inert on IBKR's all-buffered responses; a clean additive follow-up when a streaming venue lands. + +Runtime-neutral: generic over \`net-api::Timer\`, race via \`futures-util\` — no \`tokio\`/\`hyper\`. **No new dependency.** MockTimer-driven tests with inline service doubles. + +Next: **Slice 1 PR 3** — the \`Retry\` layer. + +🤖 Generated with [Claude Code](https://claude.com/claude-code)" +``` + +Expected: PR open, GitHub Actions CI green (same `just ci` + MSRV job). + +--- + +## Self-Review + +**Spec coverage (design doc §Scope + Decisions):** +- `Timeout` + `TimeoutLayer` (`Layer`), response-future race → Task 2. ✅ +- `RequestTimeout(Duration)` extension; absent → default (not fail-closed) — Task 1 (type) + Task 2 (`map_or(self.default, …)` + tests). ✅ +- Body-transparent `Response`, no `B: Body` bound — Task 2. ✅ +- Infallible `const fn new`, hand-written `Clone`/`Debug`, `S: Sync` bound — Task 2. ✅ +- `HttpError::Timeout` reuse (no new variant) — Task 2 (`Either::Right`). ✅ +- `select` polls inner first (zero-deadline ordering) — Task 2 test `zero_default_still_returns_ready_inner`. ✅ +- Inner error passes through unchanged — Task 2 test `inner_error_passes_through_not_masked_as_timeout`. ✅ +- MockTimer-driven tests, inline doubles, no `MockClient` — Tasks 1–2. ✅ +- ADR-0034 Amendment #6 + CHANGELOG — Task 3. ✅ +- Deferred (correctly absent): `TimeoutBody`, `Retry`/`CircuitBreaker`/`Tracing`, `stack()`/`build()`, tokio `Timer` — noted, not built. ✅ + +**Placeholder scan:** none — every step carries actual code or an actual command with expected output. + +**Type consistency:** +- `RequestTimeout(pub Duration)` — identical in Task 1's def, Task 2's `map_or(self.default, |t| t.0)`, and the `req()` helper. +- `TimeoutLayer::new(Duration, T) -> Self` and `.layer(inner) -> Timeout` — match the `Interfaces` block and every test call. +- `Timeout` `Service` impl: inner `Response = http::Response` → `Response = http::Response` (transparent) — matches `FastLeaf`/`DelayLeaf`/`ErrLeaf` (`Response = http::Response`, so `B = StubBody`). +- `select` arms `Either::Left((res, _))` / `Either::Right(((), _))` — consistent with `futures_util::future::select` over `call: Result<…>` and `nap: ()`. +- `lib.rs` re-export accumulates to `pub use timeout::{RequestTimeout, Timeout, TimeoutLayer};`. + +**Known risks to watch during impl:** listed inline in Task 2 Step 4 (`select` `Unpin`/poll-order, `S: Sync`, no `B` bound, `const fn` fallback). From 5653efee7d6d28d5541804ae58f2ddc90c93bbed Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:58:20 +0000 Subject: [PATCH 3/6] feat(net): RequestTimeout per-request timeout override extension Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/adapter/net/http/api/src/lib.rs | 4 ++ crates/adapter/net/http/api/src/timeout.rs | 44 ++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 crates/adapter/net/http/api/src/timeout.rs diff --git a/crates/adapter/net/http/api/src/lib.rs b/crates/adapter/net/http/api/src/lib.rs index 44af9fe..e998c7a 100644 --- a/crates/adapter/net/http/api/src/lib.rs +++ b/crates/adapter/net/http/api/src/lib.rs @@ -12,6 +12,8 @@ //! `RateLimitConfig`, and the boot-time `validate_coverage` check //! - [`rate_limit`] — the `RateLimit` layer, its `RateLimitLayer` factory, and //! the `RateScope`/`Scope` per-request directive +//! - [`timeout`] — the `Timeout` layer, its `TimeoutLayer` factory, and the +//! `RequestTimeout` per-request override //! //! The resilience layers, `stack`/`build` assembly, and backends land in later //! slices. No async runtime, `hyper`, `reqwest`, or `serde` here. @@ -24,6 +26,7 @@ pub mod error; pub mod rate; pub mod rate_limit; pub mod service; +pub mod timeout; pub use auth::{Auth, AuthSource, NoAuth, SetHeaders}; pub use body::{BufferMode, Guarded, ResponseBody}; @@ -35,3 +38,4 @@ pub use rate::{ }; pub use rate_limit::{RateLimit, RateLimitLayer, RateScope, Scope}; pub use service::Service; +pub use timeout::RequestTimeout; diff --git a/crates/adapter/net/http/api/src/timeout.rs b/crates/adapter/net/http/api/src/timeout.rs new file mode 100644 index 0000000..cd8daf5 --- /dev/null +++ b/crates/adapter/net/http/api/src/timeout.rs @@ -0,0 +1,44 @@ +//! The `Timeout` resilience layer (ADR-0031 §1). +//! +//! Bounds how long the inner stack may take to **produce a response** — the +//! *send*, not the pacing-permit wait (`RateLimit` sits outside it, so a +//! throttled request never enters `Timeout`). Races `inner.call(req)` against +//! [`Timer::sleep`](oath_adapter_net_api::Timer::sleep); the deadline winning +//! yields [`HttpError::Timeout`](crate::HttpError::Timeout) with the inner +//! future dropped, the inner +//! finishing first yields its `Result` verbatim. **Body-transparent:** the +//! response body is returned untouched. The per-request [`RequestTimeout`] +//! extension overrides the layer default; an absent extension uses the default. +//! Runtime-neutral: generic over [`Timer`](oath_adapter_net_api::Timer), race +//! via `futures-util`. + +use std::time::Duration; + +/// A per-request timeout override, carried as an `http::Request` extension. +/// +/// The adapter stamps it for an endpoint that needs a non-default bound. `Copy` +/// so it survives the per-attempt request clone `Retry` performs (Slice 1). An +/// **absent** extension uses the layer default — a missing override has no +/// fail-open hazard (the global deadline still applies), so it is not rejected +/// (contrast `RateScope`, ADR-0034 Amendment #1). +#[derive(Debug, Clone, Copy)] +pub struct RequestTimeout(pub Duration); + +#[cfg(test)] +mod tests { + use super::RequestTimeout; + use std::time::Duration; + + #[test] + fn request_timeout_round_trips_through_request_extensions() { + let mut req = http::Request::new(bytes::Bytes::new()); + req.extensions_mut() + .insert(RequestTimeout(Duration::from_secs(3))); + let got = req + .extensions() + .get::() + .copied() + .expect("override present"); + assert_eq!(got.0, Duration::from_secs(3)); + } +} From ca66fe2f7fe6329290f9a79d57d2abe1ee836223 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:11:13 +0000 Subject: [PATCH 4/6] =?UTF-8?q?feat(net):=20Timeout=20layer=20=E2=80=94=20?= =?UTF-8?q?response-future=20race,=20body-transparent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/adapter/net/http/api/src/lib.rs | 2 +- crates/adapter/net/http/api/src/timeout.rs | 322 ++++++++++++++++++++- 2 files changed, 319 insertions(+), 5 deletions(-) diff --git a/crates/adapter/net/http/api/src/lib.rs b/crates/adapter/net/http/api/src/lib.rs index e998c7a..9a243a5 100644 --- a/crates/adapter/net/http/api/src/lib.rs +++ b/crates/adapter/net/http/api/src/lib.rs @@ -38,4 +38,4 @@ pub use rate::{ }; pub use rate_limit::{RateLimit, RateLimitLayer, RateScope, Scope}; pub use service::Service; -pub use timeout::RequestTimeout; +pub use timeout::{RequestTimeout, Timeout, TimeoutLayer}; diff --git a/crates/adapter/net/http/api/src/timeout.rs b/crates/adapter/net/http/api/src/timeout.rs index cd8daf5..8e5abc8 100644 --- a/crates/adapter/net/http/api/src/timeout.rs +++ b/crates/adapter/net/http/api/src/timeout.rs @@ -3,15 +3,21 @@ //! Bounds how long the inner stack may take to **produce a response** — the //! *send*, not the pacing-permit wait (`RateLimit` sits outside it, so a //! throttled request never enters `Timeout`). Races `inner.call(req)` against -//! [`Timer::sleep`](oath_adapter_net_api::Timer::sleep); the deadline winning -//! yields [`HttpError::Timeout`](crate::HttpError::Timeout) with the inner +//! [`Timer::sleep`]; the deadline winning +//! yields [`HttpError::Timeout`] with the inner //! future dropped, the inner //! finishing first yields its `Result` verbatim. **Body-transparent:** the //! response body is returned untouched. The per-request [`RequestTimeout`] //! extension overrides the layer default; an absent extension uses the default. -//! Runtime-neutral: generic over [`Timer`](oath_adapter_net_api::Timer), race +//! Runtime-neutral: generic over [`Timer`], race //! via `futures-util`. +use crate::{HttpError, Service}; +use bytes::Bytes; +use futures_util::future::{Either, select}; +use oath_adapter_net_api::{Layer, Timer}; +use std::fmt; +use std::future::Future; use std::time::Duration; /// A per-request timeout override, carried as an `http::Request` extension. @@ -24,9 +30,127 @@ use std::time::Duration; #[derive(Debug, Clone, Copy)] pub struct RequestTimeout(pub Duration); +/// The `Timeout` [`Layer`] factory: holds the default deadline + clock and +/// produces a [`Timeout`] around any inner service. +pub struct TimeoutLayer { + default: Duration, + timer: T, +} + +impl TimeoutLayer { + /// Build the layer with a default deadline and a [`Timer`] clock. + /// + /// The default bounds every request lacking a [`RequestTimeout`] extension. + /// Infallible — every [`Duration`] is a valid deadline (no config to check). + #[must_use] + pub const fn new(default: Duration, timer: T) -> Self { + Self { default, timer } + } +} + +impl Clone for TimeoutLayer { + fn clone(&self) -> Self { + Self { + default: self.default, + timer: self.timer.clone(), + } + } +} + +impl fmt::Debug for TimeoutLayer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TimeoutLayer") + .field("default", &self.default) + .finish_non_exhaustive() + } +} + +impl Layer for TimeoutLayer { + type Service = Timeout; + + fn layer(&self, inner: S) -> Timeout { + Timeout { + inner, + default: self.default, + timer: self.timer.clone(), + } + } +} + +/// The `Timeout` middleware: races the inner call against a deadline. +/// +/// Returns [`HttpError::Timeout`] if the deadline +/// wins. Body-transparent — the inner `http::Response` is returned +/// untouched. +pub struct Timeout { + inner: S, + default: Duration, + timer: T, +} + +impl Clone for Timeout { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + default: self.default, + timer: self.timer.clone(), + } + } +} + +impl fmt::Debug for Timeout { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Timeout") + .field("default", &self.default) + .finish_non_exhaustive() + } +} + +impl Service> for Timeout +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, +{ + type Response = http::Response; + type Error = HttpError; + + // Not `async fn`: the trait requires the returned future to be `Send`. + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + async move { + let dur = req + .extensions() + .get::() + .map_or(self.default, |t| t.0); + // `select` polls `call` first, so a ready inner beats a zero deadline; + // pinning to the stack makes both futures `Unpin` for `select`. + let call = std::pin::pin!(self.inner.call(req)); + let nap = std::pin::pin!(self.timer.sleep(dur)); + match select(call, nap).await { + Either::Left((res, _)) => res, // inner finished first -> its Result verbatim + Either::Right(((), _)) => Err(HttpError::Timeout), // deadline won -> inner dropped + } + } + } +} + #[cfg(test)] mod tests { - use super::RequestTimeout; + use super::{RequestTimeout, TimeoutLayer}; + use crate::{HttpError, Service}; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_api::{Layer, Timer}; + use oath_adapter_net_mock::MockTimer; + use std::future::Future; + use std::pin::Pin; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::task::{Context, Poll}; use std::time::Duration; #[test] @@ -41,4 +165,194 @@ mod tests { .expect("override present"); assert_eq!(got.0, Duration::from_secs(3)); } + + // A canned one-frame response body (`Data = Bytes`, `Error = HttpError`) — + // enough to prove `Timeout` returns the body untouched. `Debug` so + // `Result::unwrap_err` can render an unexpected `Ok`. + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(body: &'static [u8]) -> Self { + Self { + data: Some(Bytes::from_static(body)), + } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data.as_ref().map_or_else( + || SizeHint::with_exact(0), + |d| SizeHint::with_exact(d.len() as u64), + ) + } + } + + // An inline leaf returning `200` immediately — the fast path. Inline (not + // `MockClient`) to avoid the net-http-mock -> net-http-api dev-dep cycle. + #[derive(Clone)] + struct FastLeaf { + body: &'static [u8], + } + impl Service> for FastLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let body = self.body; + async move { Ok(http::Response::new(StubBody::new(body))) } + } + } + + // An inline leaf that sleeps `delay` on the shared clock before returning — + // lets a test hold the inner future pending while the layer deadline fires, + // or (with a finite delay) complete after the deadline would have. + #[derive(Clone)] + struct DelayLeaf { + timer: T, + delay: Duration, + completed: Arc, + } + impl DelayLeaf { + fn new(timer: T, delay: Duration) -> Self { + Self { + timer, + delay, + completed: Arc::new(AtomicBool::new(false)), + } + } + fn completed(&self) -> bool { + self.completed.load(Ordering::Relaxed) + } + } + impl Service> for DelayLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let timer = self.timer.clone(); + let delay = self.delay; + let completed = self.completed.clone(); + async move { + timer.sleep(delay).await; + completed.store(true, Ordering::Relaxed); + Ok(http::Response::new(StubBody::new(b"slow"))) + } + } + } + + // An inline leaf returning a `Connection` error immediately. + #[derive(Clone)] + struct ErrLeaf; + impl Service> for ErrLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Err(HttpError::connection("reset")) } + } + } + + fn req(override_to: Option) -> http::Request { + let mut r = http::Request::new(Bytes::new()); + if let Some(d) = override_to { + r.extensions_mut().insert(RequestTimeout(d)); + } + r + } + + #[tokio::test] + async fn fast_inner_passes_and_body_is_transparent() { + let svc = TimeoutLayer::new(Duration::from_secs(1), MockTimer::new()) + .layer(FastLeaf { body: b"ok" }); + let resp = svc.call(req(None)).await.expect("inner within deadline"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // Response passed straight through + } + + #[tokio::test] + async fn slow_inner_times_out_at_default() { + let timer = MockTimer::new(); + let leaf = DelayLeaf::new(timer.clone(), Duration::from_secs(3600)); + let leaf_probe = leaf.clone(); + let svc = TimeoutLayer::new(Duration::from_secs(1), timer.clone()).layer(leaf); + let waiter = tokio::spawn(async move { svc.call(req(None)).await }); + tokio::task::yield_now().await; // task registers inner sleep(3600s) + deadline sleep(1s) + timer.advance(Duration::from_secs(1)); // fire the layer deadline + let err = waiter.await.unwrap().unwrap_err(); + assert!(matches!(err, HttpError::Timeout)); // HttpError has no PartialEq + assert!( + !leaf_probe.completed(), + "inner future must be dropped, not run to completion" + ); + } + + #[tokio::test] + async fn request_timeout_override_shortens_deadline() { + // Layer default is huge; a per-request 1s override fires first. + let timer = MockTimer::new(); + let svc = TimeoutLayer::new(Duration::from_secs(3600), timer.clone()) + .layer(DelayLeaf::new(timer.clone(), Duration::from_secs(3600))); + let waiter = tokio::spawn(async move { svc.call(req(Some(Duration::from_secs(1)))).await }); + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // fires the override, not the default + let err = waiter.await.unwrap().unwrap_err(); + assert!(matches!(err, HttpError::Timeout)); + } + + #[tokio::test] + async fn request_timeout_override_lengthens_deadline() { + // Default 1s would time out; a 5s override lets the 2s inner complete. + let timer = MockTimer::new(); + let svc = TimeoutLayer::new(Duration::from_secs(1), timer.clone()) + .layer(DelayLeaf::new(timer.clone(), Duration::from_secs(2))); + let waiter = tokio::spawn(async move { svc.call(req(Some(Duration::from_secs(5)))).await }); + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // now=1s: neither the 2s inner nor the 5s override is due + tokio::task::yield_now().await; + timer.advance(Duration::from_secs(1)); // now=2s: the inner completes first + let resp = waiter + .await + .unwrap() + .expect("override outlived the default; inner completed"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"slow")); + } + + #[tokio::test] + async fn inner_error_passes_through_not_masked_as_timeout() { + let svc = TimeoutLayer::new(Duration::from_secs(1), MockTimer::new()).layer(ErrLeaf); + let err = svc.call(req(None)).await.unwrap_err(); + assert!(matches!(err, HttpError::Connection(_))); // its own error, never Timeout + } + + #[tokio::test] + async fn zero_default_still_returns_ready_inner() { + // `select` polls the inner call first, so a ready inner is never + // preempted by a Duration::ZERO deadline. + let svc = + TimeoutLayer::new(Duration::ZERO, MockTimer::new()).layer(FastLeaf { body: b"ok" }); + svc.call(req(None)) + .await + .expect("inner polled first, not the zero deadline"); + } } From 7b760940136c233da4cbe1e81e81ffc3ff8b7cdb Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:19:44 +0000 Subject: [PATCH 5/6] docs(net): record Timeout layer amendment (ADR-0034) + changelog Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++++++++ ...onstruction-surface-auth-guarded-boot-coverage.md | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 564cc35..9a85617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 TokenBucket` gains `per: Duration` for sub-1/second venue limits, and the ≤1-concurrency-permit invariant is a boot check (`BuildError::MultipleConcurrency`). (ADR-0031 §3–4.) +- `oath-adapter-net-http-api` `Timeout` resilience layer (Slice 1 PR 2) — the + `Timeout` service + `TimeoutLayer` factory (`net-api::Layer`): bounds the + send (inner call → response) against a `net-api::Timer` deadline, returning + `HttpError::Timeout` when it elapses first (inner future dropped); body-transparent. + Adds the `RequestTimeout(Duration)` per-request override extension (absent → the + layer default). Response-future-only (ADR-0031 §1's "bounds the send, not the permit + wait"); a streaming-body timeout is deferred. No new dependency. (ADR-0031 §1, + ADR-0034.) - net-http construction-surface design refinements (ADR-0034 append-only Amendments 2026-07-04, spec updated) — an absent `RateLimit` directive now **fails closed** (not "defaults to `Global`"), closing the last silent diff --git a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md index 7fb55a2..f50a8c5 100644 --- a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md +++ b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md @@ -162,3 +162,15 @@ carries the full reasoning. `RateLimit` sketch, which collided with the layer name). The ≤1-concurrency-permit invariant (`Guarded` holds one) is enforced at construction by `BuildError::MultipleConcurrency` / `validate_concurrency_singleton`. + +6. **`Timeout` layer (Slice 1 PR 2).** The `Timeout` layer + `TimeoutLayer` + factory bound the **send** (`inner.call` → response), not the pacing-permit wait + (ADR-0031 §1) — a response-future race against `Timer::sleep`, `HttpError::Timeout` + on the deadline (inner future dropped). Body-transparent: `http::Response` is + returned untouched (no `Guarded`-style carrier, no `B: Body` bound). A per-request + `RequestTimeout(Duration)` extension overrides the layer default; an **absent** + extension uses the default (not fail-closed, unlike `RateScope` — a missing override + has no fail-open pacing hazard, the global deadline still applies). A `TimeoutBody` + bounding a *streaming* transfer's mid-stream stall is **deferred**: it is inert on + IBKR's all-buffered responses (a `Buffered` body is already in memory when `call` + returns) and lands additively when a streaming venue first needs it. From 981ad678c0bf49dfc569dfb6daea23189e6ed505 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:44:10 +0000 Subject: [PATCH 6/6] docs(net): document RequestTimeout's public field (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a field-level doc comment to RequestTimeout(pub Duration). Note: this was NOT a CI failure — missing_docs does not lint positional tuple-struct fields (just ci was green with it undocumented), contrary to the review's premise — but documenting the public field is a harmless improvement. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/adapter/net/http/api/src/timeout.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/adapter/net/http/api/src/timeout.rs b/crates/adapter/net/http/api/src/timeout.rs index 8e5abc8..efcd459 100644 --- a/crates/adapter/net/http/api/src/timeout.rs +++ b/crates/adapter/net/http/api/src/timeout.rs @@ -28,7 +28,10 @@ use std::time::Duration; /// fail-open hazard (the global deadline still applies), so it is not rejected /// (contrast `RateScope`, ADR-0034 Amendment #1). #[derive(Debug, Clone, Copy)] -pub struct RequestTimeout(pub Duration); +pub struct RequestTimeout( + /// The deadline this request must complete within, overriding the layer default. + pub Duration, +); /// The `Timeout` [`Layer`] factory: holds the default deadline + clock and /// produces a [`Timeout`] around any inner service.