Skip to content

Commit 9ecada2

Browse files
BunsDevclaude
BunsDev
and
claude
committed
feat(api): remove startup panics from provider constructors
AGENTS.md forbids `.expect()` on fallible production paths. Provider constructors previously inlined `reqwest::Client::builder().timeout(...).build() .expect("failed to build reqwest client")` which crashes the whole process on rare TLS init failures. New shared helper providers/http_util::build_default_http_client centralises the timeout-customised reqwest client build and returns a structured ProviderError on failure. Eight provider constructors adopt it and now return Result<Self, ProviderError>: * AnthropicProvider::from_config * OpenAiProvider::new * CodexProvider::new + from_stored * AzureProvider::new + from_env * CopilotProvider::new + from_env * CohereProvider::new + from_env * MinimaxProvider::new And BedrockProvider::from_env propagates via `?`. registry.rs adapts via a small `lift` helper that logs the error and returns None, plus per-site `.ok().map(Arc::new)` chains. The "provider unavailable" path was already supported by every downstream caller (Option<Arc<dyn LlmProvider>>), so the worst-case behaviour is the same provider degrading silently instead of crashing the process. OpenAiCompatProvider::new stays infallible (it has ~30 call sites in openai_compat_providers.rs that build per-provider helpers); it falls back to reqwest::Client::new() with a tracing warning when the timeout-customised build fails, which keeps long-running streams working with reqwest's defaults instead of panicking. cargo test --workspace: 1523 passing, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0aa05f1 commit 9ecada2

12 files changed

Lines changed: 211 additions & 100 deletions

File tree

‎src-rust/crates/api/src/providers/anthropic.rs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,20 @@ impl AnthropicProvider {
4747
}
4848

4949
/// Construct directly from a [`ClientConfig`], creating the inner client.
50-
pub fn from_config(config: ClientConfig) -> Self {
51-
let client = AnthropicClient::new(config)
52-
.expect("AnthropicProvider::from_config: failed to create AnthropicClient");
53-
Self {
50+
/// Returns an error when the underlying HTTP client cannot be built (e.g.
51+
/// TLS init failure); callers should treat that as "provider unavailable"
52+
/// rather than letting the process crash.
53+
pub fn from_config(config: ClientConfig) -> Result<Self, ProviderError> {
54+
let client = AnthropicClient::new(config).map_err(|e| ProviderError::Other {
55+
provider: ProviderId::new(ProviderId::ANTHROPIC),
56+
message: format!("failed to create AnthropicClient: {e}"),
57+
status: None,
58+
body: None,
59+
})?;
60+
Ok(Self {
5461
client: Arc::new(client),
5562
id: ProviderId::new(ProviderId::ANTHROPIC),
56-
}
63+
})
5764
}
5865

5966
/// Build a [`CreateMessageRequest`] from a [`ProviderRequest`].

‎src-rust/crates/api/src/providers/azure.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,16 @@ pub struct AzureProvider {
4141
}
4242

4343
impl AzureProvider {
44-
pub fn new(resource_name: String, api_key: String) -> Self {
45-
let http_client = reqwest::Client::builder()
46-
.timeout(std::time::Duration::from_secs(600))
47-
.build()
48-
.expect("failed to build reqwest client");
49-
50-
Self {
51-
id: ProviderId::new(ProviderId::AZURE),
44+
pub fn new(resource_name: String, api_key: String) -> Result<Self, ProviderError> {
45+
let id = ProviderId::new(ProviderId::AZURE);
46+
let http_client = crate::providers::http_util::build_default_http_client(&id)?;
47+
Ok(Self {
48+
id,
5249
resource_name,
5350
api_key,
5451
api_version: "2024-08-01-preview".to_string(),
5552
http_client,
56-
}
53+
})
5754
}
5855

5956
pub fn with_api_version(mut self, version: String) -> Self {
@@ -66,7 +63,9 @@ impl AzureProvider {
6663
let resource = std::env::var("AZURE_RESOURCE_NAME").ok()?;
6764
let version =
6865
std::env::var("AZURE_API_VERSION").unwrap_or_else(|_| "2024-08-01-preview".to_string());
69-
Some(Self::new(resource, key).with_api_version(version))
66+
Self::new(resource, key)
67+
.ok()
68+
.map(|p| p.with_api_version(version))
7069
}
7170

7271
fn endpoint_url(&self, deployment: &str) -> String {

‎src-rust/crates/api/src/providers/bedrock.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,8 @@ impl BedrockProvider {
5454
.or_else(|_| std::env::var("AWS_DEFAULT_REGION"))
5555
.unwrap_or_else(|_| "us-east-1".to_string());
5656

57-
let http_client = reqwest::Client::builder()
58-
.timeout(std::time::Duration::from_secs(600))
59-
.build()
60-
.expect("failed to build reqwest client");
57+
let id = ProviderId::new(ProviderId::AMAZON_BEDROCK);
58+
let http_client = crate::providers::http_util::build_default_http_client(&id).ok()?;
6159

6260
// Bearer token takes priority over SigV4 credentials.
6361
if let Ok(token) = std::env::var("AWS_BEARER_TOKEN_BEDROCK") {

‎src-rust/crates/api/src/providers/codex.rs‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,24 @@ pub struct CodexProvider {
5252
}
5353

5454
impl CodexProvider {
55-
pub fn new(tokens: CodexTokens) -> Self {
56-
let http_client = reqwest::Client::builder()
57-
.timeout(std::time::Duration::from_secs(600))
58-
.build()
59-
.expect("failed to build reqwest client");
60-
61-
Self {
62-
id: ProviderId::new(ProviderId::CODEX),
55+
pub fn new(tokens: CodexTokens) -> Result<Self, ProviderError> {
56+
let id = ProviderId::new(ProviderId::CODEX);
57+
let http_client = crate::providers::http_util::build_default_http_client(&id)?;
58+
Ok(Self {
59+
id,
6360
http_client,
6461
tokens: Arc::new(Mutex::new(tokens)),
65-
}
62+
})
6663
}
6764

68-
/// Construct from stored tokens; returns `None` if no tokens are saved.
65+
/// Construct from stored tokens; returns `None` if no tokens are saved or
66+
/// if the HTTP client could not be built.
6967
pub fn from_stored() -> Option<Self> {
7068
let tokens = get_codex_tokens()?;
7169
if tokens.access_token.is_empty() {
7270
return None;
7371
}
74-
Some(Self::new(tokens))
72+
Self::new(tokens).ok()
7573
}
7674

7775
// -----------------------------------------------------------------------

‎src-rust/crates/api/src/providers/cohere.rs‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,24 @@ pub struct CohereProvider {
4040

4141
impl CohereProvider {
4242
/// Create a new CohereProvider with the given API key.
43-
pub fn new(api_key: String) -> Self {
44-
let http_client = reqwest::Client::builder()
45-
.timeout(std::time::Duration::from_secs(600))
46-
.build()
47-
.expect("failed to build reqwest client");
48-
49-
Self {
50-
id: ProviderId::new(ProviderId::COHERE),
43+
pub fn new(api_key: String) -> Result<Self, ProviderError> {
44+
let id = ProviderId::new(ProviderId::COHERE);
45+
let http_client = crate::providers::http_util::build_default_http_client(&id)?;
46+
Ok(Self {
47+
id,
5148
api_key,
5249
http_client,
53-
}
50+
})
5451
}
5552

5653
/// Construct from the `COHERE_API_KEY` environment variable.
57-
/// Returns `None` if the variable is absent or empty.
54+
/// Returns `None` if the variable is absent, empty, or the HTTP client
55+
/// could not be built.
5856
pub fn from_env() -> Option<Self> {
5957
std::env::var("COHERE_API_KEY")
6058
.ok()
6159
.filter(|k| !k.is_empty())
62-
.map(Self::new)
60+
.and_then(|k| Self::new(k).ok())
6361
}
6462

6563
// -----------------------------------------------------------------------

‎src-rust/crates/api/src/providers/copilot.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,21 +50,20 @@ pub struct CopilotProvider {
5050
}
5151

5252
impl CopilotProvider {
53-
pub fn new(token: String) -> Self {
54-
let http_client = reqwest::Client::builder()
55-
.timeout(std::time::Duration::from_secs(600))
56-
.build()
57-
.expect("failed to build reqwest client");
58-
59-
Self {
60-
id: ProviderId::new(ProviderId::GITHUB_COPILOT),
53+
pub fn new(token: String) -> Result<Self, ProviderError> {
54+
let id = ProviderId::new(ProviderId::GITHUB_COPILOT);
55+
let http_client = crate::providers::http_util::build_default_http_client(&id)?;
56+
Ok(Self {
57+
id,
6158
token,
6259
http_client,
63-
}
60+
})
6461
}
6562

6663
pub fn from_env() -> Option<Self> {
67-
std::env::var("GITHUB_TOKEN").ok().map(Self::new)
64+
std::env::var("GITHUB_TOKEN")
65+
.ok()
66+
.and_then(|t| Self::new(t).ok())
6867
}
6968

7069
fn base_url() -> &'static str {

‎src-rust/crates/api/src/providers/http_util.rs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// providers/http_util.rs — shared HTTP-client construction helpers for
2+
// provider adapters.
3+
//
4+
// Every provider needs a `reqwest::Client` with a long timeout (LLM calls
5+
// routinely run >60s). Before this module each constructor inlined
6+
// `Client::builder().timeout(...).build().expect("...")` which panicked
7+
// at startup on TLS-init failures. AGENTS.md forbids `.expect()` on
8+
// fallible production paths, so we centralize the build here and return
9+
// a structured `ProviderError` instead.
10+
11+
use std::time::Duration;
12+
13+
use claurst_core::provider_id::ProviderId;
14+
15+
use crate::provider_error::ProviderError;
16+
17+
/// Default timeout applied to every provider HTTP client. LLM streaming
18+
/// responses can take well over a minute; ten minutes is the standard
19+
/// upper bound across the adapters in this crate.
20+
pub const DEFAULT_PROVIDER_TIMEOUT: Duration = Duration::from_secs(600);
21+
22+
/// Build a default `reqwest::Client` for a provider adapter, propagating
23+
/// any builder failure as a structured [`ProviderError::Other`] so the
24+
/// registry can skip the provider instead of crashing the process.
25+
pub fn build_default_http_client(provider: &ProviderId) -> Result<reqwest::Client, ProviderError> {
26+
reqwest::Client::builder()
27+
.timeout(DEFAULT_PROVIDER_TIMEOUT)
28+
.build()
29+
.map_err(|e| ProviderError::Other {
30+
provider: provider.clone(),
31+
message: format!("failed to build HTTP client: {e}"),
32+
status: None,
33+
body: None,
34+
})
35+
}
36+
37+
#[cfg(test)]
38+
mod tests {
39+
use super::*;
40+
41+
#[test]
42+
fn build_default_http_client_succeeds_for_well_known_provider() {
43+
// Under normal conditions the reqwest builder succeeds; the test
44+
// primarily guards against a regression in the timeout arg or the
45+
// ProviderError mapping.
46+
let pid = ProviderId::new("test");
47+
let client = build_default_http_client(&pid).expect("client must build");
48+
// Smoke-check: the client is usable.
49+
let _ = client.get("http://127.0.0.1:1").build();
50+
}
51+
}

‎src-rust/crates/api/src/providers/minimax.rs‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,35 @@ pub struct MinimaxProvider {
2929
}
3030

3131
impl MinimaxProvider {
32-
pub fn new(api_key: String) -> Self {
32+
pub fn new(api_key: String) -> Result<Self, ProviderError> {
33+
let id = ProviderId::new(ProviderId::MINIMAX);
3334
let api_base = std::env::var("MINIMAX_BASE_URL")
3435
.unwrap_or_else(|_| "https://api.minimax.io/anthropic".to_string());
3536
let mut headers = header::HeaderMap::new();
36-
headers.insert(
37-
"X-Api-Key",
38-
header::HeaderValue::from_str(&api_key)
39-
.expect("unable to parse api key for http header"),
40-
);
37+
let header_value =
38+
header::HeaderValue::from_str(&api_key).map_err(|e| ProviderError::AuthFailed {
39+
provider: id.clone(),
40+
message: format!(
41+
"API key contains characters that cannot be sent in an HTTP header: {e}"
42+
),
43+
})?;
44+
headers.insert("X-Api-Key", header_value);
4145
let http_client = Client::builder()
4246
.default_headers(headers)
4347
.timeout(std::time::Duration::from_secs(600))
4448
.build()
45-
.expect("MinimaxProvider: failed to build HTTP client");
46-
47-
Self {
49+
.map_err(|e| ProviderError::Other {
50+
provider: id.clone(),
51+
message: format!("failed to build HTTP client: {e}"),
52+
status: None,
53+
body: None,
54+
})?;
55+
Ok(Self {
4856
http_client,
4957
api_key,
5058
api_base,
51-
id: ProviderId::new(ProviderId::MINIMAX),
52-
}
59+
id,
60+
})
5361
}
5462

5563
fn build_request(request: &ProviderRequest) -> CreateMessageRequest {

‎src-rust/crates/api/src/providers/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
pub mod anthropic;
22
pub use anthropic::AnthropicProvider;
33

4+
pub(crate) mod http_util;
45
pub(crate) mod message_normalization;
56
pub(crate) mod request_options;
67

‎src-rust/crates/api/src/providers/openai.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,16 @@ pub struct OpenAiProvider {
4949
}
5050

5151
impl OpenAiProvider {
52-
pub fn new(api_key: String) -> Self {
53-
let http_client = reqwest::Client::builder()
54-
.timeout(std::time::Duration::from_secs(600))
55-
.build()
56-
.expect("failed to build reqwest client");
57-
58-
Self {
59-
id: ProviderId::new(ProviderId::OPENAI),
52+
pub fn new(api_key: String) -> Result<Self, ProviderError> {
53+
let id = ProviderId::new(ProviderId::OPENAI);
54+
let http_client = crate::providers::http_util::build_default_http_client(&id)?;
55+
Ok(Self {
56+
id,
6057
name: "OpenAI".to_string(),
6158
base_url: "https://api.openai.com".to_string(),
6259
api_key,
6360
http_client,
64-
}
61+
})
6562
}
6663

6764
/// Override the API base URL (e.g. for Azure, Ollama, or other compatible

0 commit comments

Comments
 (0)