diff --git a/src/core/observability.rs b/src/core/observability.rs index bb48a93f3..723571988 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -59,6 +59,7 @@ pub enum ExpectedErrorKind { NetworkUnreachable, TransientUpstreamHttp, LocalAiBinaryMissing, + BackendUserError, LocalAiCapabilityUnavailable, } @@ -79,6 +80,9 @@ pub fn expected_error_kind(message: &str) -> Option { if lower.contains("binary not found") { return Some(ExpectedErrorKind::LocalAiBinaryMissing); } + if is_backend_user_error_message(&lower) { + return Some(ExpectedErrorKind::BackendUserError); + } if is_local_ai_capability_unavailable_message(&lower) { return Some(ExpectedErrorKind::LocalAiCapabilityUnavailable); } @@ -132,6 +136,46 @@ fn is_transient_upstream_http_message(lower: &str) -> bool { .any(|code| lower.contains(&format!("api error ({code}"))) } +/// Detect non-2xx HTTP failures returned from the backend integrations / composio +/// clients that are by definition user-input or user-auth-state problems — not +/// bugs Sentry can act on. +/// +/// The canonical wire format from +/// [`crate::openhuman::integrations::client::IntegrationClient::post`] / `get` +/// and [`crate::openhuman::composio::client::ComposioClient`] is: +/// `"Backend returned for : "` — e.g. +/// `"Backend returned 400 Bad Request for POST https://api.tinyhumans.ai/agent-integrations/composio/authorize: Composio authorization failed: 400 …"` +/// (OPENHUMAN-TAURI-BC: user submitted SharePoint authorize without filling in +/// the required Tenant Name field). The backend correctly returned a 4xx; the +/// UI already surfaces the structured error to the user via toast — Sentry has +/// no remediation path because the request was malformed *by the user's +/// input*, not by our code. +/// +/// We pin the match to the `"backend returned "` prefix so an unrelated +/// message merely mentioning "400" (a log line, doc URL) is not silenced. +/// +/// We classify only 4xx codes, with **two exclusions**: +/// - `408 Request Timeout` and `429 Too Many Requests` are *transient* — they +/// are surfaced via [`is_transient_upstream_http_message`] for the provider +/// path and stay actionable for the backend path so a sustained 429 (rate +/// limit cliff) still pages. +/// +/// 5xx is intentionally **not** classified here — server-side failures from +/// our backend are real bugs that should reach Sentry. The transient +/// 502/503/504 deduplication is handled by the threshold logic in callers +/// (see e.g. `openhuman::socket::ws_loop::FAIL_ESCALATE_THRESHOLD`). +fn is_backend_user_error_message(lower: &str) -> bool { + let Some(rest) = lower.split_once("backend returned ").map(|(_, r)| r) else { + return false; + }; + let status_digits: String = rest.chars().take_while(|c| c.is_ascii_digit()).collect(); + let Ok(status) = status_digits.parse::() else { + return false; + }; + // 4xx (except transient 408 / 429 which are handled separately). + matches!(status, 400..=499) && status != 408 && status != 429 +} + /// Detect " is disabled / unavailable for this RAM tier" errors /// emitted by the local-AI service when the user's hardware tier doesn't /// support a capability (OPENHUMAN-TAURI-3B: vision asset download invoked @@ -248,6 +292,19 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, "[observability] {domain}.{operation} skipped expected local-ai binary-missing error: {message}" ); } + ExpectedErrorKind::BackendUserError => { + // 4xx from the integrations / composio backend client — + // user-input or auth-state failure that the backend already + // surfaced to the user via the structured error toast. + // OPENHUMAN-TAURI-BC: SharePoint authorize 400 because the + // user didn't fill in the required Tenant Name field. + tracing::warn!( + domain = domain, + operation = operation, + error = %message, + "[observability] {domain}.{operation} skipped expected backend user-error response: {message}" + ); + } ExpectedErrorKind::LocalAiCapabilityUnavailable => { // User-state condition: the local-AI service refused a // capability (vision summarization, vision asset download) @@ -671,6 +728,89 @@ mod tests { ); } + #[test] + fn classifies_backend_user_error_responses() { + // OPENHUMAN-TAURI-BC: SharePoint authorize 400 because the user + // didn't fill in the required Tenant Name field. The exact wire + // shape `IntegrationClient::post` builds — must classify as + // expected so the Sentry event is suppressed. + let bc = "Backend returned 400 Bad Request for POST \ + https://api.tinyhumans.ai/agent-integrations/composio/authorize: \ + Composio authorization failed: 400 \ + {\"error\":{\"message\":\"Missing required fields: Tenant Name\",\ + \"slug\":\"ConnectedAccount_MissingRequiredFields\",\"status\":400}}"; + assert_eq!( + expected_error_kind(bc), + Some(ExpectedErrorKind::BackendUserError), + "OPENHUMAN-TAURI-BC wire shape must classify" + ); + + // Cover the rest of the 4xx surface produced by integrations / + // composio clients — all user-input / auth-state failures that + // Sentry can't action. + for raw in [ + "Backend returned 400 Bad Request for POST https://api.example.com/x: bad input", + "Backend returned 401 Unauthorized for GET https://api.example.com/x: token expired", + "Backend returned 403 Forbidden for GET https://api.example.com/x: permission denied", + "Backend returned 404 Not Found for GET https://api.example.com/x: missing", + "Backend returned 422 Unprocessable Entity for POST https://api.example.com/x: validation failed", + "Backend returned 451 Unavailable for Legal Reasons for GET https://api.example.com/x: blocked", + // Lowercased context wrapping is irrelevant — substring match is case-insensitive. + "[observability] integrations.post failed: Backend returned 400 Bad Request for POST https://api.tinyhumans.ai/x: detail", + ] { + assert_eq!( + expected_error_kind(raw), + Some(ExpectedErrorKind::BackendUserError), + "must classify as backend user-error: {raw}" + ); + } + } + + #[test] + fn does_not_classify_transient_or_server_backend_errors_as_user_error() { + // 408 / 429 are transient — they belong to the + // upstream-transient bucket (or are retried at the caller), not + // the user-error bucket. A sustained 429 (rate limit cliff) MUST + // still surface so we can react. + for raw in [ + "Backend returned 408 Request Timeout for POST https://api.example.com/x: timeout", + "Backend returned 429 Too Many Requests for POST https://api.example.com/x: slow down", + ] { + assert_eq!( + expected_error_kind(raw), + None, + "transient 4xx must NOT be classified as user-error: {raw}" + ); + } + + // 5xx is always actionable — server bugs need to reach Sentry. + for raw in [ + "Backend returned 500 Internal Server Error for POST https://api.example.com/x: oops", + "Backend returned 502 Bad Gateway for POST https://api.example.com/x: upstream down", + "Backend returned 503 Service Unavailable for POST https://api.example.com/x: maintenance", + "Backend returned 504 Gateway Timeout for POST https://api.example.com/x: slow upstream", + ] { + assert_eq!( + expected_error_kind(raw), + None, + "5xx must NOT be classified as user-error: {raw}" + ); + } + + // A free-form message that mentions "400" but doesn't follow the + // `Backend returned ` prefix from the integrations / + // composio clients must not be silenced. + assert_eq!( + expected_error_kind("see HTTP 400 specification at https://example.com/400"), + None + ); + assert_eq!( + expected_error_kind("OpenAI API error (400): bad request"), + None, + "provider-formatted 4xx must keep going through the provider classifier path" + ); + } + #[test] fn classifies_local_ai_binary_missing_errors() { // OPENHUMAN-TAURI-9N: `local_ai_tts` returns this exact string diff --git a/src/openhuman/composio/client.rs b/src/openhuman/composio/client.rs index 6ebd45ca4..df08c4b4e 100644 --- a/src/openhuman/composio/client.rs +++ b/src/openhuman/composio/client.rs @@ -359,7 +359,11 @@ impl ComposioClient { logged_body ); let status_str = status.as_u16().to_string(); - crate::core::observability::report_error( + // Mirrors the integrations post()/get() sites — see + // OPENHUMAN-TAURI-BC. 4xx user-input / auth-state shapes + // demote via the observability classifier; 5xx and + // non-transient 4xx still surface as actionable events. + crate::core::observability::report_error_or_expected( format!("Backend returned {status} for DELETE {url}: {detail}").as_str(), "composio", "delete", diff --git a/src/openhuman/integrations/client.rs b/src/openhuman/integrations/client.rs index c97e0cd23..f41e3b8f9 100644 --- a/src/openhuman/integrations/client.rs +++ b/src/openhuman/integrations/client.rs @@ -121,27 +121,23 @@ impl IntegrationClient { let body_text = resp.text().await.unwrap_or_default(); let detail = extract_error_detail(&body_text, MAX_ERROR_BODY_LEN); let status_str = status.as_u16().to_string(); - if crate::core::observability::is_transient_http_status_code(status.as_u16()) { - tracing::warn!( - domain = "integrations", - operation = "post", - path = path, - status = status_str.as_str(), - failure = "non_2xx", - "[integrations] transient {status} for POST {url}: {detail}", - ); - } else { - crate::core::observability::report_error( - format!("Backend returned {status} for POST {url}: {detail}").as_str(), - "integrations", - "post", - &[ - ("path", path), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); - } + // Route through `report_error_or_expected` so 4xx user-input / + // auth-state failures (e.g. OPENHUMAN-TAURI-BC: SharePoint + // authorize 400 because the user didn't fill in the required + // Tenant Name field) demote to a warn breadcrumb instead of + // firing a Sentry event. 5xx and non-transient 4xx still + // surface — see `is_backend_user_error_message` for the exact + // status set classified as expected. + crate::core::observability::report_error_or_expected( + format!("Backend returned {status} for POST {url}: {detail}").as_str(), + "integrations", + "post", + &[ + ("path", path), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); anyhow::bail!("Backend returned {status} for POST {url}: {detail}"); } @@ -200,27 +196,20 @@ impl IntegrationClient { let body_text = resp.text().await.unwrap_or_default(); let detail = extract_error_detail(&body_text, MAX_ERROR_BODY_LEN); let status_str = status.as_u16().to_string(); - if crate::core::observability::is_transient_http_status_code(status.as_u16()) { - tracing::warn!( - domain = "integrations", - operation = "get", - path = path, - status = status_str.as_str(), - failure = "non_2xx", - "[integrations] transient {status} for GET {url}: {detail}", - ); - } else { - crate::core::observability::report_error( - format!("Backend returned {status} for GET {url}: {detail}").as_str(), - "integrations", - "get", - &[ - ("path", path), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); - } + // Mirrors the post() site — see OPENHUMAN-TAURI-BC. 4xx + // user-input / auth-state shapes demote to a warn breadcrumb + // via the observability classifier; 5xx and non-transient 4xx + // still surface. + crate::core::observability::report_error_or_expected( + format!("Backend returned {status} for GET {url}: {detail}").as_str(), + "integrations", + "get", + &[ + ("path", path), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); anyhow::bail!("Backend returned {status} for GET {url}: {detail}"); } diff --git a/src/openhuman/integrations/client_tests.rs b/src/openhuman/integrations/client_tests.rs index c077f0241..aa8fa3553 100644 --- a/src/openhuman/integrations/client_tests.rs +++ b/src/openhuman/integrations/client_tests.rs @@ -184,3 +184,83 @@ async fn get_403_propagates_backend_error_envelope_message() { ); assert!(msg.contains("403"), "expected status code, got: {msg}"); } + +// ── OPENHUMAN-TAURI-BC regression: wire format pins to classifier ─ + +/// Regression guard for OPENHUMAN-TAURI-BC: the exact bail message +/// `IntegrationClient::post` builds for a 4xx user-input failure must +/// classify as `BackendUserError` so the observability layer routes +/// the report through a warn breadcrumb instead of a Sentry event. +/// +/// If the format string in `client.rs` drifts away from the prefix +/// `is_backend_user_error_message` matches on, every Composio / +/// integrations 4xx will start spamming Sentry again — exactly the +/// regression this guards. +#[tokio::test] +async fn post_400_user_input_failure_classifies_as_backend_user_error() { + use crate::core::observability::{expected_error_kind, ExpectedErrorKind}; + + let app = Router::new().route( + "/agent-integrations/composio/authorize", + post(|| async { + ( + StatusCode::BAD_REQUEST, + Json(json!({ + "success": false, + "error": "Composio authorization failed: 400 {\"error\":{\"message\":\"Missing required fields: Tenant Name\",\"slug\":\"ConnectedAccount_MissingRequiredFields\",\"status\":400}}" + })), + ) + .into_response() + }), + ); + let base = start_mock_backend(app).await; + let client = client_for(base); + let err = client + .post::( + "/agent-integrations/composio/authorize", + &json!({ "toolkit": "sharepoint" }), + ) + .await + .expect_err("400 must surface as Err"); + let msg = format!("{err:#}"); + + // The propagated message must still match the classifier — both the + // `IntegrationClient::post` bail string and the + // `observability::report_error_or_expected` argument share the same + // shape, so this is a tight pin against drift on either side. + assert_eq!( + expected_error_kind(&msg), + Some(ExpectedErrorKind::BackendUserError), + "OPENHUMAN-TAURI-BC: propagated 400 must classify as BackendUserError; got: {msg}" + ); +} + +/// Counterpart: a 5xx must remain actionable. If the classifier ever +/// over-reaches and silences 5xx, this test catches it before users do. +#[tokio::test] +async fn post_500_remains_actionable() { + use crate::core::observability::expected_error_kind; + + let app = Router::new().route( + "/foo", + post(|| async { + ( + StatusCode::INTERNAL_SERVER_ERROR, + "upstream blew up", + ) + .into_response() + }), + ); + let base = start_mock_backend(app).await; + let client = client_for(base); + let err = client + .post::("/foo", &json!({})) + .await + .expect_err("500 must surface as Err"); + let msg = format!("{err:#}"); + assert_eq!( + expected_error_kind(&msg), + None, + "5xx must remain actionable, not classified as expected; got: {msg}" + ); +}