diff --git a/src/core/observability.rs b/src/core/observability.rs index c827635a1..0f5e3c79a 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -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 { @@ -49,9 +51,62 @@ pub fn expected_error_kind(message: &str) -> Option { 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: +/// `" API error (): "` — e.g. +/// `"OpenHuman API error (504 Gateway Timeout): error code: 504"`. Pin the +/// match to that exact `"api error ("` 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 " ". + 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"); diff --git a/src/openhuman/agent/harness/session/runtime.rs b/src/openhuman/agent/harness/session/runtime.rs index 14f24e25a..67c106ca2 100644 --- a/src/openhuman/agent/harness/session/runtime.rs +++ b/src/openhuman/agent/harness/session/runtime.rs @@ -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", diff --git a/src/openhuman/channels/providers/web.rs b/src/openhuman/channels/providers/web.rs index 50889b578..15edea267 100644 --- a/src/openhuman/channels/providers/web.rs +++ b/src/openhuman/channels/providers/web.rs @@ -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", diff --git a/src/openhuman/integrations/client.rs b/src/openhuman/integrations/client.rs index bedbf98e3..289146ceb 100644 --- a/src/openhuman/integrations/client.rs +++ b/src/openhuman/integrations/client.rs @@ -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",