fix(observability): skip Sentry report for 429 from upstream providers (#1512)

This commit is contained in:
Steven Enamakel
2026-05-11 19:25:28 -07:00
committed by GitHub
parent 5bef91ea4d
commit c1200b8a2e
2 changed files with 115 additions and 66 deletions
+65 -55
View File
@@ -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;
}
+50 -11
View File
@@ -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(|_| "<failed to read provider error body>".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
));
}
}