ADR-014: Shared reqwest::Client Per Service

On this page

Status

Accepted (2026-04-20)

Context

CRAIG services make outbound HTTP calls in several places:

  • craig-intake — CAPTCHA verification, convert-report → craig-cases, partner forwarding in standalone mode.

  • craig-exchange — 11 external-partner adapters × 2 operations each (send, test_connectivity).

  • craig-security — webhook notifications on critical alerts.

  • craig-cases — rules-engine evaluation calls.

  • craig-financial — no outbound calls currently (audited).

Before 2026-04-20 the codebase contained ~36 per-request reqwest::Client::new() call sites plus four TODO comments explicitly flagging the pool churn as technical debt.

Per-request Client::new() has three costs:

  • Connection pool churn — every call builds a fresh pool and tears it down when the Client is dropped. No keep-alive reuse across requests even when the target is the same.

  • Inconsistent timeouts — each call site picked its own timeout (5 s, 10 s, 30 s, sometimes none) based on what the author happened to remember. Adapter-to-CWCA and adapter-to-court silently differed.

  • No user-agent policy — default reqwest UA (reqwest/0.12.28) gave downstream partner server logs no way to distinguish which CRAIG service issued a call.

The Code Quality Review April 2026 plan (#181 Step 3) captured the cleanup as mechanical debt.

Decision

CRAIG services MUST:

  1. Build one reqwest::Client per service, at startup, via craig_common::build_shared_client(service_name, service_version).

  2. Inject that client into Axum via axum::Extension<reqwest::Client>.

  3. Extract via Extension(client): Extension<reqwest::Client> in handlers, or store as a struct field in adapters / sinks / long-lived collaborators.

  4. Clone rather than rebuild — reqwest::Client is internally Arc-backed, so all clones share one connection pool, one timeout policy, one user-agent string.

CRAIG services MUST NOT call reqwest::Client::new() or reqwest::Client::builder() in handler, adapter, sink, or worker code. The only legitimate callers of the builder are:

  • craig_common::build_shared_client itself (single source of truth).

  • Test code (fresh reqwest::Client::new() per test is fine — no pooling concern at test scale).

  • Bootstrap code in main.rs files that need a different timeout policy for a specialized purpose. In that case the reason must appear in a // Reason: comment adjacent to the builder.

build_shared_client currently produces a client with:

  • 30-second total request timeout

  • 5-second TCP connect timeout (per candidate address)

  • 32 idle connections per origin

  • <service-name>/<service-version> user-agent (from env!("CARGO_PKG_NAME") and env!("CARGO_PKG_VERSION"))

Services inherit the defaults; no knobs are exposed unless a future deployment needs differentiated pooling.

Rationale

  • Predictable performance — one connection pool means TCP keep-alives actually amortize. For the exchange adapter use case (repeated posts to the same partner endpoint) this is measurable.

  • Uniform timeout policy — a 30-second ceiling is applied consistently; incidents rooted in "one adapter blocked at 60 seconds" can’t happen.

  • Server-log attributability — downstream partners logging craig-exchange/0.1.0 as the user agent can tell which service initiated a request without needing a custom header.

  • Single place to update — when CRAIG needs to add TLS pinning, mTLS client certs, or a proxy, the change is one function.

  • Closes four TODOs explicitly documenting the deferred work.

Consequences

  • A new module crates/craig-common/src/http.rs exports build_shared_client. reqwest is a direct dep of craig-common, as it already was for telemetry.rs.

  • Every service main.rs calls build_shared_client and .layer(axum::extract::Extension(http_client)) on its router.

  • The ExchangeAdapter::adapter_for(partner_type, format, client) signature grew a client parameter (backwards-incompatible for anyone who wrote a custom factory, though the trait was internal).

  • Tests that construct adapters directly pass reqwest::Client::new() — fine; tests don’t hit the real network.

  • Clippy’s too_many_arguments tripped on convert_report (axum handler extractors count as args); suppressed with a // Reason: comment because the axum extractor pattern is idiomatic and consolidating them into a struct would defeat the dependency-injection model.

Enforcement

  • Build-time: workspace clippy + -D warnings catches accidental regressions when Client::new() produces lint warnings (via future #[deny(clippy::disallowed_methods)] configuration — see Follow-up below).

  • Review-time: CHANGELOG Step 3 (2026-04-20) documents the migration; reviewers rejecting a new Client::new() in a service call path can reference this ADR.

  • Grep gate (manual, pre-merge): grep -rn 'reqwest::Client::new()' services/ crates/ should show only test code, build_shared_client itself, and well-commented main.rs specialists.

Follow-up

  • Add clippy.toml with disallowed-methods = ["reqwest::Client::new", "reqwest::Client::builder"] scoped to service crates (dev-dep tests excluded) so regressions fail CI without reviewer vigilance.

  • Revisit timeout defaults before large-jurisdiction onboarding — a 30-second ceiling may be aggressive for some partner systems with known slow responses.

Related ADRs

  • ADR-003: RabbitMQ Topology — async message bus is the preferred inter-service transport; this ADR governs the remaining synchronous calls.

  • ADR-012: Service Modularity — inter-CRAIG URLs remain Option<Url> + 503; this ADR governs how the clients that consume those URLs are constructed.

  • ADR-010: Partner JWS Integrity — partner submissions use the same shared client, with JWS signatures verified downstream of the transport layer.

Edit this page · latest