From 4edb82233d57a2b4b17eee21d644127f5be3180a Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Wed, 22 Jul 2026 16:35:03 +1200 Subject: [PATCH 1/3] feat: connection pooling --- CHANGELOG.md | 19 +++- src/provider.rs | 84 ++++++++++++++++++ src/provider/anthropic.rs | 175 +++++++++++++++++++++++++++++++++++-- src/provider/gemini.rs | 172 ++++++++++++++++++++++++++++++++++-- src/provider/openai.rs | 177 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 602 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62ad1c3..07a5cf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. Gemini 3). Defaults to `false` — the Gemini API rejects `thinkingConfig` with `400 INVALID_ARGUMENT` on non-reasoning models, so the caller must opt in once they know their model supports thinking. +- HTTP connection-pool injection and tuning on all three provider builders + (`OpenAiClientBuilder`, `AnthropicClientBuilder`, `GeminiClientBuilder`): + `.http_client(reqwest::Client)` injects a shared client so multiple providers + can reuse one connection pool. `.pool_max_idle_per_host(usize)`, + `.pool_idle_timeout(Duration)`, and `.tcp_keepalive(Duration)` expose the + underlying `reqwest` pool knobs (default to reqwest's built-in defaults when + unset). When an injected client is used, these knobs and `.timeout()` / + `.connect_timeout()` are ignored — configure them on the injected client. - `ContextContributor` trait and `ContributorContext<'a>` (`engine::contributor` module): a write-side hook at the turn boundary. Implementors return an optional `Message` that the loop appends to the conversation before the next @@ -165,10 +173,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. ### Changed - **Breaking:** `stream::DeltaPart` is now `#[non_exhaustive]`. Every - `match` on `DeltaPart` in downstream code must include a `_ =>` wildcard arm - — handling `Thinking` explicitly is optional but does not replace the - wildcard requirement. Same-crate matches are unaffected. Future variant - additions (e.g. `Image`, `Audio`) will arrive non-breaking. + exhaustive `match` on `DeltaPart` in downstream code must add a `_ =>` arm + (or a `Thinking =>` arm for the new variant). Same-crate matches are + unaffected. Future variant additions (e.g. `Image`, `Audio`) will arrive + non-breaking. - **Breaking:** `ApiClient::stream_messages`, `stream_messages_with_options`, `create_message`, and `create_message_with_options` now take a single `StreamRequest` parameter instead of positional `(messages, system, tools)`. @@ -246,6 +254,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. between calls. Previously, a Ctrl-C during a multi-tool batch was only honored at the next turn boundary; now it aborts the remaining calls in the batch. +- Internally-built `reqwest::Client`s now set `tcp_nodelay(true)` by default. + SSE streaming emits many small packets; disabling Nagle's algorithm reduces + per-delta latency. No correctness impact. ### Removed diff --git a/src/provider.rs b/src/provider.rs index f7dd5f8..fb9720e 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -58,6 +58,90 @@ use crate::{ api::error::ApiError, message::{MessagePart, Role}, }; +use std::time::Duration; + +/// Extension trait for [`reqwest::ClientBuilder`] that accepts `Option` +/// for pool and TCP knobs, no-opping when `None`. +/// +/// Each provider builder stores pool/TCP settings as `Option` so that +/// `None` means "defer to reqwest's default." Without this trait, applying +/// those settings in `build()` requires three separate `if let Some(...)` +/// blocks. With it, the builder chain stays fluent: +/// +/// ```rust,ignore +/// reqwest::Client::builder() +/// .timeout(timeout) +/// .connect_timeout(connect_timeout) +/// .tcp_nodelay(true) +/// .maybe_pool_max_idle_per_host(pool_max_idle) +/// .maybe_pool_idle_timeout(pool_idle_timeout) +/// .maybe_tcp_keepalive(tcp_keepalive) +/// .build() +/// ``` +/// +/// When the value is `None`, the method returns the builder unchanged — +/// reqwest's built-in default is used. When `Some`, the value is forwarded +/// to the corresponding `reqwest::ClientBuilder` method. +pub(super) trait ClientBuilderExt: Sized { + /// Set the maximum number of idle connections kept alive per host. + /// + /// When `Some(n)`, forwards to + /// [`pool_max_idle_per_host`](reqwest::ClientBuilder::pool_max_idle_per_host). + /// When `None`, the builder is returned unchanged and reqwest's default + /// (unlimited) is used. + /// + /// Most callers leave this at `None`. Set to a small value (e.g. 1–4) + /// for memory-constrained runners or workloads that make mostly serial + /// requests to a single host. + fn maybe_pool_max_idle_per_host(self, val: Option) -> Self; + + /// Set how long an idle connection stays in the pool before being closed. + /// + /// When `Some(d)`, forwards to + /// [`pool_idle_timeout`](reqwest::ClientBuilder::pool_idle_timeout). + /// When `None`, the builder is returned unchanged and reqwest's default + /// (90 seconds) is used. + /// + /// Raise for long-idle interactive workloads to keep TLS sessions warm + /// between turns. Lower for tight batch jobs to free file descriptors + /// sooner. + fn maybe_pool_idle_timeout(self, val: Option) -> Self; + + /// Enable OS-level TCP keepalive at the given interval. + /// + /// When `Some(d)`, forwards to + /// [`tcp_keepalive`](reqwest::ClientBuilder::tcp_keepalive). When `None`, + /// the builder is returned unchanged and TCP keepalive stays disabled + /// (reqwest default). + /// + /// Enable (~60 seconds) if connections are silently dropped after idle + /// periods — e.g. behind aggressive NATs, firewalls, or load balancers + /// that reap idle TCP sessions without sending FIN. + fn maybe_tcp_keepalive(self, val: Option) -> Self; +} + +impl ClientBuilderExt for reqwest::ClientBuilder { + fn maybe_pool_max_idle_per_host(self, val: Option) -> Self { + match val { + Some(n) => self.pool_max_idle_per_host(n), + None => self, + } + } + + fn maybe_pool_idle_timeout(self, val: Option) -> Self { + match val { + Some(d) => self.pool_idle_timeout(Some(d)), + None => self, + } + } + + fn maybe_tcp_keepalive(self, val: Option) -> Self { + match val { + Some(d) => self.tcp_keepalive(d), + None => self, + } + } +} #[cfg(feature = "openai")] pub mod openai; diff --git a/src/provider/anthropic.rs b/src/provider/anthropic.rs index 10ecd81..3dd41a4 100644 --- a/src/provider/anthropic.rs +++ b/src/provider/anthropic.rs @@ -26,6 +26,7 @@ use reqwest::Response; use serde_json::Value; use std::time::Duration; +use super::ClientBuilderExt; use crate::api::ApiClient; use crate::api::error::ApiError; use crate::message::{Message, MessagePart, Role}; @@ -378,13 +379,50 @@ pub struct AnthropicClientBuilder { /// The total HTTP request timeout (connect + response + body). /// /// Bounds the entire request lifecycle. Defaults to 120 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client); configure it on that client instead. timeout: Duration, /// The TCP connection establishment timeout (including TLS handshake). /// /// Separate from the total timeout so a slow-connecting server can be /// detected faster than a slow-responding one. Defaults to 10 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). connect_timeout: Duration, + + /// A pre-built shared `reqwest::Client`, if injected via + /// [`http_client`](Self::http_client). When set, pool and TCP knobs are + /// ignored. + http: Option, + + /// Maximum idle connections kept alive per host. + /// + /// `None` defers to reqwest's default (unlimited). Set to a small value + /// (e.g. 1–4) for memory-constrained runners or workloads that make + /// mostly serial requests to a single host. + pool_max_idle_per_host: Option, + + /// How long an idle connection stays in the pool before being closed. + /// + /// `None` defers to reqwest's default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + pool_idle_timeout: Option, + + /// OS-level TCP keepalive interval. + /// + /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if + /// connections are silently dropped after idle periods (e.g. behind + /// aggressive NATs or load balancers). + tcp_keepalive: Option, + + /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). + /// + /// Defaults to `true` — SSE streaming emits many small packets, and + /// Nagle's algorithm coalesces them, adding latency per delta. No public + /// setter; always applied on internally-built clients. + tcp_nodelay: bool, } impl Default for AnthropicClientBuilder { @@ -396,6 +434,11 @@ impl Default for AnthropicClientBuilder { max_tokens: DEFAULT_MAX_TOKENS, timeout: DEFAULT_REQUEST_TIMEOUT, connect_timeout: DEFAULT_CONNECT_TIMEOUT, + http: None, + pool_max_idle_per_host: None, + pool_idle_timeout: None, + tcp_keepalive: None, + tcp_nodelay: true, } } } @@ -460,12 +503,67 @@ impl AnthropicClientBuilder { /// /// Defaults to 10 seconds. This is the maximum time to wait for the TCP /// connection (including TLS handshake) to be established. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). #[must_use] pub fn connect_timeout(mut self, timeout: Duration) -> Self { self.connect_timeout = timeout; self } + /// Inject a pre-built, shared `reqwest::Client`. + /// + /// When set, the client's connection pool is shared with every other + /// provider built from the same handle, and the pool/TCP knobs are + /// ignored. Configure timeouts on the injected client, not here. + #[must_use] + pub fn http_client(mut self, client: reqwest::Client) -> Self { + self.http = Some(client); + self + } + + /// Set the maximum idle connections kept alive per host. + /// + /// Defaults to reqwest's built-in default (unlimited). Set to a small + /// value (e.g. 1–4) for memory-constrained runners or workloads that + /// make mostly serial requests to a single host. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { + self.pool_max_idle_per_host = Some(n); + self + } + + /// Set how long an idle connection stays in the pool before being closed. + /// + /// Defaults to reqwest's built-in default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_idle_timeout(mut self, d: Duration) -> Self { + self.pool_idle_timeout = Some(d); + self + } + + /// Set the OS-level TCP keepalive interval. + /// + /// Defaults to disabled (reqwest default). Enable (~60s) if connections + /// are silently dropped after idle periods (e.g. behind aggressive NATs + /// or load balancers). + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn tcp_keepalive(mut self, d: Duration) -> Self { + self.tcp_keepalive = Some(d); + self + } + /// Construct the [`AnthropicClient`] from the builder's configuration. /// /// Creates the internal `reqwest::Client` with the configured timeouts, @@ -479,11 +577,18 @@ impl AnthropicClientBuilder { let api_key = self .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - let http = reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .build() - .map_err(|e| ApiError::http(e.to_string()))?; + let http = match self.http { + Some(shared) => shared, + None => reqwest::Client::builder() + .timeout(self.timeout) + .connect_timeout(self.connect_timeout) + .tcp_nodelay(self.tcp_nodelay) + .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) + .maybe_pool_idle_timeout(self.pool_idle_timeout) + .maybe_tcp_keepalive(self.tcp_keepalive) + .build() + .map_err(|e| ApiError::http(e.to_string()))?, + }; Ok(AnthropicClient { http, @@ -1875,9 +1980,6 @@ mod tests { #[test] fn builder_timeouts_applied_on_build() { - // Verify the build succeeds — reqwest validates the configuration - // internally. If timeout/connect_timeout were somehow invalid, - // .build() would return an error. let client = AnthropicClient::builder() .api_key("sk-test") .timeout(Duration::from_mins(3)) @@ -1886,6 +1988,63 @@ mod tests { assert!(client.is_ok(), "build should succeed with valid timeouts"); } + #[test] + fn builder_accepts_injected_http_client() { + let shared = reqwest::Client::new(); + let client_a = AnthropicClient::builder() + .api_key("sk-a") + .http_client(shared.clone()) + .build(); + let client_b = AnthropicClient::builder() + .api_key("sk-b") + .http_client(shared) + .build(); + assert!(client_a.is_ok() && client_b.is_ok()); + } + + #[test] + fn injected_client_supersedes_builder_timeouts() { + let shared = reqwest::Client::builder() + .timeout(Duration::from_secs(1)) + .build() + .unwrap(); + let client = AnthropicClient::builder() + .api_key("sk-test") + .http_client(shared) + .timeout(Duration::from_secs(99)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn pool_knobs_build_clean() { + let client = AnthropicClient::builder() + .api_key("sk-test") + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .tcp_keepalive(Duration::from_secs(90)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn tcp_nodelay_default_is_true() { + let builder = AnthropicClientBuilder::default(); + assert!(builder.tcp_nodelay); + } + + #[test] + fn injected_client_ignores_pool_knobs() { + let shared = reqwest::Client::new(); + let client = AnthropicClient::builder() + .api_key("sk-test") + .http_client(shared) + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .build(); + assert!(client.is_ok()); + } + #[tokio::test] async fn sse_reader_take_line_splits_on_newline() { let mut reader = SseReader { diff --git a/src/provider/gemini.rs b/src/provider/gemini.rs index 27513f1..22bbc62 100644 --- a/src/provider/gemini.rs +++ b/src/provider/gemini.rs @@ -27,6 +27,7 @@ use reqwest::Response; use serde_json::Value; use std::time::Duration; +use super::ClientBuilderExt; use crate::api::ApiClient; use crate::api::error::ApiError; use crate::message::{Message, MessagePart, Role}; @@ -426,12 +427,16 @@ pub struct GeminiClientBuilder { /// The total HTTP request timeout (connect + response + body). /// /// Bounds the entire request lifecycle. Defaults to 120 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client); configure it on that client instead. timeout: Duration, /// The TCP connection establishment timeout (including TLS handshake). /// /// Separate from the total timeout so a slow-connecting server can be /// detected faster. Defaults to 10 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). connect_timeout: Duration, /// Whether to opt into thought summaries from reasoning-capable models. @@ -442,6 +447,39 @@ pub struct GeminiClientBuilder { /// `400 INVALID_ARGUMENT`, so this is `false` by default and the caller /// must opt in once they know their model supports thinking. include_thoughts: bool, + + /// A pre-built shared `reqwest::Client`, if injected via + /// [`http_client`](Self::http_client). When set, pool and TCP knobs are + /// ignored. + http: Option, + + /// Maximum idle connections kept alive per host. + /// + /// `None` defers to reqwest's default (unlimited). Set to a small value + /// (e.g. 1–4) for memory-constrained runners or workloads that make + /// mostly serial requests to a single host. + pool_max_idle_per_host: Option, + + /// How long an idle connection stays in the pool before being closed. + /// + /// `None` defers to reqwest's default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + pool_idle_timeout: Option, + + /// OS-level TCP keepalive interval. + /// + /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if + /// connections are silently dropped after idle periods (e.g. behind + /// aggressive NATs or load balancers). + tcp_keepalive: Option, + + /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). + /// + /// Defaults to `true` — SSE streaming emits many small packets, and + /// Nagle's algorithm coalesces them, adding latency per delta. No public + /// setter; always applied on internally-built clients. + tcp_nodelay: bool, } impl Default for GeminiClientBuilder { @@ -453,6 +491,11 @@ impl Default for GeminiClientBuilder { timeout: DEFAULT_REQUEST_TIMEOUT, connect_timeout: DEFAULT_CONNECT_TIMEOUT, include_thoughts: false, + http: None, + pool_max_idle_per_host: None, + pool_idle_timeout: None, + tcp_keepalive: None, + tcp_nodelay: true, } } } @@ -505,6 +548,8 @@ impl GeminiClientBuilder { /// /// Defaults to 10 seconds. This is the maximum time to wait for the TCP /// connection (including TLS handshake) to be established. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). #[must_use] pub fn connect_timeout(mut self, timeout: Duration) -> Self { self.connect_timeout = timeout; @@ -529,6 +574,59 @@ impl GeminiClientBuilder { self } + /// Inject a pre-built, shared `reqwest::Client`. + /// + /// When set, the client's connection pool is shared with every other + /// provider built from the same handle, and the pool/TCP knobs are + /// ignored. Configure timeouts on the injected client, not here. + #[must_use] + pub fn http_client(mut self, client: reqwest::Client) -> Self { + self.http = Some(client); + self + } + + /// Set the maximum idle connections kept alive per host. + /// + /// Defaults to reqwest's built-in default (unlimited). Set to a small + /// value (e.g. 1–4) for memory-constrained runners or workloads that + /// make mostly serial requests to a single host. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { + self.pool_max_idle_per_host = Some(n); + self + } + + /// Set how long an idle connection stays in the pool before being closed. + /// + /// Defaults to reqwest's built-in default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_idle_timeout(mut self, d: Duration) -> Self { + self.pool_idle_timeout = Some(d); + self + } + + /// Set the OS-level TCP keepalive interval. + /// + /// Defaults to disabled (reqwest default). Enable (~60s) if connections + /// are silently dropped after idle periods (e.g. behind aggressive NATs + /// or load balancers). + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn tcp_keepalive(mut self, d: Duration) -> Self { + self.tcp_keepalive = Some(d); + self + } + /// Build the client. /// /// # Errors @@ -538,11 +636,18 @@ impl GeminiClientBuilder { let api_key = self .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - let http = reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .build() - .map_err(|e| ApiError::http(e.to_string()))?; + let http = match self.http { + Some(shared) => shared, + None => reqwest::Client::builder() + .timeout(self.timeout) + .connect_timeout(self.connect_timeout) + .tcp_nodelay(self.tcp_nodelay) + .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) + .maybe_pool_idle_timeout(self.pool_idle_timeout) + .maybe_tcp_keepalive(self.tcp_keepalive) + .build() + .map_err(|e| ApiError::http(e.to_string()))?, + }; Ok(GeminiClient { http, @@ -2002,6 +2107,63 @@ mod tests { assert!(client.is_ok(), "build should succeed with valid timeouts"); } + #[test] + fn builder_accepts_injected_http_client() { + let shared = reqwest::Client::new(); + let client_a = GeminiClient::builder() + .api_key("key-a") + .http_client(shared.clone()) + .build(); + let client_b = GeminiClient::builder() + .api_key("key-b") + .http_client(shared) + .build(); + assert!(client_a.is_ok() && client_b.is_ok()); + } + + #[test] + fn injected_client_supersedes_builder_timeouts() { + let shared = reqwest::Client::builder() + .timeout(Duration::from_secs(1)) + .build() + .unwrap(); + let client = GeminiClient::builder() + .api_key("test-key") + .http_client(shared) + .timeout(Duration::from_secs(99)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn pool_knobs_build_clean() { + let client = GeminiClient::builder() + .api_key("test-key") + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .tcp_keepalive(Duration::from_secs(90)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn tcp_nodelay_default_is_true() { + let builder = GeminiClientBuilder::default(); + assert!(builder.tcp_nodelay); + } + + #[test] + fn injected_client_ignores_pool_knobs() { + let shared = reqwest::Client::new(); + let client = GeminiClient::builder() + .api_key("test-key") + .http_client(shared) + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .build(); + assert!(client.is_ok()); + } + #[test] fn builder_include_thoughts_defaults_false() { let client = GeminiClient::builder().api_key("test-key").build().unwrap(); diff --git a/src/provider/openai.rs b/src/provider/openai.rs index e0e8bef..38e19a1 100644 --- a/src/provider/openai.rs +++ b/src/provider/openai.rs @@ -29,6 +29,7 @@ use reqwest::Response; use serde::Deserialize; use serde_json::Value; +use super::ClientBuilderExt; use crate::api::ApiClient; use crate::api::error::ApiError; use crate::message::{Message, MessagePart, Role}; @@ -436,13 +437,50 @@ pub struct OpenAiClientBuilder { /// The total HTTP request timeout (connect + response + body). /// /// Bounds the entire request lifecycle. Defaults to 120 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client); configure it on that client instead. timeout: Duration, /// The TCP connection establishment timeout (including TLS handshake). /// /// Separate from the total timeout so a slow-connecting server can be /// detected faster. Defaults to 10 seconds. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). connect_timeout: Duration, + + /// A pre-built shared `reqwest::Client`, if injected via + /// [`http_client`](Self::http_client). When set, pool and TCP knobs are + /// ignored. + http: Option, + + /// Maximum idle connections kept alive per host. + /// + /// `None` defers to reqwest's default (unlimited). Set to a small value + /// (e.g. 1–4) for memory-constrained runners or workloads that make + /// mostly serial requests to a single host. + pool_max_idle_per_host: Option, + + /// How long an idle connection stays in the pool before being closed. + /// + /// `None` defers to reqwest's default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + pool_idle_timeout: Option, + + /// OS-level TCP keepalive interval. + /// + /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if + /// connections are silently dropped after idle periods (e.g. behind + /// aggressive NATs or load balancers). + tcp_keepalive: Option, + + /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). + /// + /// Defaults to `true` — SSE streaming emits many small packets, and + /// Nagle's algorithm coalesces them, adding latency per delta. No public + /// setter; always applied on internally-built clients. + tcp_nodelay: bool, } impl Default for OpenAiClientBuilder { @@ -453,6 +491,11 @@ impl Default for OpenAiClientBuilder { model: DEFAULT_MODEL.into(), timeout: DEFAULT_REQUEST_TIMEOUT, connect_timeout: DEFAULT_CONNECT_TIMEOUT, + http: None, + pool_max_idle_per_host: None, + pool_idle_timeout: None, + tcp_keepalive: None, + tcp_nodelay: true, } } } @@ -506,12 +549,69 @@ impl OpenAiClientBuilder { /// /// Defaults to 10 seconds. This is the maximum time to wait for the TCP /// connection (including TLS handshake) to be established. + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). #[must_use] pub fn connect_timeout(mut self, timeout: Duration) -> Self { self.connect_timeout = timeout; self } + /// Inject a pre-built, shared `reqwest::Client`. + /// + /// When set, the client's connection pool is shared with every other + /// provider built from the same handle, and the pool/TCP knobs below + /// (`pool_*`, `tcp_*`) are ignored — they are only applied when the + /// builder constructs its own client. Configure timeouts on the injected + /// client (`reqwest::Client::builder().timeout(...)`), not here. + #[must_use] + pub fn http_client(mut self, client: reqwest::Client) -> Self { + self.http = Some(client); + self + } + + /// Set the maximum idle connections kept alive per host. + /// + /// Defaults to reqwest's built-in default (unlimited). Set to a small + /// value (e.g. 1–4) for memory-constrained runners or workloads that + /// make mostly serial requests to a single host. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { + self.pool_max_idle_per_host = Some(n); + self + } + + /// Set how long an idle connection stays in the pool before being closed. + /// + /// Defaults to reqwest's built-in default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn pool_idle_timeout(mut self, d: Duration) -> Self { + self.pool_idle_timeout = Some(d); + self + } + + /// Set the OS-level TCP keepalive interval. + /// + /// Defaults to disabled (reqwest default). Enable (~60s) if connections + /// are silently dropped after idle periods (e.g. behind aggressive NATs + /// or load balancers). + /// + /// Ignored when a client was supplied via + /// [`http_client`](Self::http_client). + #[must_use] + pub fn tcp_keepalive(mut self, d: Duration) -> Self { + self.tcp_keepalive = Some(d); + self + } + /// Build the client. /// /// # Errors @@ -522,11 +622,18 @@ impl OpenAiClientBuilder { .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - let http = reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .build() - .map_err(|e| ApiError::http(e.to_string()))?; + let http = match self.http { + Some(shared) => shared, + None => reqwest::Client::builder() + .timeout(self.timeout) + .connect_timeout(self.connect_timeout) + .tcp_nodelay(self.tcp_nodelay) + .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) + .maybe_pool_idle_timeout(self.pool_idle_timeout) + .maybe_tcp_keepalive(self.tcp_keepalive) + .build() + .map_err(|e| ApiError::http(e.to_string()))?, + }; Ok(OpenAiClient { http, @@ -1795,9 +1902,6 @@ mod tests { #[test] fn builder_timeouts_applied_on_build() { - // Verify the build succeeds — reqwest validates the configuration - // internally. If timeout/connect_timeout were somehow invalid, - // .build() would return an error. let client = OpenAiClient::builder() .api_key("sk-test") .timeout(Duration::from_mins(3)) @@ -1806,6 +1910,63 @@ mod tests { assert!(client.is_ok(), "build should succeed with valid timeouts"); } + #[test] + fn builder_accepts_injected_http_client() { + let shared = reqwest::Client::new(); + let client_a = OpenAiClient::builder() + .api_key("sk-a") + .http_client(shared.clone()) + .build(); + let client_b = OpenAiClient::builder() + .api_key("sk-b") + .http_client(shared) + .build(); + assert!(client_a.is_ok() && client_b.is_ok()); + } + + #[test] + fn injected_client_supersedes_builder_timeouts() { + let shared = reqwest::Client::builder() + .timeout(Duration::from_secs(1)) + .build() + .unwrap(); + let client = OpenAiClient::builder() + .api_key("sk-test") + .http_client(shared) + .timeout(Duration::from_secs(99)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn pool_knobs_build_clean() { + let client = OpenAiClient::builder() + .api_key("sk-test") + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .tcp_keepalive(Duration::from_secs(90)) + .build(); + assert!(client.is_ok()); + } + + #[test] + fn tcp_nodelay_default_is_true() { + let builder = OpenAiClientBuilder::default(); + assert!(builder.tcp_nodelay); + } + + #[test] + fn injected_client_ignores_pool_knobs() { + let shared = reqwest::Client::new(); + let client = OpenAiClient::builder() + .api_key("sk-test") + .http_client(shared) + .pool_max_idle_per_host(4) + .pool_idle_timeout(Duration::from_secs(30)) + .build(); + assert!(client.is_ok()); + } + #[tokio::test] async fn sse_reader_take_line_splits_on_newline() { let mut reader = SseReader { From 48fc42941a64b66e7315053c80b823c6362ad068 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Wed, 22 Jul 2026 21:41:23 +1200 Subject: [PATCH 2/3] feat: shared http client config, with_ prefix for client builder --- CHANGELOG.md | 5 + examples/chat.rs | 6 +- src/provider.rs | 302 ++++++++++++++++++++++++++++++-------- src/provider/anthropic.rs | 283 ++++++++--------------------------- src/provider/gemini.rs | 295 +++++++++---------------------------- src/provider/openai.rs | 281 ++++++++--------------------------- 6 files changed, 432 insertions(+), 740 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07a5cf0..9efd2cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,6 +196,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. - **Breaking:** `StreamHandler::stream_turn` now takes `options: RequestOptions` as an explicit parameter. `StreamHandler::with_request_options` is removed — use `BareLoop::set_request_options` instead. +- **Breaking:** All three provider builders (`OpenAiClientBuilder`, + `AnthropicClientBuilder`, `GeminiClientBuilder`) now use `with_` prefix on + consuming builder methods (e.g. `.with_api_key()`, `.with_model()`, + `.with_timeout()`, `.with_http_client()`). The old no-prefix names + (`.api_key()`, `.model()`, etc.) are removed. - **Breaking:** `ToolOutput`, `ToolDispatchResult`, and `ToolPostContext` are now `#[non_exhaustive]`, matching `DisplayHint`. Downstream code that constructs these via struct literal must switch to the named constructors diff --git a/examples/chat.rs b/examples/chat.rs index 8435fb2..bebc4c1 100644 --- a/examples/chat.rs +++ b/examples/chat.rs @@ -476,9 +476,9 @@ async fn main() { let client = build_or_die!( loopctl::provider::OpenAiClient::builder() - .api_key("ollama") - .base_url(base) - .model(model) + .with_api_key("ollama") + .with_base_url(base) + .with_model(model) .build(), "Ollama" ); diff --git a/src/provider.rs b/src/provider.rs index fb9720e..bf90923 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -60,63 +60,191 @@ use crate::{ }; use std::time::Duration; -/// Extension trait for [`reqwest::ClientBuilder`] that accepts `Option` -/// for pool and TCP knobs, no-opping when `None`. -/// -/// Each provider builder stores pool/TCP settings as `Option` so that -/// `None` means "defer to reqwest's default." Without this trait, applying -/// those settings in `build()` requires three separate `if let Some(...)` -/// blocks. With it, the builder chain stays fluent: -/// -/// ```rust,ignore -/// reqwest::Client::builder() -/// .timeout(timeout) -/// .connect_timeout(connect_timeout) -/// .tcp_nodelay(true) -/// .maybe_pool_max_idle_per_host(pool_max_idle) -/// .maybe_pool_idle_timeout(pool_idle_timeout) -/// .maybe_tcp_keepalive(tcp_keepalive) -/// .build() -/// ``` -/// -/// When the value is `None`, the method returns the builder unchanged — -/// reqwest's built-in default is used. When `Some`, the value is forwarded -/// to the corresponding `reqwest::ClientBuilder` method. -pub(super) trait ClientBuilderExt: Sized { - /// Set the maximum number of idle connections kept alive per host. +/// Shared HTTP-client configuration embedded by every provider builder. +/// +/// Holds the timeout, connection-pool, and TCP knobs that are identical +/// across [`OpenAiClient`](crate::provider::OpenAiClient), +/// [`AnthropicClient`](crate::provider::AnthropicClient), and +/// [`GeminiClient`](crate::provider::GeminiClient). Each provider builder +/// embeds this struct and delegates its HTTP-related setters to it, so the +/// pool/TCP documentation and construction logic lives in one place. +#[derive(Clone)] +pub(super) struct HttpClientConfig { + /// The total HTTP request timeout (connect + response + body). /// - /// When `Some(n)`, forwards to - /// [`pool_max_idle_per_host`](reqwest::ClientBuilder::pool_max_idle_per_host). - /// When `None`, the builder is returned unchanged and reqwest's default - /// (unlimited) is used. + /// Bounds the entire request lifecycle — a hanging server will be + /// aborted after this duration rather than blocking the agent loop + /// indefinitely. Defaults to 120 seconds. /// - /// Most callers leave this at `None`. Set to a small value (e.g. 1–4) - /// for memory-constrained runners or workloads that make mostly serial - /// requests to a single host. - fn maybe_pool_max_idle_per_host(self, val: Option) -> Self; + /// Ignored when an external client is supplied via + /// [`with_http_client`](Self::with_http_client); configure it on that + /// client instead. + timeout: Duration, + + /// The TCP connection establishment timeout (including TLS handshake). + /// + /// Separate from the total timeout so a slow-connecting server can be + /// detected faster than a slow-responding one. Defaults to 10 seconds. + /// + /// Ignored when an external client is supplied via + /// [`with_http_client`](Self::with_http_client). + connect_timeout: Duration, + + /// A pre-built, shared `reqwest::Client`, if injected via + /// [`with_http_client`](Self::with_http_client). + /// + /// When set, the client's connection pool is shared with every other + /// provider built from the same handle, and the pool/TCP knobs below + /// (`pool_*`, `tcp_*`) are ignored — they are only applied when the + /// builder constructs its own client. Configure timeouts on the injected + /// client, not here. + http: Option, + + /// Maximum idle connections kept alive per host. + /// + /// `None` defers to reqwest's default (unlimited). Set to a small value + /// (e.g. 1–4) for memory-constrained runners or workloads that make + /// mostly serial requests to a single host. + pool_max_idle_per_host: Option, + + /// How long an idle connection stays in the pool before being closed. + /// + /// `None` defers to reqwest's default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. + pool_idle_timeout: Option, + + /// OS-level TCP keepalive interval. + /// + /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if + /// connections are silently dropped after idle periods (e.g. behind + /// aggressive NATs or load balancers). + tcp_keepalive: Option, + + /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). + /// + /// Defaults to `true` — SSE streaming emits many small packets, and + /// Nagle's algorithm coalesces them, adding latency per delta. + tcp_nodelay: bool, +} + +impl Default for HttpClientConfig { + fn default() -> Self { + Self { + timeout: Duration::from_mins(2), + connect_timeout: Duration::from_secs(10), + http: None, + pool_max_idle_per_host: None, + pool_idle_timeout: None, + tcp_keepalive: None, + tcp_nodelay: true, + } + } +} + +impl HttpClientConfig { + /// Set the total request timeout. + /// + /// Ignored when an external client was supplied via + /// [`with_http_client`](Self::with_http_client). + #[must_use] + pub(super) fn with_timeout(mut self, timeout: Duration) -> Self { + self.timeout = timeout; + self + } + + /// Set the TCP connection establishment timeout. + /// + /// Ignored when an external client was supplied. + #[must_use] + pub(super) fn with_connect_timeout(mut self, timeout: Duration) -> Self { + self.connect_timeout = timeout; + self + } + + /// Inject a pre-built, shared `reqwest::Client`. + /// + /// When set, the client's connection pool is shared with every other + /// provider built from the same handle, and the pool/TCP knobs are + /// ignored. Configure timeouts on the injected client, not here. + #[must_use] + pub(super) fn with_http_client(mut self, client: reqwest::Client) -> Self { + self.http = Some(client); + self + } + + /// Set the maximum idle connections kept alive per host. + /// + /// Defaults to reqwest's built-in default (unlimited). Set to a small + /// value (e.g. 1–4) for memory-constrained runners or workloads that + /// make mostly serial requests to a single host. + /// + /// Ignored when an external client was supplied. + #[must_use] + pub(super) fn with_pool_max_idle_per_host(mut self, n: usize) -> Self { + self.pool_max_idle_per_host = Some(n); + self + } /// Set how long an idle connection stays in the pool before being closed. /// - /// When `Some(d)`, forwards to - /// [`pool_idle_timeout`](reqwest::ClientBuilder::pool_idle_timeout). - /// When `None`, the builder is returned unchanged and reqwest's default - /// (90 seconds) is used. + /// Defaults to reqwest's built-in default (90s). Raise for long-idle + /// interactive workloads to keep TLS sessions warm; lower for tight + /// batch jobs to free file descriptors sooner. /// - /// Raise for long-idle interactive workloads to keep TLS sessions warm - /// between turns. Lower for tight batch jobs to free file descriptors - /// sooner. - fn maybe_pool_idle_timeout(self, val: Option) -> Self; + /// Ignored when an external client was supplied. + #[must_use] + pub(super) fn with_pool_idle_timeout(mut self, d: Duration) -> Self { + self.pool_idle_timeout = Some(d); + self + } - /// Enable OS-level TCP keepalive at the given interval. + /// Set the OS-level TCP keepalive interval. + /// + /// Defaults to disabled (reqwest default). Enable (~60s) if connections + /// are silently dropped after idle periods (e.g. behind aggressive NATs + /// or load balancers). + /// + /// Ignored when an external client was supplied. + #[must_use] + pub(super) fn with_tcp_keepalive(mut self, d: Duration) -> Self { + self.tcp_keepalive = Some(d); + self + } + + /// Build a `reqwest::Client` from this configuration. + /// + /// If an external client was supplied via + /// [`http_client`](Self::http_client), it is returned verbatim. Otherwise + /// a new client is constructed with the configured timeouts, pool knobs, + /// and `tcp_nodelay(true)`. /// - /// When `Some(d)`, forwards to - /// [`tcp_keepalive`](reqwest::ClientBuilder::tcp_keepalive). When `None`, - /// the builder is returned unchanged and TCP keepalive stays disabled - /// (reqwest default). + /// # Errors /// - /// Enable (~60 seconds) if connections are silently dropped after idle - /// periods — e.g. behind aggressive NATs, firewalls, or load balancers - /// that reap idle TCP sessions without sending FIN. + /// Returns [`ApiError`] if `reqwest::Client::builder().build()` fails. + pub(super) fn build(self) -> Result { + match self.http { + Some(shared) => Ok(shared), + None => reqwest::Client::builder() + .timeout(self.timeout) + .connect_timeout(self.connect_timeout) + .tcp_nodelay(self.tcp_nodelay) + .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) + .maybe_pool_idle_timeout(self.pool_idle_timeout) + .maybe_tcp_keepalive(self.tcp_keepalive) + .build() + .map_err(|e| ApiError::http(e.to_string())), + } + } +} + +/// Extension trait for [`reqwest::ClientBuilder`] that accepts `Option` +/// for pool and TCP knobs, no-opping when `None`. +/// +/// Used internally by [`HttpClientConfig::build`]. +trait ClientBuilderExt: Sized { + fn maybe_pool_max_idle_per_host(self, val: Option) -> Self; + fn maybe_pool_idle_timeout(self, val: Option) -> Self; fn maybe_tcp_keepalive(self, val: Option) -> Self; } @@ -307,9 +435,9 @@ pub fn ollama(model: &str) -> Result { let api_key = env_or_default("OLLAMA_API_KEY", "ollama"); OpenAiClient::builder() - .api_key(api_key) - .base_url(base) - .model(model) + .with_api_key(api_key) + .with_base_url(base) + .with_model(model) .build() } @@ -335,9 +463,9 @@ pub fn deepseek() -> Result { let model = env_or_default("DEEPSEEK_MODEL", DEEPSEEK_DEFAULT_MODEL); OpenAiClient::builder() - .api_key(api_key) - .base_url(DEEPSEEK_BASE_URL) - .model(model) + .with_api_key(api_key) + .with_base_url(DEEPSEEK_BASE_URL) + .with_model(model) .build() } @@ -363,9 +491,9 @@ pub fn grok() -> Result { let model = env_or_default("GROK_MODEL", GROK_DEFAULT_MODEL); OpenAiClient::builder() - .api_key(api_key) - .base_url(GROK_BASE_URL) - .model(model) + .with_api_key(api_key) + .with_base_url(GROK_BASE_URL) + .with_model(model) .build() } @@ -395,9 +523,9 @@ pub fn zai() -> Result { let model = env_or_default("ZAI_MODEL", ZAI_DEFAULT_MODEL); AnthropicClient::builder() - .api_key(api_key) - .base_url(ZAI_BASE_URL) - .model(model) + .with_api_key(api_key) + .with_base_url(ZAI_BASE_URL) + .with_model(model) .build() } @@ -425,9 +553,9 @@ pub fn self_hosted(base_url: &str, model: &str) -> Result, @@ -376,53 +373,12 @@ pub struct AnthropicClientBuilder { /// Anthropic requires this field on every request. Defaults to 8192. max_tokens: u32, - /// The total HTTP request timeout (connect + response + body). + /// Shared HTTP client configuration (timeouts, pool, TCP). /// - /// Bounds the entire request lifecycle. Defaults to 120 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client); configure it on that client instead. - timeout: Duration, - - /// The TCP connection establishment timeout (including TLS handshake). - /// - /// Separate from the total timeout so a slow-connecting server can be - /// detected faster than a slow-responding one. Defaults to 10 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). - connect_timeout: Duration, - - /// A pre-built shared `reqwest::Client`, if injected via - /// [`http_client`](Self::http_client). When set, pool and TCP knobs are - /// ignored. - http: Option, - - /// Maximum idle connections kept alive per host. - /// - /// `None` defers to reqwest's default (unlimited). Set to a small value - /// (e.g. 1–4) for memory-constrained runners or workloads that make - /// mostly serial requests to a single host. - pool_max_idle_per_host: Option, - - /// How long an idle connection stays in the pool before being closed. - /// - /// `None` defers to reqwest's default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - pool_idle_timeout: Option, - - /// OS-level TCP keepalive interval. - /// - /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if - /// connections are silently dropped after idle periods (e.g. behind - /// aggressive NATs or load balancers). - tcp_keepalive: Option, - - /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). - /// - /// Defaults to `true` — SSE streaming emits many small packets, and - /// Nagle's algorithm coalesces them, adding latency per delta. No public - /// setter; always applied on internally-built clients. - tcp_nodelay: bool, + /// Holds the timeout, connection-pool, and TCP knobs that apply to the + /// internally-built `reqwest::Client`, or an externally-supplied client + /// injected via [`with_http_client`](Self::with_http_client). + http: super::HttpClientConfig, } impl Default for AnthropicClientBuilder { @@ -432,13 +388,7 @@ impl Default for AnthropicClientBuilder { base_url: DEFAULT_BASE_URL.into(), model: DEFAULT_MODEL.into(), max_tokens: DEFAULT_MAX_TOKENS, - timeout: DEFAULT_REQUEST_TIMEOUT, - connect_timeout: DEFAULT_CONNECT_TIMEOUT, - http: None, - pool_max_idle_per_host: None, - pool_idle_timeout: None, - tcp_keepalive: None, - tcp_nodelay: true, + http: super::HttpClientConfig::default(), } } } @@ -449,7 +399,7 @@ impl AnthropicClientBuilder { /// Required — [`build`](Self::build) returns an error if this is not set. /// The key is sent as the `x-api-key` header on every request. #[must_use] - pub fn api_key(mut self, key: impl Into) -> Self { + pub fn with_api_key(mut self, key: impl Into) -> Self { self.api_key = Some(key.into()); self } @@ -460,7 +410,7 @@ impl AnthropicClientBuilder { /// proxy, gateway, or Anthropic-compatible endpoint (e.g. Z.AI at /// `https://api.z.ai/api/anthropic`). #[must_use] - pub fn base_url(mut self, url: impl Into) -> Self { + pub fn with_base_url(mut self, url: impl Into) -> Self { self.base_url = url.into(); self } @@ -471,7 +421,7 @@ impl AnthropicClientBuilder { /// changed at runtime via [`AnthropicClient::set_model`] (e.g. when the /// [`FallbackManager`](crate::fallback::FallbackManager) trips). #[must_use] - pub fn model(mut self, model: impl Into) -> Self { + pub fn with_model(mut self, model: impl Into) -> Self { self.model = model.into(); self } @@ -483,31 +433,28 @@ impl AnthropicClientBuilder { /// response. Defaults to 8192. Increase for long-form generation; /// decrease to cap cost on simple queries. #[must_use] - pub fn max_tokens(mut self, tokens: u32) -> Self { + pub fn with_max_tokens(mut self, tokens: u32) -> Self { self.max_tokens = tokens; self } /// Set the total request timeout (connect + response + body). /// - /// Defaults to 120 seconds. This bounds the entire HTTP request lifecycle — - /// a hanging server will be aborted after this duration rather than - /// blocking the agent loop indefinitely. + /// Defaults to 120 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn timeout(mut self, timeout: Duration) -> Self { - self.timeout = timeout; + pub fn with_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_timeout(timeout); self } /// Set the TCP connection establishment timeout. /// - /// Defaults to 10 seconds. This is the maximum time to wait for the TCP - /// connection (including TLS handshake) to be established. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to 10 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn connect_timeout(mut self, timeout: Duration) -> Self { - self.connect_timeout = timeout; + pub fn with_connect_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_connect_timeout(timeout); self } @@ -517,50 +464,38 @@ impl AnthropicClientBuilder { /// provider built from the same handle, and the pool/TCP knobs are /// ignored. Configure timeouts on the injected client, not here. #[must_use] - pub fn http_client(mut self, client: reqwest::Client) -> Self { - self.http = Some(client); + pub fn with_http_client(mut self, client: reqwest::Client) -> Self { + self.http = self.http.with_http_client(client); self } /// Set the maximum idle connections kept alive per host. /// - /// Defaults to reqwest's built-in default (unlimited). Set to a small - /// value (e.g. 1–4) for memory-constrained runners or workloads that - /// make mostly serial requests to a single host. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (unlimited). Ignored when a + /// client was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { - self.pool_max_idle_per_host = Some(n); + pub fn with_pool_max_idle_per_host(mut self, n: usize) -> Self { + self.http = self.http.with_pool_max_idle_per_host(n); self } /// Set how long an idle connection stays in the pool before being closed. /// - /// Defaults to reqwest's built-in default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (90s). Ignored when a client + /// was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_idle_timeout(mut self, d: Duration) -> Self { - self.pool_idle_timeout = Some(d); + pub fn with_pool_idle_timeout(mut self, d: Duration) -> Self { + self.http = self.http.with_pool_idle_timeout(d); self } /// Set the OS-level TCP keepalive interval. /// - /// Defaults to disabled (reqwest default). Enable (~60s) if connections - /// are silently dropped after idle periods (e.g. behind aggressive NATs - /// or load balancers). - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to disabled (reqwest default). Ignored when a client was + /// supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn tcp_keepalive(mut self, d: Duration) -> Self { - self.tcp_keepalive = Some(d); + pub fn with_tcp_keepalive(mut self, d: Duration) -> Self { + self.http = self.http.with_tcp_keepalive(d); self } @@ -572,23 +507,12 @@ impl AnthropicClientBuilder { /// # Errors /// /// Returns [`ApiError`] if no API key was set via - /// [`api_key`](Self::api_key). + /// [`with_api_key`](Self::with_api_key). pub fn build(self) -> Result { let api_key = self .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - let http = match self.http { - Some(shared) => shared, - None => reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .tcp_nodelay(self.tcp_nodelay) - .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) - .maybe_pool_idle_timeout(self.pool_idle_timeout) - .maybe_tcp_keepalive(self.tcp_keepalive) - .build() - .map_err(|e| ApiError::http(e.to_string()))?, - }; + let http = self.http.build()?; Ok(AnthropicClient { http, @@ -1735,7 +1659,7 @@ mod tests { #[test] fn builder_succeeds_with_key() { let client = AnthropicClient::builder() - .api_key("sk-test") + .with_api_key("sk-test") .build() .unwrap(); assert_eq!(client.model(), DEFAULT_MODEL); @@ -1744,9 +1668,9 @@ mod tests { #[test] fn builder_custom_base_url_and_model() { let client = AnthropicClient::builder() - .api_key("sk-test") - .base_url("https://custom.example.com") - .model("claude-3-haiku") + .with_api_key("sk-test") + .with_base_url("https://custom.example.com") + .with_model("claude-3-haiku") .build() .unwrap(); assert_eq!(client.model(), "claude-3-haiku"); @@ -1940,111 +1864,16 @@ mod tests { assert_eq!(reader.take_line().unwrap(), "data: hi"); } - #[test] - fn builder_has_default_timeouts() { - // The builder should initialize with sensible non-zero defaults - // so that a hanging server cannot block the agent loop indefinitely. - let builder = AnthropicClientBuilder::default(); - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_timeout() { - let custom = Duration::from_mins(5); - let builder = AnthropicClientBuilder::default().timeout(custom); - assert_eq!(builder.timeout, custom); - // connect_timeout should be unchanged. - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_connect_timeout() { - let custom = Duration::from_secs(45); - let builder = AnthropicClientBuilder::default().connect_timeout(custom); - assert_eq!(builder.connect_timeout, custom); - // timeout should be unchanged. - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - } - - #[test] - fn builder_custom_both_timeouts() { - let req_timeout = Duration::from_mins(10); - let conn_timeout = Duration::from_secs(30); - let builder = AnthropicClientBuilder::default() - .timeout(req_timeout) - .connect_timeout(conn_timeout); - assert_eq!(builder.timeout, req_timeout); - assert_eq!(builder.connect_timeout, conn_timeout); - } - #[test] fn builder_timeouts_applied_on_build() { let client = AnthropicClient::builder() - .api_key("sk-test") - .timeout(Duration::from_mins(3)) - .connect_timeout(Duration::from_secs(15)) + .with_api_key("sk-test") + .with_timeout(Duration::from_mins(3)) + .with_connect_timeout(Duration::from_secs(15)) .build(); assert!(client.is_ok(), "build should succeed with valid timeouts"); } - #[test] - fn builder_accepts_injected_http_client() { - let shared = reqwest::Client::new(); - let client_a = AnthropicClient::builder() - .api_key("sk-a") - .http_client(shared.clone()) - .build(); - let client_b = AnthropicClient::builder() - .api_key("sk-b") - .http_client(shared) - .build(); - assert!(client_a.is_ok() && client_b.is_ok()); - } - - #[test] - fn injected_client_supersedes_builder_timeouts() { - let shared = reqwest::Client::builder() - .timeout(Duration::from_secs(1)) - .build() - .unwrap(); - let client = AnthropicClient::builder() - .api_key("sk-test") - .http_client(shared) - .timeout(Duration::from_secs(99)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn pool_knobs_build_clean() { - let client = AnthropicClient::builder() - .api_key("sk-test") - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .tcp_keepalive(Duration::from_secs(90)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn tcp_nodelay_default_is_true() { - let builder = AnthropicClientBuilder::default(); - assert!(builder.tcp_nodelay); - } - - #[test] - fn injected_client_ignores_pool_knobs() { - let shared = reqwest::Client::new(); - let client = AnthropicClient::builder() - .api_key("sk-test") - .http_client(shared) - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .build(); - assert!(client.is_ok()); - } - #[tokio::test] async fn sse_reader_take_line_splits_on_newline() { let mut reader = SseReader { @@ -2217,7 +2046,10 @@ mod tests { #[test] fn extract_structured_from_tool_use_input() { - let client = AnthropicClient::builder().api_key("test").build().unwrap(); + let client = AnthropicClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "content": [{ "type": "tool_use", @@ -2231,7 +2063,10 @@ mod tests { #[test] fn extract_structured_text_only_falls_back_to_raw() { - let client = AnthropicClient::builder().api_key("test").build().unwrap(); + let client = AnthropicClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "id": "msg_1", "model": "claude-3", diff --git a/src/provider/gemini.rs b/src/provider/gemini.rs index 22bbc62..27c4b98 100644 --- a/src/provider/gemini.rs +++ b/src/provider/gemini.rs @@ -14,8 +14,8 @@ //! //! // Explicit: //! let client = GeminiClient::builder() -//! .api_key("AIza...") -//! .model("gemini-2.0-flash") +//! .with_api_key("AIza...") +//! .with_model("gemini-2.0-flash") //! .build()?; //! ``` @@ -27,7 +27,6 @@ use reqwest::Response; use serde_json::Value; use std::time::Duration; -use super::ClientBuilderExt; use crate::api::ApiClient; use crate::api::error::ApiError; use crate::message::{Message, MessagePart, Role}; @@ -48,11 +47,9 @@ const DEFAULT_MODEL: &str = "gemini-2.0-flash"; const SSE_DATA_PREFIX: &str = "data: "; const TEXT_PART_INDEX: usize = 0; const THINKING_PART_INDEX: usize = 1; -const DEFAULT_REQUEST_TIMEOUT: Duration = Duration::from_mins(2); // connect + response + body const MAX_RESPONSE_BODY: usize = 10 * 1024 * 1024; // 10 Mb const SSE_MAX_BUFFER: usize = 1024 * 1024; // 1 Mb const MAX_ERROR_BODY: usize = 8 * 1024; // 8 Kb -const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); // ================================================== // Client @@ -107,7 +104,7 @@ impl GeminiClient { /// /// Returns a [`GeminiClientBuilder`] with sensible defaults. The only /// required field is `api_key`; everything else has a production-ready - /// default. Call `.api_key(...).build()` to finish, or chain additional + /// default. Call `.with_api_key(...).build()` to finish, or chain additional /// setters for custom configuration. /// /// # Example @@ -116,8 +113,8 @@ impl GeminiClient { /// use loopctl::provider::GeminiClient; /// /// let client = GeminiClient::builder() - /// .api_key("AI...") - /// .model("gemini-2.0-flash") + /// .with_api_key("AI...") + /// .with_model("gemini-2.0-flash") /// .build() /// .unwrap(); /// ``` @@ -150,9 +147,9 @@ impl GeminiClient { let model = std::env::var("GEMINI_MODEL").unwrap_or_else(|_| DEFAULT_MODEL.into()); Self::builder() - .api_key(api_key) - .base_url(base_url) - .model(model) + .with_api_key(api_key) + .with_base_url(base_url) + .with_model(model) .build() } @@ -424,21 +421,6 @@ pub struct GeminiClientBuilder { /// Can be changed at runtime via [`GeminiClient::set_model`]. model: String, - /// The total HTTP request timeout (connect + response + body). - /// - /// Bounds the entire request lifecycle. Defaults to 120 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client); configure it on that client instead. - timeout: Duration, - - /// The TCP connection establishment timeout (including TLS handshake). - /// - /// Separate from the total timeout so a slow-connecting server can be - /// detected faster. Defaults to 10 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). - connect_timeout: Duration, - /// Whether to opt into thought summaries from reasoning-capable models. /// /// When `true`, every request gets @@ -448,38 +430,12 @@ pub struct GeminiClientBuilder { /// must opt in once they know their model supports thinking. include_thoughts: bool, - /// A pre-built shared `reqwest::Client`, if injected via - /// [`http_client`](Self::http_client). When set, pool and TCP knobs are - /// ignored. - http: Option, - - /// Maximum idle connections kept alive per host. - /// - /// `None` defers to reqwest's default (unlimited). Set to a small value - /// (e.g. 1–4) for memory-constrained runners or workloads that make - /// mostly serial requests to a single host. - pool_max_idle_per_host: Option, - - /// How long an idle connection stays in the pool before being closed. - /// - /// `None` defers to reqwest's default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - pool_idle_timeout: Option, - - /// OS-level TCP keepalive interval. - /// - /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if - /// connections are silently dropped after idle periods (e.g. behind - /// aggressive NATs or load balancers). - tcp_keepalive: Option, - - /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). + /// Shared HTTP client configuration (timeouts, pool, TCP). /// - /// Defaults to `true` — SSE streaming emits many small packets, and - /// Nagle's algorithm coalesces them, adding latency per delta. No public - /// setter; always applied on internally-built clients. - tcp_nodelay: bool, + /// Holds the timeout, connection-pool, and TCP knobs that apply to the + /// internally-built `reqwest::Client`, or an externally-supplied client + /// injected via [`with_http_client`](Self::with_http_client). + http: super::HttpClientConfig, } impl Default for GeminiClientBuilder { @@ -488,14 +444,8 @@ impl Default for GeminiClientBuilder { api_key: None, base_url: DEFAULT_BASE_URL.into(), model: DEFAULT_MODEL.into(), - timeout: DEFAULT_REQUEST_TIMEOUT, - connect_timeout: DEFAULT_CONNECT_TIMEOUT, include_thoughts: false, - http: None, - pool_max_idle_per_host: None, - pool_idle_timeout: None, - tcp_keepalive: None, - tcp_nodelay: true, + http: super::HttpClientConfig::default(), } } } @@ -506,7 +456,7 @@ impl GeminiClientBuilder { /// Required — [`build`](Self::build) returns an error if this is not set. /// The key is sent as the `x-goog-api-key` header on every request. #[must_use] - pub fn api_key(mut self, key: impl Into) -> Self { + pub fn with_api_key(mut self, key: impl Into) -> Self { self.api_key = Some(key.into()); self } @@ -516,7 +466,7 @@ impl GeminiClientBuilder { /// Defaults to `https://generativelanguage.googleapis.com/v1beta`. /// Override when targeting a proxy or Google AI-compatible endpoint. #[must_use] - pub fn base_url(mut self, url: impl Into) -> Self { + pub fn with_base_url(mut self, url: impl Into) -> Self { self.base_url = url.into(); self } @@ -528,31 +478,28 @@ impl GeminiClientBuilder { /// [`GeminiClient::set_model`] (e.g. when the /// [`FallbackManager`](crate::fallback::FallbackManager) trips). #[must_use] - pub fn model(mut self, model: impl Into) -> Self { + pub fn with_model(mut self, model: impl Into) -> Self { self.model = model.into(); self } /// Set the total request timeout (connect + response + body). /// - /// Defaults to 120 seconds. This bounds the entire HTTP request lifecycle — - /// a hanging server will be aborted after this duration rather than - /// blocking the agent loop indefinitely. + /// Defaults to 120 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn timeout(mut self, timeout: Duration) -> Self { - self.timeout = timeout; + pub fn with_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_timeout(timeout); self } /// Set the TCP connection establishment timeout. /// - /// Defaults to 10 seconds. This is the maximum time to wait for the TCP - /// connection (including TLS handshake) to be established. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to 10 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn connect_timeout(mut self, timeout: Duration) -> Self { - self.connect_timeout = timeout; + pub fn with_connect_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_connect_timeout(timeout); self } @@ -569,7 +516,7 @@ impl GeminiClientBuilder { /// [`DeltaPart::Thinking`] regardless of this flag — it's purely the /// request-side opt-in. #[must_use] - pub fn include_thoughts(mut self, enabled: bool) -> Self { + pub fn with_include_thoughts(mut self, enabled: bool) -> Self { self.include_thoughts = enabled; self } @@ -580,50 +527,38 @@ impl GeminiClientBuilder { /// provider built from the same handle, and the pool/TCP knobs are /// ignored. Configure timeouts on the injected client, not here. #[must_use] - pub fn http_client(mut self, client: reqwest::Client) -> Self { - self.http = Some(client); + pub fn with_http_client(mut self, client: reqwest::Client) -> Self { + self.http = self.http.with_http_client(client); self } /// Set the maximum idle connections kept alive per host. /// - /// Defaults to reqwest's built-in default (unlimited). Set to a small - /// value (e.g. 1–4) for memory-constrained runners or workloads that - /// make mostly serial requests to a single host. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (unlimited). Ignored when a + /// client was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { - self.pool_max_idle_per_host = Some(n); + pub fn with_pool_max_idle_per_host(mut self, n: usize) -> Self { + self.http = self.http.with_pool_max_idle_per_host(n); self } /// Set how long an idle connection stays in the pool before being closed. /// - /// Defaults to reqwest's built-in default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (90s). Ignored when a client + /// was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_idle_timeout(mut self, d: Duration) -> Self { - self.pool_idle_timeout = Some(d); + pub fn with_pool_idle_timeout(mut self, d: Duration) -> Self { + self.http = self.http.with_pool_idle_timeout(d); self } /// Set the OS-level TCP keepalive interval. /// - /// Defaults to disabled (reqwest default). Enable (~60s) if connections - /// are silently dropped after idle periods (e.g. behind aggressive NATs - /// or load balancers). - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to disabled (reqwest default). Ignored when a client was + /// supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn tcp_keepalive(mut self, d: Duration) -> Self { - self.tcp_keepalive = Some(d); + pub fn with_tcp_keepalive(mut self, d: Duration) -> Self { + self.http = self.http.with_tcp_keepalive(d); self } @@ -636,18 +571,7 @@ impl GeminiClientBuilder { let api_key = self .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - let http = match self.http { - Some(shared) => shared, - None => reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .tcp_nodelay(self.tcp_nodelay) - .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) - .maybe_pool_idle_timeout(self.pool_idle_timeout) - .maybe_tcp_keepalive(self.tcp_keepalive) - .build() - .map_err(|e| ApiError::http(e.to_string()))?, - }; + let http = self.http.build()?; Ok(GeminiClient { http, @@ -1400,15 +1324,18 @@ mod tests { #[test] fn builder_succeeds_with_key() { - let client = GeminiClient::builder().api_key("test-key").build().unwrap(); + let client = GeminiClient::builder() + .with_api_key("test-key") + .build() + .unwrap(); assert_eq!(client.model(), DEFAULT_MODEL); } #[test] fn builder_custom_model() { let client = GeminiClient::builder() - .api_key("test-key") - .model("gemini-1.5-pro") + .with_api_key("test-key") + .with_model("gemini-1.5-pro") .build() .unwrap(); assert_eq!(client.model(), "gemini-1.5-pro"); @@ -1417,9 +1344,9 @@ mod tests { #[test] fn builder_custom_base_url() { let client = GeminiClient::builder() - .api_key("test-key") - .base_url("https://custom.example.com") - .model("gemini-pro") + .with_api_key("test-key") + .with_base_url("https://custom.example.com") + .with_model("gemini-pro") .build() .unwrap(); assert_eq!(client.model(), "gemini-pro"); @@ -1428,7 +1355,7 @@ mod tests { #[test] fn stream_url_does_not_expose_api_key() { let client = GeminiClient::builder() - .api_key("secret-key-123") + .with_api_key("secret-key-123") .build() .unwrap(); let url = client.stream_url(); @@ -1445,7 +1372,7 @@ mod tests { #[test] fn generate_url_does_not_expose_api_key() { let client = GeminiClient::builder() - .api_key("secret-key-456") + .with_api_key("secret-key-456") .build() .unwrap(); let url = client.generate_url(); @@ -2059,114 +1986,22 @@ mod tests { assert_eq!(reader.take_line().unwrap(), "data: hi"); } - #[test] - fn builder_has_default_timeouts() { - // The builder should initialize with sensible non-zero defaults - // so that a hanging server cannot block the agent loop indefinitely. - let builder = GeminiClientBuilder::default(); - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_timeout() { - let custom = Duration::from_mins(5); - let builder = GeminiClientBuilder::default().timeout(custom); - assert_eq!(builder.timeout, custom); - // connect_timeout should be unchanged. - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_connect_timeout() { - let custom = Duration::from_secs(45); - let builder = GeminiClientBuilder::default().connect_timeout(custom); - assert_eq!(builder.connect_timeout, custom); - // timeout should be unchanged. - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - } - - #[test] - fn builder_custom_both_timeouts() { - let req_timeout = Duration::from_mins(10); - let conn_timeout = Duration::from_secs(30); - let builder = GeminiClientBuilder::default() - .timeout(req_timeout) - .connect_timeout(conn_timeout); - assert_eq!(builder.timeout, req_timeout); - assert_eq!(builder.connect_timeout, conn_timeout); - } - #[test] fn builder_timeouts_applied_on_build() { let client = GeminiClient::builder() - .api_key("test-key") - .timeout(Duration::from_mins(3)) - .connect_timeout(Duration::from_secs(15)) + .with_api_key("test-key") + .with_timeout(Duration::from_mins(3)) + .with_connect_timeout(Duration::from_secs(15)) .build(); assert!(client.is_ok(), "build should succeed with valid timeouts"); } #[test] - fn builder_accepts_injected_http_client() { - let shared = reqwest::Client::new(); - let client_a = GeminiClient::builder() - .api_key("key-a") - .http_client(shared.clone()) - .build(); - let client_b = GeminiClient::builder() - .api_key("key-b") - .http_client(shared) - .build(); - assert!(client_a.is_ok() && client_b.is_ok()); - } - - #[test] - fn injected_client_supersedes_builder_timeouts() { - let shared = reqwest::Client::builder() - .timeout(Duration::from_secs(1)) + fn builder_include_thoughts_defaults_false() { + let client = GeminiClient::builder() + .with_api_key("test-key") .build() .unwrap(); - let client = GeminiClient::builder() - .api_key("test-key") - .http_client(shared) - .timeout(Duration::from_secs(99)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn pool_knobs_build_clean() { - let client = GeminiClient::builder() - .api_key("test-key") - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .tcp_keepalive(Duration::from_secs(90)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn tcp_nodelay_default_is_true() { - let builder = GeminiClientBuilder::default(); - assert!(builder.tcp_nodelay); - } - - #[test] - fn injected_client_ignores_pool_knobs() { - let shared = reqwest::Client::new(); - let client = GeminiClient::builder() - .api_key("test-key") - .http_client(shared) - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn builder_include_thoughts_defaults_false() { - let client = GeminiClient::builder().api_key("test-key").build().unwrap(); assert!( !client.include_thoughts, "include_thoughts must default to false (non-reasoning models reject thinkingConfig)" @@ -2176,8 +2011,8 @@ mod tests { #[test] fn builder_include_thoughts_true_propagates_to_client() { let client = GeminiClient::builder() - .api_key("test-key") - .include_thoughts(true) + .with_api_key("test-key") + .with_include_thoughts(true) .build() .unwrap(); assert!( @@ -2311,7 +2146,10 @@ mod tests { #[test] fn extract_structured_from_text_field() { - let client = GeminiClient::builder().api_key("test").build().unwrap(); + let client = GeminiClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "candidates": [{ "content": { @@ -2327,7 +2165,10 @@ mod tests { #[test] fn extract_structured_prose_falls_back_to_raw() { - let client = GeminiClient::builder().api_key("test").build().unwrap(); + let client = GeminiClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "candidates": [{ "content": { diff --git a/src/provider/openai.rs b/src/provider/openai.rs index 38e19a1..b8a66a7 100644 --- a/src/provider/openai.rs +++ b/src/provider/openai.rs @@ -14,9 +14,9 @@ //! //! // Explicit: //! let client = OpenAiClient::builder() -//! .api_key("sk-...") -//! .base_url("https://api.deepseek.com/v1") -//! .model("deepseek-chat") +//! .with_api_key("sk-...") +//! .with_base_url("https://api.deepseek.com/v1") +//! .with_model("deepseek-chat") //! .build()?; //! ``` @@ -29,7 +29,6 @@ use reqwest::Response; use serde::Deserialize; use serde_json::Value; -use super::ClientBuilderExt; use crate::api::ApiClient; use crate::api::error::ApiError; use crate::message::{Message, MessagePart, Role}; @@ -51,11 +50,9 @@ const SSE_DONE: &str = "[DONE]"; const SSE_DATA_PREFIX: &str = "data: "; const TEXT_PART_INDEX: usize = 0; const THINKING_PART_INDEX: usize = 1; -const DEFAULT_REQUEST_TIMEOUT: Duration = Duration::from_mins(2); // connect + response + body const MAX_RESPONSE_BODY: usize = 10 * 1024 * 1024; // 10 Mb const SSE_MAX_BUFFER: usize = 1024 * 1024; // 1 Mb const MAX_ERROR_BODY: usize = 8 * 1024; // 8 Kb -const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); // ================================================== // Client @@ -102,7 +99,7 @@ impl OpenAiClient { /// /// Returns an [`OpenAiClientBuilder`] with sensible defaults. The only /// required field is `api_key`; everything else has a production-ready - /// default. Call `.api_key(...).build()` to finish, or chain additional + /// default. Call `.with_api_key(...).build()` to finish, or chain additional /// setters for custom configuration. /// /// # Example @@ -111,8 +108,8 @@ impl OpenAiClient { /// use loopctl::provider::OpenAiClient; /// /// let client = OpenAiClient::builder() - /// .api_key("sk-...") - /// .model("gpt-4o") + /// .with_api_key("sk-...") + /// .with_model("gpt-4o") /// .build() /// .unwrap(); /// ``` @@ -151,9 +148,9 @@ impl OpenAiClient { .unwrap_or_else(|_| DEFAULT_MODEL.into()); Self::builder() - .api_key(api_key) - .base_url(base_url) - .model(model) + .with_api_key(api_key) + .with_base_url(base_url) + .with_model(model) .build() } @@ -434,53 +431,12 @@ pub struct OpenAiClientBuilder { /// Can be changed at runtime via [`OpenAiClient::set_model`]. model: String, - /// The total HTTP request timeout (connect + response + body). + /// Shared HTTP client configuration (timeouts, pool, TCP). /// - /// Bounds the entire request lifecycle. Defaults to 120 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client); configure it on that client instead. - timeout: Duration, - - /// The TCP connection establishment timeout (including TLS handshake). - /// - /// Separate from the total timeout so a slow-connecting server can be - /// detected faster. Defaults to 10 seconds. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). - connect_timeout: Duration, - - /// A pre-built shared `reqwest::Client`, if injected via - /// [`http_client`](Self::http_client). When set, pool and TCP knobs are - /// ignored. - http: Option, - - /// Maximum idle connections kept alive per host. - /// - /// `None` defers to reqwest's default (unlimited). Set to a small value - /// (e.g. 1–4) for memory-constrained runners or workloads that make - /// mostly serial requests to a single host. - pool_max_idle_per_host: Option, - - /// How long an idle connection stays in the pool before being closed. - /// - /// `None` defers to reqwest's default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - pool_idle_timeout: Option, - - /// OS-level TCP keepalive interval. - /// - /// `None` disables TCP keepalive (reqwest default). Enable (~60s) if - /// connections are silently dropped after idle periods (e.g. behind - /// aggressive NATs or load balancers). - tcp_keepalive: Option, - - /// Whether to disable Nagle's algorithm (`TCP_NODELAY`). - /// - /// Defaults to `true` — SSE streaming emits many small packets, and - /// Nagle's algorithm coalesces them, adding latency per delta. No public - /// setter; always applied on internally-built clients. - tcp_nodelay: bool, + /// Holds the timeout, connection-pool, and TCP knobs that apply to the + /// internally-built `reqwest::Client`, or an externally-supplied client + /// injected via [`with_http_client`](Self::with_http_client). + http: super::HttpClientConfig, } impl Default for OpenAiClientBuilder { @@ -489,13 +445,7 @@ impl Default for OpenAiClientBuilder { api_key: None, base_url: DEFAULT_BASE_URL.into(), model: DEFAULT_MODEL.into(), - timeout: DEFAULT_REQUEST_TIMEOUT, - connect_timeout: DEFAULT_CONNECT_TIMEOUT, - http: None, - pool_max_idle_per_host: None, - pool_idle_timeout: None, - tcp_keepalive: None, - tcp_nodelay: true, + http: super::HttpClientConfig::default(), } } } @@ -507,7 +457,7 @@ impl OpenAiClientBuilder { /// The key is sent as the `Authorization: Bearer ` header on every /// request. #[must_use] - pub fn api_key(mut self, key: impl Into) -> Self { + pub fn with_api_key(mut self, key: impl Into) -> Self { self.api_key = Some(key.into()); self } @@ -518,7 +468,7 @@ impl OpenAiClientBuilder { /// OpenAI-compatible endpoint (e.g. `https://api.deepseek.com/v1`, /// `http://localhost:11434/v1` for Ollama, or a vLLM server). #[must_use] - pub fn base_url(mut self, url: impl Into) -> Self { + pub fn with_base_url(mut self, url: impl Into) -> Self { self.base_url = url.into(); self } @@ -529,86 +479,69 @@ impl OpenAiClientBuilder { /// changed at runtime via [`OpenAiClient::set_model`] (e.g. when the /// [`FallbackManager`](crate::fallback::FallbackManager) trips). #[must_use] - pub fn model(mut self, model: impl Into) -> Self { + pub fn with_model(mut self, model: impl Into) -> Self { self.model = model.into(); self } /// Set the total request timeout (connect + response + body). /// - /// Defaults to 120 seconds. This bounds the entire HTTP request lifecycle — - /// a hanging server will be aborted after this duration rather than - /// blocking the agent loop indefinitely. + /// Defaults to 120 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn timeout(mut self, timeout: Duration) -> Self { - self.timeout = timeout; + pub fn with_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_timeout(timeout); self } /// Set the TCP connection establishment timeout. /// - /// Defaults to 10 seconds. This is the maximum time to wait for the TCP - /// connection (including TLS handshake) to be established. - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to 10 seconds. Ignored when a client was supplied via + /// [`with_http_client`](Self::with_http_client). #[must_use] - pub fn connect_timeout(mut self, timeout: Duration) -> Self { - self.connect_timeout = timeout; + pub fn with_connect_timeout(mut self, timeout: Duration) -> Self { + self.http = self.http.with_connect_timeout(timeout); self } /// Inject a pre-built, shared `reqwest::Client`. /// /// When set, the client's connection pool is shared with every other - /// provider built from the same handle, and the pool/TCP knobs below - /// (`pool_*`, `tcp_*`) are ignored — they are only applied when the - /// builder constructs its own client. Configure timeouts on the injected - /// client (`reqwest::Client::builder().timeout(...)`), not here. + /// provider built from the same handle, and the pool/TCP knobs are + /// ignored. Configure timeouts on the injected client, not here. #[must_use] - pub fn http_client(mut self, client: reqwest::Client) -> Self { - self.http = Some(client); + pub fn with_http_client(mut self, client: reqwest::Client) -> Self { + self.http = self.http.with_http_client(client); self } /// Set the maximum idle connections kept alive per host. /// - /// Defaults to reqwest's built-in default (unlimited). Set to a small - /// value (e.g. 1–4) for memory-constrained runners or workloads that - /// make mostly serial requests to a single host. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (unlimited). Ignored when a + /// client was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_max_idle_per_host(mut self, n: usize) -> Self { - self.pool_max_idle_per_host = Some(n); + pub fn with_pool_max_idle_per_host(mut self, n: usize) -> Self { + self.http = self.http.with_pool_max_idle_per_host(n); self } /// Set how long an idle connection stays in the pool before being closed. /// - /// Defaults to reqwest's built-in default (90s). Raise for long-idle - /// interactive workloads to keep TLS sessions warm; lower for tight - /// batch jobs to free file descriptors sooner. - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to reqwest's built-in default (90s). Ignored when a client + /// was supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn pool_idle_timeout(mut self, d: Duration) -> Self { - self.pool_idle_timeout = Some(d); + pub fn with_pool_idle_timeout(mut self, d: Duration) -> Self { + self.http = self.http.with_pool_idle_timeout(d); self } /// Set the OS-level TCP keepalive interval. /// - /// Defaults to disabled (reqwest default). Enable (~60s) if connections - /// are silently dropped after idle periods (e.g. behind aggressive NATs - /// or load balancers). - /// - /// Ignored when a client was supplied via - /// [`http_client`](Self::http_client). + /// Defaults to disabled (reqwest default). Ignored when a client was + /// supplied via [`with_http_client`](Self::with_http_client). #[must_use] - pub fn tcp_keepalive(mut self, d: Duration) -> Self { - self.tcp_keepalive = Some(d); + pub fn with_tcp_keepalive(mut self, d: Duration) -> Self { + self.http = self.http.with_tcp_keepalive(d); self } @@ -621,19 +554,7 @@ impl OpenAiClientBuilder { let api_key = self .api_key .ok_or_else(|| ApiError::auth_invalid_key("API key not provided"))?; - - let http = match self.http { - Some(shared) => shared, - None => reqwest::Client::builder() - .timeout(self.timeout) - .connect_timeout(self.connect_timeout) - .tcp_nodelay(self.tcp_nodelay) - .maybe_pool_max_idle_per_host(self.pool_max_idle_per_host) - .maybe_pool_idle_timeout(self.pool_idle_timeout) - .maybe_tcp_keepalive(self.tcp_keepalive) - .build() - .map_err(|e| ApiError::http(e.to_string()))?, - }; + let http = self.http.build()?; Ok(OpenAiClient { http, @@ -1862,111 +1783,16 @@ mod tests { assert_eq!(line, "data: hi"); } - #[test] - fn builder_has_default_timeouts() { - // The builder should initialize with sensible non-zero defaults - // so that a hanging server cannot block the agent loop indefinitely. - let builder = OpenAiClientBuilder::default(); - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_timeout() { - let custom = Duration::from_mins(5); - let builder = OpenAiClientBuilder::default().timeout(custom); - assert_eq!(builder.timeout, custom); - // connect_timeout should be unchanged. - assert_eq!(builder.connect_timeout, DEFAULT_CONNECT_TIMEOUT); - } - - #[test] - fn builder_custom_connect_timeout() { - let custom = Duration::from_secs(45); - let builder = OpenAiClientBuilder::default().connect_timeout(custom); - assert_eq!(builder.connect_timeout, custom); - // timeout should be unchanged. - assert_eq!(builder.timeout, DEFAULT_REQUEST_TIMEOUT); - } - - #[test] - fn builder_custom_both_timeouts() { - let req_timeout = Duration::from_mins(10); - let conn_timeout = Duration::from_secs(30); - let builder = OpenAiClientBuilder::default() - .timeout(req_timeout) - .connect_timeout(conn_timeout); - assert_eq!(builder.timeout, req_timeout); - assert_eq!(builder.connect_timeout, conn_timeout); - } - #[test] fn builder_timeouts_applied_on_build() { let client = OpenAiClient::builder() - .api_key("sk-test") - .timeout(Duration::from_mins(3)) - .connect_timeout(Duration::from_secs(15)) + .with_api_key("sk-test") + .with_timeout(Duration::from_mins(3)) + .with_connect_timeout(Duration::from_secs(15)) .build(); assert!(client.is_ok(), "build should succeed with valid timeouts"); } - #[test] - fn builder_accepts_injected_http_client() { - let shared = reqwest::Client::new(); - let client_a = OpenAiClient::builder() - .api_key("sk-a") - .http_client(shared.clone()) - .build(); - let client_b = OpenAiClient::builder() - .api_key("sk-b") - .http_client(shared) - .build(); - assert!(client_a.is_ok() && client_b.is_ok()); - } - - #[test] - fn injected_client_supersedes_builder_timeouts() { - let shared = reqwest::Client::builder() - .timeout(Duration::from_secs(1)) - .build() - .unwrap(); - let client = OpenAiClient::builder() - .api_key("sk-test") - .http_client(shared) - .timeout(Duration::from_secs(99)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn pool_knobs_build_clean() { - let client = OpenAiClient::builder() - .api_key("sk-test") - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .tcp_keepalive(Duration::from_secs(90)) - .build(); - assert!(client.is_ok()); - } - - #[test] - fn tcp_nodelay_default_is_true() { - let builder = OpenAiClientBuilder::default(); - assert!(builder.tcp_nodelay); - } - - #[test] - fn injected_client_ignores_pool_knobs() { - let shared = reqwest::Client::new(); - let client = OpenAiClient::builder() - .api_key("sk-test") - .http_client(shared) - .pool_max_idle_per_host(4) - .pool_idle_timeout(Duration::from_secs(30)) - .build(); - assert!(client.is_ok()); - } - #[tokio::test] async fn sse_reader_take_line_splits_on_newline() { let mut reader = SseReader { @@ -2104,7 +1930,10 @@ mod tests { #[test] fn extract_structured_from_string_content() { - let client = OpenAiClient::builder().api_key("test").build().unwrap(); + let client = OpenAiClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "choices": [{ "message": { @@ -2118,7 +1947,10 @@ mod tests { #[test] fn extract_structured_from_object_content() { - let client = OpenAiClient::builder().api_key("test").build().unwrap(); + let client = OpenAiClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "choices": [{ "message": { @@ -2132,7 +1964,10 @@ mod tests { #[test] fn extract_structured_prose_falls_back_to_raw() { - let client = OpenAiClient::builder().api_key("test").build().unwrap(); + let client = OpenAiClient::builder() + .with_api_key("test") + .build() + .unwrap(); let raw = serde_json::json!({ "choices": [{ "message": { From ebe4b95e785b7a40df27585d6191c45e7cfb1c02 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Thu, 23 Jul 2026 08:57:20 +1200 Subject: [PATCH 3/3] chore: with_http_client docs fix --- src/provider.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/provider.rs b/src/provider.rs index bf90923..01fce98 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -215,7 +215,7 @@ impl HttpClientConfig { /// Build a `reqwest::Client` from this configuration. /// /// If an external client was supplied via - /// [`http_client`](Self::http_client), it is returned verbatim. Otherwise + /// [`with_http_client`](Self::with_http_client), it is returned verbatim. Otherwise /// a new client is constructed with the configured timeouts, pool knobs, /// and `tcp_nodelay(true)`. ///