From f7dfa94e5d4d0cec1ad462e05ea5253d165ebf1f Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Thu, 21 May 2026 05:24:07 +0530 Subject: [PATCH] =?UTF-8?q?fix(observability):=20Wave=204=20classifier=20?= =?UTF-8?q?=E2=80=94=20socket=20transport=20+=20custom-provider=20config-r?= =?UTF-8?q?ejection=20(~366=20events,=2013=20IDs)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Extends `expected_error_kind` matcher ladder with 11 new substring arms across two existing buckets — no new variants, no new emit sites, no behavior change beyond reclassifying known wire shapes that were leaking past Wave 1-3 anchors. - Closes 13 Sentry IDs (~366 events) — **all deterministic user-environment / user-config errors with no remediation path** (offline, DNS fail, captive portal, bad API key, out of credits, wrong model id, missing OAuth scope, region block). - Pure classifier — no UI, retry, or fallback logic touched. The reliable-provider stack still falls back to OpenHuman's hosted tier for the Lane O bodies; users keep their chat. Lane N socket failures already gated through `report_error_or_expected` at `ws_loop.rs:191`, so the threshold-escalation event simply demotes to a `warn` breadcrumb instead of paging. - Two micro-commits — Lane O (`is_provider_config_rejection_message` PHRASES const) and Lane N (`is_network_unreachable_message` substring arms) — each independently revertible. ## Problem After the Wave 1-3 sweep landed, fresh Sentry triage surfaced **13 unresolved IDs** whose bodies match the *spirit* of an existing classifier bucket but use wire-shape variants the current substring anchors miss: ### Lane O — custom-provider config rejection (~250 events) The `ProviderConfigRejection` variant + `is_provider_config_rejection_message` exist precisely for "user pointed OpenHuman at a custom_openai endpoint with a model / temperature / region / credential that provider doesn't accept." But the 7 phrases shipped in Wave 1-3 (#2079 / #2076 / #2202) only cover the DeepSeek / OpenRouter abstract-tier-leak and Moonshot temperature shapes. New surfaces: | Sentry ID | Events | Real wire body | |---|---|---| | `R1` | 44 | `{"error":{"message":"This model is not available in your region.","code":403}}` | | `R4` | 14 | `{"code":403,"reason":"ModelNotAllowed","message":"模型不允许访问"…}` (Doubao/ChatGLM) | | `YC` | 16 | `{"error":{"type":"invalid_authentication_error"…}}` | | `S5` | 14 | `{"error":{"message":"This request requires more credits, or fewer max_tokens…"}}` (OpenRouter 402) | | `Y0` | 13 | `{'error': '/chat/completions: Invalid model name passed in model=reasoning-v1…'}` | | `JN` | 14 | `{"error":{"message":"No active credentials for provider: openai"…}}` | | `KB` | 16 | Same `No active credentials for provider` shape from OpenHuman backend re-emit | | `JK` | 17 | `litellm.BadRequestError: Github_copilotException - Bad Request…` | | `J2` / `J5` / `J4` | 62 | `{"error":{"message":"model 'llama3.3' not found","type":"not_found_error"…}}` | ### Lane N — socket WebSocket-connect transport (~116 events) `is_network_unreachable_message` already catches `connection refused` / `dns error` / `network is unreachable` but two real shapes escape: | Sentry ID | Events | Real wire body | |---|---|---| | `44` | 50 | `[socket] Connection failed: WebSocket connect: IO error: failed to lookup address information: nodename nor servname provided, or not known` | | `4P` | 66 | `[socket] Connection failed: WebSocket connect: HTTP error: 200 OK` (captive portal / corporate proxy intercepting the WS upgrade handshake) | Every one of these is deterministic **user-environment** / **user-configuration** state — the maintainers have no remediation. Sentry has no signal to act on. Every event was pure noise. ## Solution ### Lane O — `src/openhuman/inference/provider/config_rejection.rs:55-79` Append 8 new phrases to the `PHRASES` const (case-insensitive substring match, same precedence as existing 7): ```rust "not available in your region", // R1 "modelnotallowed", // R4 "invalid_authentication_error", // YC "requires more credits", // S5 "invalid model name passed in model=", // Y0 "no active credentials for provider", // JN + KB "litellm.badrequesterror", // JK "not_found_error", // J2 + J5 + J4 ``` Each anchor is intentionally narrow (e.g. `passed in model=` not bare `invalid model name`; `litellm.badrequesterror` not bare `litellm`) so a stray log line elsewhere can't accidentally demote a real provider/backend bug. The HTTP-layer wrapper (`is_provider_config_rejection_http`) still guards on `provider != openhuman_backend::PROVIDER_LABEL`, so a model rejection from **our own** backend (which would be a real regression) still reaches Sentry. ### Lane N — `src/core/observability.rs:299-319` Three new substring arms appended to `is_network_unreachable_message`: - `failed to lookup address` + `nodename nor servname` — libc `getaddrinfo()` failure renderings on macOS / BSD / POSIX resolvers when tungstenite wraps as `IO error` without the reqwest `dns error` prefix. - `http error: 200 ok` — tungstenite-only render. Reqwest renders HTTP 200 as `"HTTP status server error (200)"`, so no collision with the regular HTTP call path. A negative precedence test (`http_200_classifier_does_not_silence_unrelated_log_lines`) pins this against benign `HTTP/1.1 200 OK` / `status: 200 OK` prose so a future broadening cannot silence success traces. ### Design trade-off — classifier vs. root-fix validation layer The Lane O bugs *do* have a more durable root fix: a pre-save provider validation layer (test the API key, validate the model id against `/v1/models`, surface region-blocks at config-save time). That's a real product initiative requiring UX design, per-provider model-list infrastructure, and meaningful spec work — out of scope for a triage sweep. This PR follows the Wave 1-3 maintainer precedent (classifier-first noise suppression) so the Sentry signal/noise ratio improves immediately; the validation layer remains tracked as a separate follow-up. If/when it lands, these classifier arms become redundant belt-and-suspenders and can be deleted without conflict. Lane N has no root-fix alternative — offline / firewall / captive-portal / DNS failures are pure user-environment state. Classifier-demote is the correct disposition. ## Submission Checklist > If a section does not apply to this change, mark the item as `N/A` with a one-line reason. Do not delete items. - [x] Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement) - [x] **Diff coverage ≥ 80%** — changed lines (Vitest + cargo-llvm-cov merged via `diff-cover`) meet the gate enforced by [`.github/workflows/coverage.yml`](../.github/workflows/coverage.yml). 11 positive tests pinned to real Sentry bodies + 1 negative precedence test cover every new substring arm; only the rustdoc comments and the negative `unrelated_*` test body are non-executable lines. - [x] N/A: behaviour-only change — Coverage matrix updated — added/removed/renamed feature rows in [`docs/TEST-COVERAGE-MATRIX.md`](../docs/TEST-COVERAGE-MATRIX.md) reflect this change - [x] All affected feature IDs from the matrix are listed in the PR description under `## Related` - [x] No new external network dependencies introduced (mock backend used per [Testing Strategy](../gitbooks/developing/testing-strategy.md#mock-policy)) - [x] N/A: classifier-only change with no UI / release-cut surface touched — Manual smoke checklist updated if this touches release-cut surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md)) - [x] Linked issue closed via `Closes #NNN` in the `## Related` section ## Impact - **Runtime**: no behavior change for users — chat / agent / web-channel / socket paths continue to fall back / retry exactly as before. The only delta is that `report_error_or_expected` now classifies these 13 wire shapes as `ExpectedErrorKind::*` instead of escalating as `tracing::error`, so Sentry stops receiving the events. - **Performance**: negligible — `is_provider_config_rejection_message` lowercases once and runs 15 substring scans (was 7); `is_network_unreachable_message` runs 11 substring scans (was 8). Both already on the error-path, never on the hot path. - **Security**: no new attack surface. Substring matchers run on already-rendered error messages — no parsing, no allocation beyond the existing `to_ascii_lowercase()`. - **Migration**: none. - **Compatibility**: forward-compatible. New phrases are append-only — no existing matcher behavior changes. If a future maintainer ships the root-fix validation layer, deleting these arms is a clean local edit. ## Related - Closes OPENHUMAN-TAURI-R1 - Closes OPENHUMAN-TAURI-R4 - Closes OPENHUMAN-TAURI-YC - Closes OPENHUMAN-TAURI-S5 - Closes OPENHUMAN-TAURI-Y0 - Closes OPENHUMAN-TAURI-JN - Closes OPENHUMAN-TAURI-KB - Closes OPENHUMAN-TAURI-JK - Closes OPENHUMAN-TAURI-J2 - Closes OPENHUMAN-TAURI-J5 - Closes OPENHUMAN-TAURI-J4 - Closes OPENHUMAN-TAURI-44 - Closes OPENHUMAN-TAURI-4P - Follow-up PR(s)/TODOs: pre-save provider-validation layer (validate API key + model id + region at config-save time) tracked as separate product initiative; would supersede the Lane O arms. --- ## AI Authored PR Metadata (required for Codex/Linear PRs) > Keep this section for AI-authored PRs. For human-only PRs, mark each field `N/A`. ### Linear Issue - Key: N/A (Sentry-driven triage; no Linear ticket) - URL: N/A ### Commit & Branch - Branch: fix/sentry-wave4-classifier-no - Commit SHA: e16414ac (tip) ### Agent - Claude Code (Sonnet 4.6) running locally ## Summary by CodeRabbit * **Bug Fixes** * Enhanced detection of network connectivity failures and provider configuration rejection scenarios for more accurate error classification. * **Tests** * Added comprehensive test coverage for network-related transport failures and provider configuration rejection cases. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2309?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) Co-authored-by: oxoxDev Co-authored-by: Steven Enamakel --- src/core/observability.rs | 64 +++++++++ .../inference/provider/config_rejection.rs | 133 +++++++++++++++++- 2 files changed, 191 insertions(+), 6 deletions(-) diff --git a/src/core/observability.rs b/src/core/observability.rs index ee6fd8727..5d7033041 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -289,15 +289,34 @@ fn is_loopback_unavailable(lower: &str) -> bool { /// through [`is_loopback_unavailable`] *before* this matcher so the /// boot-window race against the embedded core keeps its own bucket — see /// the precedence comment in [`expected_error_kind`]. +/// +/// Three additional substrings cover wire-shape variants observed in +/// Wave 4 that the original `"dns error"` / status-code matchers miss: +/// +/// - `"failed to lookup address"` / `"nodename nor servname"` — +/// `getaddrinfo()` failure renderings on macOS / BSD libc and POSIX +/// resolvers (`OPENHUMAN-TAURI-44` ~50 events, +/// `[socket] Connection failed: WebSocket connect: IO error: failed to +/// lookup address information: nodename nor servname provided, or not +/// known`). +/// - `"http error: 200 ok"` — tungstenite's `WsError::Http(200)` render +/// when a corporate proxy / captive portal intercepts the WebSocket +/// handshake and returns a plain HTML 200 page (`OPENHUMAN-TAURI-4P` +/// ~66 events). Tungstenite-only — reqwest renders HTTP 200 as +/// `"HTTP status server error (200)"`, so this can't collide with the +/// regular HTTP call path. fn is_network_unreachable_message(lower: &str) -> bool { lower.contains("error sending request for url") || lower.contains("dns error") + || lower.contains("failed to lookup address") + || lower.contains("nodename nor servname") || 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") + || lower.contains("http error: 200 ok") } /// Detect transient upstream HTTP failures that have bubbled up out of the @@ -1278,6 +1297,51 @@ mod tests { ); } + #[test] + fn classifies_wave4_socket_transport_wire_shapes() { + // OPENHUMAN-TAURI-44 (~50 events): libc `getaddrinfo()` rendering + // without the `dns error` token, wrapped by the socket emit site. + // The Wave 4 matcher arms catch the literal resolver phrases that + // the original `dns error` substring would miss when reqwest's + // wrapper isn't in the chain (e.g. tungstenite IO errors). + assert_eq!( + expected_error_kind( + "[socket] Connection failed (sustained outage after 5 attempts): \ + WebSocket connect: IO error: failed to lookup address information: \ + nodename nor servname provided, or not known" + ), + Some(ExpectedErrorKind::NetworkUnreachable) + ); + + // OPENHUMAN-TAURI-4P (~66 events): tungstenite renders a captive + // portal / corporate proxy that intercepts the WS handshake as + // `WsError::Http(200)` → `"HTTP error: 200 OK"`. Classify as + // network-unreachable since no amount of app-side retry can pierce + // an intercepting proxy. + assert_eq!( + expected_error_kind( + "[socket] Connection failed (sustained outage after 5 attempts): \ + WebSocket connect: HTTP error: 200 OK" + ), + Some(ExpectedErrorKind::NetworkUnreachable) + ); + } + + #[test] + fn http_200_classifier_does_not_silence_unrelated_log_lines() { + // The captive-portal arm anchors on `"http error: 200 ok"` (the + // exact tungstenite `WsError::Http(200)` Display rendering). + // Adjacent non-WebSocket log lines that mention `"HTTP/1.1 200 OK"` + // or `"status: 200 OK"` MUST NOT classify — those are normal-flow + // success traces, not failure events. Pin this precedence so a + // future refactor doesn't broaden the substring. + assert_eq!(expected_error_kind("HTTP/1.1 200 OK"), None); + assert_eq!( + expected_error_kind("upstream returned status: 200 OK after retry"), + None + ); + } + #[test] fn classifies_transient_upstream_http_errors() { // OPENHUMAN-TAURI-5Z: the canonical shape emitted by diff --git a/src/openhuman/inference/provider/config_rejection.rs b/src/openhuman/inference/provider/config_rejection.rs index 542ef8723..c64eaefa4 100644 --- a/src/openhuman/inference/provider/config_rejection.rs +++ b/src/openhuman/inference/provider/config_rejection.rs @@ -15,15 +15,32 @@ //! Moonshot Kimi K2) //! - `"The model \`gpt-5.5\` does not exist or you do not have access to //! it."` / `"model_not_found"` (stale model pin) +//! - `"This model is not available in your region."` (R1 — region-blocked +//! model on a custom cloud provider) +//! - `"ModelNotAllowed"` (R4 — Doubao/ChatGLM model-allowlist enforcement) +//! - `"invalid_authentication_error"` (YC — user pasted a malformed / +//! revoked API key into the provider config) +//! - `"This request requires more credits"` (S5 — OpenRouter `402` when +//! the user's account is out of credits) +//! - `"Invalid model name passed in model="` (Y0 — litellm-style proxy +//! rejecting a model id pre-routing) +//! - `"No active credentials for provider:"` (JN / KB — user hasn't +//! plugged in their API key for the selected provider yet) +//! - `"litellm.BadRequestError"` (JK — litellm github_copilot proxy 400 +//! from a user OAuth/scope gap) +//! - `"not_found_error"` (J2 / J5 / J4 — litellm-compatible envelope +//! `type` field carrying "model 'X' not found") //! //! These are **deterministic user-configuration state**, not bugs the //! maintainers can act on: the user pointed OpenHuman at a custom -//! provider with a model / temperature that provider does not accept. The -//! remediation is "fix the model or routing in Settings", which the UI -//! surfaces. Yet every agent turn produces a fresh Sentry event -//! (OPENHUMAN-TAURI-WJ / -QW / -HB / -NH — 88 + 146 + 39 events). This is -//! the same class as budget-exhaustion ([`super::billing_error`]) and -//! must be demoted from Sentry to an info log the same way. +//! provider with a model / temperature / region / credential that +//! provider does not accept. The remediation is "fix the model, key, or +//! routing in Settings", which the UI surfaces. Yet every agent turn +//! produces a fresh Sentry event (OPENHUMAN-TAURI-WJ / -QW / -HB / -NH / +//! -R1 / -R4 / -YC / -S5 / -Y0 / -JN / -KB / -JK / -J2 / -J5 / -J4 — +//! ~250 additional events on top of the Wave 1-3 IDs). This is the +//! same class as budget-exhaustion ([`super::billing_error`]) and must +//! be demoted from Sentry to an info log the same way. //! //! ## Provider-aware polarity (important) //! @@ -72,6 +89,53 @@ pub fn is_provider_config_rejection_message(body: &str) -> bool { // Our own actionable error once a proper tier→model resolution // is in place (keeps this classifier stable across that fix). "is an abstract tier", + // OPENHUMAN-TAURI-R1 — custom_openai upstream 403 with body + // `{"error":{"message":"This model is not available in your region.","code":403}}`. + // User picked a model the provider blocks for their account's + // region. Sentry has no remediation; user must switch model. + "not available in your region", + // OPENHUMAN-TAURI-R4 — Doubao / ChatGLM-style model allowlist + // enforcement. Body: `{"reason":"ModelNotAllowed",...}`. Match + // lowercased — the provider sends the camelCase token as a + // sentinel `reason` value. + "modelnotallowed", + // OPENHUMAN-TAURI-YC — user-supplied custom_openai API key was + // rejected by upstream with the OpenAI-compatible + // `{"error":{"type":"invalid_authentication_error",...}}` + // envelope. Anchored on the type token (stable across providers + // that emit this OpenAI-compatible body). + "invalid_authentication_error", + // OPENHUMAN-TAURI-S5 — OpenRouter 402 when the user is out of + // credits. Body always carries "requires more credits, or fewer + // max_tokens"; pin to the unique-enough credits phrase. (The + // separate `billing_error` classifier handles our own + // OpenHuman-backend balance gate; this catches the third-party + // OpenRouter shape that re-emits via `agent.run_single`.) + "requires more credits", + // OPENHUMAN-TAURI-Y0 — litellm-style proxy rejected the model + // id pre-routing with `Invalid model name passed in model=…`. + // Anchored on the `passed in model=` suffix so a stray "invalid + // model name" log line elsewhere does not classify. + "invalid model name passed in model=", + // OPENHUMAN-TAURI-JN / -KB — custom provider proxy that fronts + // multiple upstream APIs surfaces a "you haven't configured the + // upstream provider yet" 401/404 as `{"error":{"message":"No + // active credentials for provider: openai",...}}`. The + // remediation is "add the upstream API key in Settings". + "no active credentials for provider", + // OPENHUMAN-TAURI-JK — litellm github_copilot proxy 400 driven + // by the user's missing / expired Copilot OAuth scope. The body + // always starts with the `litellm.BadRequestError:` envelope. + // Anchor to that prefix-shaped substring so we don't catch + // unrelated 400s that merely mention litellm in passing. + "litellm.badrequesterror", + // OPENHUMAN-TAURI-J2 / -J5 / -J4 — litellm-compatible + // envelope with `"type":"not_found_error"` carrying "model 'X' + // not found". Distinct from the existing `model_not_found` + // phrase: that's the `code` field used by OpenAI-native bodies; + // this is the `type` field used by litellm/Anthropic-style + // envelopes for the same class of user-state error. + "not_found_error", ]; let lower = body.to_ascii_lowercase(); @@ -102,6 +166,63 @@ mod tests { } } + #[test] + fn detects_wave4_sentry_bodies() { + // Real wire bodies pulled from the OPENHUMAN-TAURI-* Sentry + // events the Wave 4 phrases drop. + for (sentry_id, body) in [ + ( + "R1", + r#"custom_openai API error (403 Forbidden): {"error":{"message":"This model is not available in your region.","code":403}}"#, + ), + ( + "R4", + r#"custom_openai API error (403 Forbidden): {"code":403,"reason":"ModelNotAllowed","message":"模型不允许访问","metadata":{"request_id":"2026051706431574423265420620337"}}"#, + ), + ( + "YC", + r#"custom_openai API error (401 Unauthorized): {"error":{"message":"Invalid Authentication","type":"invalid_authentication_error"}}"#, + ), + ( + "S5", + r#"custom_openai API error (402 Payment Required): {"error":{"message":"This request requires more credits, or fewer max_tokens. You requested up to 65536 tokens, but can only afford 597.","type":"insufficient_credits"}}"#, + ), + ( + "Y0", + r#"custom_openai API error (400 Bad Request): {"error":{"message":"{'error': '/chat/completions: Invalid model name passed in model=reasoning-v1. Call `/v1/models` to view available models for your key.'}","type":"None"}}"#, + ), + ( + "JN", + r#"custom_openai Responses API error: {"error":{"message":"No active credentials for provider: openai","type":"invalid_request_error","code":"model_not_found"}}"#, + ), + ( + "KB", + r#"OpenHuman API error (404 Not Found): {"error":{"message":"No active credentials for provider: openai","type":"invalid_request_error","code":"model_not_found"}}"#, + ), + ( + "JK", + r#"custom_openai API error (400 Bad Request): {"error":{"message":"litellm.BadRequestError: Github_copilotException - Bad Request. Received Model Group=github_copilot/claude-haiku-4.5\nAvailable Model Group Fallbacks=None","type":null}}"#, + ), + ( + "J2", + r#"custom_openai Responses API error: {"error":{"message":"model 'llama3.3' not found","type":"not_found_error","param":null,"code":null}}"#, + ), + ( + "J5", + r#"custom_openai API error (404 Not Found): {"error":{"message":"model 'llama3.3' not found","type":"not_found_error","param":null,"code":null}}"#, + ), + ( + "J4", + r#"custom_openai streaming API error (404 Not Found): {"error":{"message":"model 'llama3.3' not found","type":"not_found_error","param":null,"code":null}}"#, + ), + ] { + assert!( + is_provider_config_rejection_message(body), + "OPENHUMAN-TAURI-{sentry_id} body must classify as provider config-rejection: {body:?}" + ); + } + } + #[test] fn detection_is_case_insensitive() { assert!(is_provider_config_rejection_message(