mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(observability): skip Sentry for transport-level + transient-upstream errors (TAURI-32 / 5Z / 2G) (#1601)
This commit is contained in:
@@ -39,6 +39,8 @@ pub const TRANSIENT_PROVIDER_HTTP_STATUSES: &[u16] = &[408, 429, 502, 503, 504];
|
||||
pub enum ExpectedErrorKind {
|
||||
LocalAiDisabled,
|
||||
ApiKeyMissing,
|
||||
NetworkUnreachable,
|
||||
TransientUpstreamHttp,
|
||||
}
|
||||
|
||||
pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
|
||||
@@ -49,9 +51,62 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
|
||||
if lower.contains("api key not set") || lower.contains("missing api key") {
|
||||
return Some(ExpectedErrorKind::ApiKeyMissing);
|
||||
}
|
||||
if is_network_unreachable_message(&lower) {
|
||||
return Some(ExpectedErrorKind::NetworkUnreachable);
|
||||
}
|
||||
if is_transient_upstream_http_message(&lower) {
|
||||
return Some(ExpectedErrorKind::TransientUpstreamHttp);
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Detect transport-level connection failures that fire before any HTTP status
|
||||
/// is observed — DNS resolution failures, TCP connect refused/reset, TLS
|
||||
/// handshake failures, or ISP/firewall blocks. The canonical shape is
|
||||
/// reqwest's `"error sending request for url (…)"`, which surfaces from any
|
||||
/// HTTP call site (provider chat, embeddings, backend RPC) when the request
|
||||
/// can't reach the server at all.
|
||||
///
|
||||
/// These are user-environment problems — VPN drop, captive portal, ISP-level
|
||||
/// block (OPENHUMAN-TAURI-32: user in RU couldn't reach `api.tinyhumans.ai`),
|
||||
/// firewall — that no amount of retry / fallback on our side can resolve.
|
||||
/// Sentry has no signal to act on (no status, no trace, no payload), so each
|
||||
/// occurrence is pure noise. Classify them as expected so the report site
|
||||
/// logs a breadcrumb rather than spawning an error event.
|
||||
fn is_network_unreachable_message(lower: &str) -> bool {
|
||||
lower.contains("error sending request for url")
|
||||
|| lower.contains("dns error")
|
||||
|| lower.contains("connection refused")
|
||||
|| lower.contains("connection reset")
|
||||
|| lower.contains("network is unreachable")
|
||||
|| lower.contains("no route to host")
|
||||
|| lower.contains("tls handshake")
|
||||
|| lower.contains("certificate verify failed")
|
||||
}
|
||||
|
||||
/// Detect transient upstream HTTP failures that have bubbled up out of the
|
||||
/// provider layer and into higher-level domains (`agent`, `web_channel`, …).
|
||||
///
|
||||
/// The reliable-provider stack already retries / falls back on
|
||||
/// [`TRANSIENT_PROVIDER_HTTP_STATUSES`] (408/429/502/503/504), and the
|
||||
/// `before_send` filter drops the per-attempt provider events that carry
|
||||
/// `domain=llm_provider`. But the same error is *also* returned via
|
||||
/// `Result::Err` and re-reported by callers that wrap the provider — e.g.
|
||||
/// `agent.run_single` (OPENHUMAN-TAURI-5Z), `web_channel.run_chat_task`,
|
||||
/// scheduler tick handlers — under a different `domain` tag, escaping the
|
||||
/// provider-scoped filter and producing one Sentry event per failed turn.
|
||||
///
|
||||
/// The canonical wire format from `providers::ops::api_error` is:
|
||||
/// `"<provider> API error (<status>): <sanitized>"` — e.g.
|
||||
/// `"OpenHuman API error (504 Gateway Timeout): error code: 504"`. Pin the
|
||||
/// match to that exact `"api error (<status>"` prefix so an unrelated message
|
||||
/// that merely mentions "504" (a log line, a doc URL) is not silenced.
|
||||
fn is_transient_upstream_http_message(lower: &str) -> bool {
|
||||
TRANSIENT_PROVIDER_HTTP_STATUSES
|
||||
.iter()
|
||||
.any(|code| lower.contains(&format!("api error ({code}")))
|
||||
}
|
||||
|
||||
/// Capture an error to Sentry with structured tags.
|
||||
///
|
||||
/// `domain` and `operation` are required and become tags `domain:<…>` and
|
||||
@@ -116,6 +171,22 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str,
|
||||
"[observability] {domain}.{operation} skipped expected API-key configuration error: {message}"
|
||||
);
|
||||
}
|
||||
ExpectedErrorKind::NetworkUnreachable => {
|
||||
tracing::warn!(
|
||||
domain = domain,
|
||||
operation = operation,
|
||||
error = %message,
|
||||
"[observability] {domain}.{operation} skipped expected network-unreachable error: {message}"
|
||||
);
|
||||
}
|
||||
ExpectedErrorKind::TransientUpstreamHttp => {
|
||||
tracing::warn!(
|
||||
domain = domain,
|
||||
operation = operation,
|
||||
error = %message,
|
||||
"[observability] {domain}.{operation} skipped transient upstream HTTP error: {message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -249,6 +320,117 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_network_unreachable_errors() {
|
||||
// OPENHUMAN-TAURI-32: reqwest's transport-level error wrapped by the
|
||||
// web_channel error site. The classifier must catch it even when
|
||||
// embedded in caller context, since `report_error_or_expected` runs
|
||||
// `expected_error_kind` on the full anyhow chain.
|
||||
assert_eq!(
|
||||
expected_error_kind(
|
||||
"run_chat_task failed client_id=abc thread_id=t1 request_id=r1 \
|
||||
error=error sending request for url (https://api.tinyhumans.ai/openai/v1/chat/completions)"
|
||||
),
|
||||
Some(ExpectedErrorKind::NetworkUnreachable)
|
||||
);
|
||||
for raw in [
|
||||
"error sending request for url (https://api.example.com/x)",
|
||||
"provider failed: dns error: failed to lookup address information",
|
||||
"tcp connect: connection refused (os error 61)",
|
||||
"stream closed: connection reset by peer",
|
||||
"network is unreachable (os error 51)",
|
||||
"no route to host",
|
||||
"tls handshake eof",
|
||||
"certificate verify failed: unable to get local issuer certificate",
|
||||
] {
|
||||
assert_eq!(
|
||||
expected_error_kind(raw),
|
||||
Some(ExpectedErrorKind::NetworkUnreachable),
|
||||
"should classify as network-unreachable: {raw}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_classify_unrelated_provider_errors_as_network() {
|
||||
// Status-bearing provider failures (404, 500, …) are surfaced via
|
||||
// their HTTP status path and must NOT be silenced by the
|
||||
// network-unreachable classifier — the body text doesn't hit any of
|
||||
// the transport-level markers.
|
||||
assert_eq!(
|
||||
expected_error_kind("OpenAI API error (404): model gpt-x not found"),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
expected_error_kind("OpenAI API error (500): internal server error"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_transient_upstream_http_errors() {
|
||||
// OPENHUMAN-TAURI-5Z: the canonical shape emitted by
|
||||
// `providers::ops::api_error` and re-raised through `agent.run_single`.
|
||||
assert_eq!(
|
||||
expected_error_kind("OpenHuman API error (504 Gateway Timeout): error code: 504"),
|
||||
Some(ExpectedErrorKind::TransientUpstreamHttp)
|
||||
);
|
||||
|
||||
// Every transient code must classify, whether the status renders as
|
||||
// bare digits or "<digits> <reason>".
|
||||
for raw in [
|
||||
"OpenHuman API error (408): request timeout",
|
||||
"OpenAI API error (429 Too Many Requests): rate limit",
|
||||
"Anthropic API error (502 Bad Gateway): upstream unhealthy",
|
||||
"OpenHuman API error (503): service unavailable",
|
||||
"Provider API error (504): upstream timed out",
|
||||
] {
|
||||
assert_eq!(
|
||||
expected_error_kind(raw),
|
||||
Some(ExpectedErrorKind::TransientUpstreamHttp),
|
||||
"should classify as transient upstream HTTP: {raw}"
|
||||
);
|
||||
}
|
||||
|
||||
// Wrapped in an anyhow chain (as it reaches the agent layer) must
|
||||
// still classify — `expected_error_kind` is substring-based.
|
||||
assert_eq!(
|
||||
expected_error_kind(
|
||||
"agent turn failed: OpenHuman API error (504 Gateway Timeout): \
|
||||
error code: 504"
|
||||
),
|
||||
Some(ExpectedErrorKind::TransientUpstreamHttp)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_classify_actionable_provider_errors_as_transient_upstream() {
|
||||
// 4xx (other than 408/429) and non-transient 5xx must continue to
|
||||
// reach Sentry — those are real bugs (wrong model name, malformed
|
||||
// request, internal exception) that need to be triaged.
|
||||
for raw in [
|
||||
"OpenAI API error (400): bad request",
|
||||
"OpenAI API error (401): unauthorized",
|
||||
"OpenAI API error (403): forbidden",
|
||||
"OpenAI API error (404): model not found",
|
||||
"OpenAI API error (500): internal server error",
|
||||
] {
|
||||
assert_eq!(
|
||||
expected_error_kind(raw),
|
||||
None,
|
||||
"must NOT silence actionable provider error: {raw}"
|
||||
);
|
||||
}
|
||||
|
||||
// A free-form message that merely mentions "504" without the
|
||||
// `api error (` prefix must not be classified — pin the match to
|
||||
// the canonical shape from `ops::api_error`.
|
||||
assert_eq!(
|
||||
expected_error_kind("see runbook for 504 handling at https://example.com/504"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn report_error_does_not_panic_with_many_tags() {
|
||||
let err = anyhow::anyhow!("multi-tag");
|
||||
|
||||
@@ -502,7 +502,16 @@ impl Agent {
|
||||
}
|
||||
Err(err) => {
|
||||
let sanitized_message = Self::sanitize_event_error_message(&err);
|
||||
crate::core::observability::report_error(
|
||||
// OPENHUMAN-TAURI-5Z: upstream transient HTTP failures
|
||||
// (408/429/502/503/504) are already retried + filtered at the
|
||||
// provider layer. When retries exhaust they bubble up here via
|
||||
// `Result::Err`, and re-reporting under `domain=agent` escapes
|
||||
// the `domain=llm_provider` filter — one Sentry event per
|
||||
// failed turn for a transient infrastructure blip. Route
|
||||
// through `report_error_or_expected` so the classifier demotes
|
||||
// those (and other expected user-state errors) to a warn-level
|
||||
// breadcrumb while preserving the AgentError publish + return.
|
||||
crate::core::observability::report_error_or_expected(
|
||||
&err,
|
||||
"agent",
|
||||
"run_single",
|
||||
|
||||
@@ -337,7 +337,14 @@ pub async fn start_chat(
|
||||
client_id_task, thread_id_task, request_id_task, err
|
||||
);
|
||||
let (classified_type, classified_message) = classify_inference_error(&err);
|
||||
crate::core::observability::report_error(
|
||||
// Route through `report_error_or_expected` so transport-level
|
||||
// connection failures (DNS/TCP/TLS handshake, ISP blocks —
|
||||
// see OPENHUMAN-TAURI-32 for a RU user who couldn't reach
|
||||
// `api.tinyhumans.ai` at all) get logged as warn-level
|
||||
// breadcrumbs instead of error events. Sentry has no signal
|
||||
// to act on these — no status, no trace, no payload — and
|
||||
// every retry exhaustion produces another noisy event.
|
||||
crate::core::observability::report_error_or_expected(
|
||||
detailed.as_str(),
|
||||
"web_channel",
|
||||
"run_chat_task",
|
||||
|
||||
@@ -101,7 +101,13 @@ impl IntegrationClient {
|
||||
chain.push_str(&s.to_string());
|
||||
src = s.source();
|
||||
}
|
||||
crate::core::observability::report_error(
|
||||
// Use `report_error_or_expected` so transport-level shapes
|
||||
// ("error sending request for url", "tls handshake eof",
|
||||
// "connection refused/reset", …) are classified as
|
||||
// `NetworkUnreachable` and skip Sentry — user-environment
|
||||
// problems (VPN drop, captive portal, ISP block, TLS MITM)
|
||||
// that no retry on our side can resolve (OPENHUMAN-TAURI-2G).
|
||||
crate::core::observability::report_error_or_expected(
|
||||
chain.as_str(),
|
||||
"integrations",
|
||||
"post",
|
||||
@@ -165,7 +171,11 @@ impl IntegrationClient {
|
||||
chain.push_str(&s.to_string());
|
||||
src = s.source();
|
||||
}
|
||||
crate::core::observability::report_error(
|
||||
// Mirrors the post() transport site — classify reqwest
|
||||
// transport-level failures as NetworkUnreachable so they
|
||||
// skip Sentry. OPENHUMAN-TAURI-2G: TLS handshake EOF
|
||||
// against api.tinyhumans.ai from a SG user.
|
||||
crate::core::observability::report_error_or_expected(
|
||||
chain.as_str(),
|
||||
"integrations",
|
||||
"get",
|
||||
|
||||
Reference in New Issue
Block a user