diff --git a/src/openhuman/providers/compatible.rs b/src/openhuman/providers/compatible.rs index 1e561a1e4..e0feb2018 100644 --- a/src/openhuman/providers/compatible.rs +++ b/src/openhuman/providers/compatible.rs @@ -366,17 +366,19 @@ impl OpenAiCompatibleProvider { let error = response.text().await?; let sanitized = super::sanitize_api_error(&error); let message = format!("{} Responses API error: {sanitized}", self.name); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "responses_api", - &[ - ("provider", self.name.as_str()), - ("model", model), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "responses_api", + &[ + ("provider", self.name.as_str()), + ("model", model), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::bail!(message); } @@ -700,17 +702,19 @@ impl OpenAiCompatibleProvider { "{} streaming API error ({}): {}", self.name, status, sanitized ); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "streaming_chat", - &[ - ("provider", self.name.as_str()), - ("model", native_request.model.as_str()), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "streaming_chat", + &[ + ("provider", self.name.as_str()), + ("model", native_request.model.as_str()), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::bail!(message); } @@ -1155,17 +1159,19 @@ impl Provider for OpenAiCompatibleProvider { let status_str = status.as_u16().to_string(); let message = format!("{} API error ({status}): {sanitized}", self.name); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "chat_completions", - &[ - ("provider", self.name.as_str()), - ("model", model), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "chat_completions", + &[ + ("provider", self.name.as_str()), + ("model", model), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::bail!(message); } @@ -1552,17 +1558,19 @@ impl Provider for OpenAiCompatibleProvider { let status_str = status.as_u16().to_string(); let message = format!("{} API error ({status}): {sanitized}", self.name); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "native_chat", - &[ - ("provider", self.name.as_str()), - ("model", model), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "native_chat", + &[ + ("provider", self.name.as_str()), + ("model", model), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::bail!(message); } @@ -1678,17 +1686,19 @@ impl Provider for OpenAiCompatibleProvider { }; let sanitized_error = super::sanitize_api_error(&raw_error); let message = format!("{}: {}", status, sanitized_error); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "stream_chat", - &[ - ("provider", provider_name.as_str()), - ("model", model_owned.as_str()), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "stream_chat", + &[ + ("provider", provider_name.as_str()), + ("model", model_owned.as_str()), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } let _ = tx.send(Err(StreamError::Provider(message))).await; return; } diff --git a/src/openhuman/providers/ops.rs b/src/openhuman/providers/ops.rs index 311f908bf..be43b346d 100644 --- a/src/openhuman/providers/ops.rs +++ b/src/openhuman/providers/ops.rs @@ -136,11 +136,25 @@ pub fn format_anyhow_chain(err: &anyhow::Error) -> String { format!("{}…", &scrubbed[..end]) } +/// Whether a non-2xx provider response is worth reporting to Sentry. +/// +/// 429 Too Many Requests is a transient, caller-side throttling signal — the +/// reliable-provider layer already retries with backoff and falls back across +/// providers/models, and the aggregate "all providers exhausted" event still +/// fires if every attempt fails. Reporting each individual 429 floods Sentry +/// (see OPENHUMAN-TAURI-6Y: ~8K events/day from one user being rate-limited +/// by an upstream model). Callers should still propagate the error so retry +/// and fallback logic runs unchanged; this only gates the Sentry report. +pub fn should_report_provider_http_failure(status: reqwest::StatusCode) -> bool { + status != reqwest::StatusCode::TOO_MANY_REQUESTS +} + /// Build a sanitized provider error from a failed HTTP response. /// /// Also reports the failure to Sentry with `provider` and `status` tags so /// upstream LLM errors are visible in observability without every call-site -/// having to remember to log. +/// having to remember to log — except for transient statuses (see +/// [`should_report_provider_http_failure`]). pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::Error { let status = response.status(); let status_str = status.as_u16().to_string(); @@ -150,16 +164,18 @@ pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::E .unwrap_or_else(|_| "".to_string()); let sanitized = sanitize_api_error(&body); let message = format!("{provider} API error ({status}): {sanitized}"); - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "api_error", - &[ - ("provider", provider), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + if should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "api_error", + &[ + ("provider", provider), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::anyhow!(message) } @@ -367,4 +383,27 @@ mod tests { .is_ok() ); } + + #[test] + fn skips_sentry_report_for_429_only() { + // 429 is transient rate-limit — reliable.rs retries + falls back, and + // the aggregate "all providers exhausted" event still fires for genuine + // outages. Reporting each 429 individually floods Sentry. + assert!(!should_report_provider_http_failure( + reqwest::StatusCode::TOO_MANY_REQUESTS + )); + // Everything else (auth, server, gateway, etc.) is still worth a report. + assert!(should_report_provider_http_failure( + reqwest::StatusCode::UNAUTHORIZED + )); + assert!(should_report_provider_http_failure( + reqwest::StatusCode::INTERNAL_SERVER_ERROR + )); + assert!(should_report_provider_http_failure( + reqwest::StatusCode::BAD_GATEWAY + )); + assert!(should_report_provider_http_failure( + reqwest::StatusCode::SERVICE_UNAVAILABLE + )); + } }