mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
revert: remove all Sentry error suppression (12 PRs) (#2959)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> Co-authored-by: sanil-23 <sanil@alphahuman.xyz> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
co-authored by
Steven Enamakel
sanil-23
Claude
parent
12908e2ccd
commit
018248a55a
+3032
-305
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
-52
@@ -66,21 +66,6 @@ fn main() {
|
||||
if openhuman_core::core::observability::is_budget_event(&event) {
|
||||
return None;
|
||||
}
|
||||
// CORE-RUST-EK (~827 events): drop all HTTP 401 responses from the
|
||||
// embeddings call path (domain=embeddings, failure=non_2xx,
|
||||
// status=401). The primary suppression for the OpenHuman-backend
|
||||
// "Invalid token" shape lives in `expected_error_kind` /
|
||||
// `is_session_expired_message`. This is defense-in-depth that also
|
||||
// catches third-party provider 401s (e.g. OpenAI `invalid_api_key`
|
||||
// body) that don't carry the OpenHuman envelope and therefore fall
|
||||
// through the string-based classifier to Sentry.
|
||||
if openhuman_core::core::observability::is_embeddings_api_key_401_event(&event) {
|
||||
log::debug!(
|
||||
"[sentry-embeddings-401-filter] dropping embeddings api-key 401 event_id={:?}",
|
||||
event.event_id
|
||||
);
|
||||
return None;
|
||||
}
|
||||
// Defense-in-depth: drop max-tool-iterations cap events that
|
||||
// slipped past the call-site filters in
|
||||
// `agent::harness::session::runtime::run_single`,
|
||||
@@ -98,43 +83,6 @@ fn main() {
|
||||
{
|
||||
return None;
|
||||
}
|
||||
// Defense-in-depth: upstream rate-limit events that slipped past
|
||||
// the call-site suppressors in `ops::api_error` (primary guard)
|
||||
// and `report_error_or_expected` (secondary guard via
|
||||
// `expected_error_kind`). Catches the three major shapes:
|
||||
// · `rate_limit_error` type in the JSON body (OPENHUMAN-TAURI-2E,
|
||||
// OPENHUMAN-TAURI-RQ — ~2 223 events combined)
|
||||
// · `"upstream rate limit exceeded"` in a 500 body (TAURI-6Y —
|
||||
// ~19 849 events)
|
||||
// · `"429 rate limit exceeded"` in a 500 body (TAURI-S — ~6 984
|
||||
// events)
|
||||
// The primary per-attempt suppression lives in
|
||||
// `openhuman::inference::provider::ops::api_error` (skips
|
||||
// `report_error` entirely for rate-limit bodies) and in
|
||||
// `embeddings::openai::embed` (uses `report_error_or_expected` with
|
||||
// the canonical `"Embedding API error ({status}): …"` format so
|
||||
// `is_transient_upstream_http_message` catches it). This filter is
|
||||
// the last line of defense for any future call site that adds a new
|
||||
// report path without routing through one of those two guards.
|
||||
{
|
||||
let direct = event.message.as_deref();
|
||||
let from_logentry = event.logentry.as_ref().map(|l| l.message.as_str());
|
||||
let from_exception = event.exception.last().and_then(|e| e.value.as_deref());
|
||||
let is_rate_limited = [direct, from_logentry, from_exception]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.map(str::to_ascii_lowercase)
|
||||
.any(|lower| {
|
||||
openhuman_core::core::observability::is_upstream_rate_limit_message(&lower)
|
||||
});
|
||||
if is_rate_limited {
|
||||
log::debug!(
|
||||
"[sentry-rate-limit-filter] dropping upstream rate-limit event_id={:?}",
|
||||
event.event_id
|
||||
);
|
||||
return None;
|
||||
}
|
||||
}
|
||||
// Defense-in-depth: 404 on PATCH/DELETE to a channel-message path
|
||||
// is an expected state (provider-side delete or backend GC). Primary
|
||||
// suppression lives in `authed_json`; this catches any future call
|
||||
|
||||
@@ -29,31 +29,6 @@ fn is_unknown_provider_user_config(err: &str) -> bool {
|
||||
err.contains("no cloud provider with id or slug")
|
||||
}
|
||||
|
||||
/// Returns `true` when the error from a provider chat attempt is a known,
|
||||
/// expected user-state or provider-state condition that already has its own
|
||||
/// Sentry report (or is deterministically expected and has no remediation
|
||||
/// path):
|
||||
///
|
||||
/// - **401 Unauthorized** — API key revoked / wrong key. Already reported by
|
||||
/// the provider layer's `api_error` path. An ops-level duplicate adds noise
|
||||
/// with no additional context.
|
||||
/// - **429 Too Many Requests / rate-limit** — Quota exhaustion. Already
|
||||
/// covered by the `is_upstream_rate_limit_message` classifier in
|
||||
/// `expected_error_kind`; the reliable-provider layer retries with
|
||||
/// backoff before propagating.
|
||||
/// - **Model not found** — User selected a model that doesn't exist for
|
||||
/// their key. The provider layer already classifies this as a config
|
||||
/// rejection (TAURI-RUST-68, ~1309 events).
|
||||
///
|
||||
/// The matcher is intentionally broad so the ops-level wrapper stays out
|
||||
/// of the Sentry funnel for all provider-state failures — the underlying
|
||||
/// call site (`compatible.rs` / `report_error_or_expected`) is already
|
||||
/// responsible for the authoritative report. Unclassified failures (5xx,
|
||||
/// unexpected payloads, network errors) are NOT matched and still escalate.
|
||||
fn is_expected_chat_failure(err: &str) -> bool {
|
||||
crate::core::observability::expected_error_kind(err).is_some()
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, serde::Serialize)]
|
||||
pub struct InferenceTestProviderModelResult {
|
||||
pub reply: String,
|
||||
@@ -204,22 +179,12 @@ pub async fn inference_test_provider_model(
|
||||
output_len = outcome.value.reply.len(),
|
||||
"{LOG_PREFIX} test_provider_model:ok"
|
||||
),
|
||||
Err(err) => {
|
||||
if is_expected_chat_failure(err) {
|
||||
// Provider-state / user-config failure (401, 429, model not
|
||||
// found, API key missing, etc.). The underlying provider
|
||||
// layer already emitted its own Sentry event or classified
|
||||
// this as expected. An ops-level duplicate adds noise.
|
||||
// Targets TAURI-RUST-68 (~1,309 events).
|
||||
warn!(
|
||||
provider,
|
||||
error = %err,
|
||||
"{LOG_PREFIX} test_provider_model:expected-error (no Sentry)"
|
||||
);
|
||||
} else {
|
||||
error!(error = %err, "{LOG_PREFIX} test_provider_model:error");
|
||||
}
|
||||
}
|
||||
Err(err) => error!(
|
||||
workload,
|
||||
provider,
|
||||
error = %err,
|
||||
"{LOG_PREFIX} test_provider_model:error"
|
||||
),
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
@@ -317,74 +317,3 @@ fn is_unknown_provider_user_config_rejects_other_list_models_failures() {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── is_expected_chat_failure (TAURI-RUST-68) ─────────────────────────────
|
||||
//
|
||||
// `inference_test_provider_model` calls `simple_chat` which can fail with
|
||||
// known provider-state or user-config conditions (401, 429, model not
|
||||
// found). Before this fix every failure escalated to `error!`, which
|
||||
// sentry-tracing shipped to Sentry as `"[inference::ops]
|
||||
// test_provider_model:error"` — 1,309 events — while the same underlying
|
||||
// errors already had their own report or were classified as expected by
|
||||
// `expected_error_kind`. The gate demotes them to `warn!` so they stay in
|
||||
// local logs but don't generate duplicate Sentry noise.
|
||||
//
|
||||
// Anchored on the shared `expected_error_kind` classifier so both the unit
|
||||
// tests here and the production gate stay in sync with the central
|
||||
// suppression logic in `core::observability`.
|
||||
|
||||
#[test]
|
||||
fn is_expected_chat_failure_matches_api_key_missing() {
|
||||
// Provider layer emits this phrase when no API key is configured.
|
||||
assert!(is_expected_chat_failure("api key not set for openai"));
|
||||
assert!(is_expected_chat_failure(
|
||||
"missing api key: openai_api_key is not configured"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_expected_chat_failure_matches_rate_limit() {
|
||||
// 429-style rate-limit phrases emitted by the provider / OpenHuman backend.
|
||||
assert!(is_expected_chat_failure(
|
||||
"openai API error (429 Too Many Requests): You exceeded your current quota"
|
||||
));
|
||||
assert!(is_expected_chat_failure(
|
||||
"openai API error (500): 429 rate limit exceeded"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_expected_chat_failure_matches_provider_config_rejection() {
|
||||
// OpenAI-style model-not-found code in error body.
|
||||
assert!(is_expected_chat_failure(
|
||||
r#"custom_openai API error (404 Not Found): {"error":{"message":"The model does not exist or you do not have access","code":"model_not_found"}}"#
|
||||
));
|
||||
// Temperature-unsupported model (e.g. o1/o3/o4 reasoning models).
|
||||
assert!(is_expected_chat_failure(
|
||||
"custom_openai API error (400 Bad Request): invalid temperature: only 1 is allowed"
|
||||
));
|
||||
// litellm-style not_found_error envelope.
|
||||
assert!(is_expected_chat_failure(
|
||||
r#"custom_openai API error (404 Not Found): {"error":{"message":"model 'gpt-99' not found","type":"not_found_error"}}"#
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_expected_chat_failure_does_not_match_real_errors() {
|
||||
// Real errors that must still reach Sentry must NOT be demoted.
|
||||
for raw in [
|
||||
// Genuine 500 server error — actionable, must escalate
|
||||
"openai API error (500 Internal Server Error): Something went wrong",
|
||||
// Unexpected JSON from provider — potential provider bug
|
||||
"openai API returned an unexpected chat-completions payload: missing field",
|
||||
// Local I/O error — real infrastructure problem
|
||||
"failed to open config file: permission denied",
|
||||
// Completely empty string — fallthrough
|
||||
"",
|
||||
] {
|
||||
assert!(
|
||||
!is_expected_chat_failure(raw),
|
||||
"must NOT demote real error: {raw:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,15 +32,6 @@
|
||||
//! from a user OAuth/scope gap)
|
||||
//! - `"not_found_error"` (J2 / J5 / J4 — litellm-compatible envelope
|
||||
//! `type` field carrying "model 'X' not found")
|
||||
//! - `"does not support tools"` / `"function calling is not supported"` /
|
||||
//! `"unknown parameter: tools"` / `"unrecognized field \`tools\`"` /
|
||||
//! `"unsupported parameter: tools"` (TAURI-RUST-4K7 — Ollama models such
|
||||
//! as `gemma3:1b-it-qat` and `huihui_ai/deepseek-r1-abliterated:8b`
|
||||
//! reject tool-enabled requests with HTTP 400. The compatible provider
|
||||
//! already retries without tools, so the initial 400 is not a
|
||||
//! bug — it's expected discovery of the model's capability boundary.
|
||||
//! Sentry noise suppressed here; the retry path in `compatible.rs` runs
|
||||
//! unchanged.)
|
||||
//!
|
||||
//! These are **deterministic user-configuration state**, not bugs the
|
||||
//! maintainers can act on: the user pointed OpenHuman at a custom
|
||||
@@ -171,54 +162,33 @@ pub fn is_provider_config_rejection_message(body: &str) -> bool {
|
||||
// this is the `type` field used by litellm/Anthropic-style
|
||||
// envelopes for the same class of user-state error.
|
||||
"not_found_error",
|
||||
// TAURI-RUST-4NM — nvidia-nim (and some other providers) return
|
||||
// `{"error":{"message":"model field is required","type":"invalid_request_error","code":"missing_required_field"}}`
|
||||
// when the `model` key is absent or empty in the request body.
|
||||
// This is a user-configuration error (provider string has no model
|
||||
// component, e.g. `nvidia-nim:` with empty model), not a product
|
||||
// regression. Demote from Sentry; the factory now validates this
|
||||
// up-front so in practice this phrase should no longer appear.
|
||||
"model field is required",
|
||||
// TAURI-RUST-4XK — Ollama 403 when the requested model requires a
|
||||
// paid Ollama subscription. Body carries the upgrade URL. User must
|
||||
// switch to a free model or upgrade their Ollama account.
|
||||
"requires a subscription",
|
||||
// TAURI-RUST-2G / TAURI-RUST-2F — DeepSeek / compatible providers
|
||||
// that use extended thinking reject tool-call turns when the
|
||||
// `reasoning_content` block from a prior assistant turn is not
|
||||
// threaded back. This is user-config state (model requires the
|
||||
// caller to replay the thinking block; the frontend replay logic in
|
||||
// `turn.rs` handles it for subsequent turns, so the first-turn 400
|
||||
// is expected capability-discovery, not a regression).
|
||||
"in the thinking mode must be passed back",
|
||||
// TAURI-RUST-35 / TAURI-RUST-4K7 / TAURI-RUST-4Z0 — Ollama models
|
||||
// (e.g. gemma3, phi3, deepseek-r1) that do not support function
|
||||
// calling return HTTP 400 with this phrase. The compatible provider
|
||||
// retries without tools on 400, so the initial rejection is expected
|
||||
// capability-discovery. Sentry noise suppressed here.
|
||||
"does not support tools",
|
||||
// TAURI-RUST-4K7-d — alternative phrasing used by some Ollama model
|
||||
// versions for the same tool-unsupported condition.
|
||||
"function calling is not supported",
|
||||
// TAURI-RUST-4K7-e — litellm / OpenAI-compatible proxies reject the
|
||||
// `tools` key in the request body when the backing model does not
|
||||
// support tool use (e.g. local Ollama via LiteLLM gateway).
|
||||
"unknown parameter: tools",
|
||||
// TAURI-RUST-4K7-f — Ollama native API surface rejects the field
|
||||
// outright when the model has no function-calling capability.
|
||||
"unrecognized field `tools`",
|
||||
// TAURI-RUST-4K7-g — another litellm / proxy variant of the same
|
||||
// tool-unsupported condition.
|
||||
"unsupported parameter: tools",
|
||||
// TAURI-RUST-4NM — nvidia-nim (and compatible providers) return
|
||||
// `{"error":{"message":"model field is required","code":"missing_required_field"}}`
|
||||
// when the request body contains an empty `"model":""` field.
|
||||
"model field is required",
|
||||
// TAURI-RUST-2G (~2684 events) / TAURI-RUST-2F (~950 events) —
|
||||
// thinking-mode model rejects a follow-up turn that doesn't echo
|
||||
// the prior assistant's `reasoning_content` field.
|
||||
// thinking-mode model (DeepSeek-R1 / Moonshot K2-thinking on
|
||||
// `provider=cloud` custom_openai) rejects a follow-up turn that
|
||||
// doesn't echo the prior assistant's `reasoning_content` field.
|
||||
// Body shape (backtick-quoted JSON literal in the upstream body):
|
||||
// `{"error":{"message":"The `reasoning_content` in the thinking
|
||||
// mode must be passed back to the API.",...}}`. The
|
||||
// provider-contract gap is on our side, but until the thinking-
|
||||
// mode round-tripping ships in the inference layer, every affected
|
||||
// turn fires a fresh Sentry event — and the UI already surfaces
|
||||
// the actionable error to the user. Anchor on the unique
|
||||
// `thinking mode must be passed back` substring so the match
|
||||
// doesn't depend on the upstream's backtick-quoting around
|
||||
// `reasoning_content` (some provider versions ship without them).
|
||||
"thinking mode must be passed back",
|
||||
// TAURI-RUST-4XK (~649 events) — Ollama Cloud subscription gate.
|
||||
// Body: `{"error":"this model requires a subscription, upgrade for
|
||||
// access: https://ollama.com/upgrade (ref: <uuid>)"}` on a 403
|
||||
// Forbidden from `compatible::OpenAiCompatibleProvider` with
|
||||
// `name = "ollama"`. User-state: the model picked in Settings is
|
||||
// a paid-tier Ollama Cloud model the user's account doesn't
|
||||
// cover. The UI surfaces an actionable upgrade link in the
|
||||
// remediation message itself.
|
||||
"requires a subscription, upgrade for access",
|
||||
// TAURI-RUST-1V / OPENHUMAN-TAURI-4JS —
|
||||
// `reliable.rs::format_failure_aggregate` (no-configured-fallbacks
|
||||
@@ -426,103 +396,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// TAURI-RUST-35 family — model picked by the user doesn't implement
|
||||
/// tool calling. The agent harness tries to send a tool spec and the
|
||||
/// upstream (Ollama / cloud Ollama relay / hosted OpenAI-compatible)
|
||||
/// rejects with `{"error":{"message":"<model> does not support tools",
|
||||
/// "type":"invalid_request_error",...}}`. Pure user-config — the user
|
||||
/// has to pick a tool-capable model (or run a non-agent flow). No
|
||||
/// remediation path through Sentry, and the long tail is large: each
|
||||
/// distinct model id + provider prefix combo creates a new Sentry
|
||||
/// fingerprint, so the same root cause is currently split across at
|
||||
/// least 10 unresolved Sentry issues (458 events total as of
|
||||
/// 2026-05-28):
|
||||
///
|
||||
/// | shortId | events | provider prefix |
|
||||
/// |---|---|---|
|
||||
/// | TAURI-RUST-35 | 307 | cloud |
|
||||
/// | TAURI-RUST-DF | 83 | cloud |
|
||||
/// | TAURI-RUST-123 | 25 | cloud |
|
||||
/// | TAURI-RUST-4K7 | 19 | ollama |
|
||||
/// | TAURI-RUST-4FS | 10 | cloud |
|
||||
/// | TAURI-RUST-4F6 | 5 | cloud |
|
||||
/// | TAURI-RUST-2YA | 4 | cloud |
|
||||
/// | TAURI-RUST-4KR | 3 | ollama |
|
||||
/// | TAURI-RUST-4KH | 1 | cloud |
|
||||
/// | TAURI-RUST-4KY | 1 | ollama |
|
||||
///
|
||||
/// Anchored on the exact `"does not support tools"` substring (the
|
||||
/// message body's stable token — the model id varies per user). The
|
||||
/// `streaming API error` / `API error` wrappers and the
|
||||
/// `cloud` / `ollama` / `custom_openai` provider prefixes all share
|
||||
/// this body, so a single phrase covers every variant.
|
||||
#[test]
|
||||
fn detects_does_not_support_tools_family() {
|
||||
for (sentry_id, body) in [
|
||||
// TAURI-RUST-35 — verbatim from latest issue 168 event
|
||||
// (model=`gemma3:1b-it-qat`, provider=cloud).
|
||||
(
|
||||
"35",
|
||||
r#"cloud streaming API error (400 Bad Request): {"error":{"message":"registry.ollama.ai/library/gemma3:1b-it-qat does not support tools","type":"invalid_request_error","param":null,"code":null}}"#,
|
||||
),
|
||||
// TAURI-RUST-4K7 — ollama prefix, different upstream wrapper.
|
||||
(
|
||||
"4K7",
|
||||
r#"ollama streaming API error (400 Bad Request): {"error":{"message":"some-local-model does not support tools","type":"invalid_request_error","param":null,"code":null}}"#,
|
||||
),
|
||||
// Non-streaming sibling — `API error` (no `streaming` token)
|
||||
// for hosted providers that aren't using the streaming endpoint.
|
||||
(
|
||||
"non-streaming",
|
||||
r#"cloud API error (400 Bad Request): {"error":{"message":"registry.ollama.ai/library/qwen2.5:0.5b does not support tools","type":"invalid_request_error"}}"#,
|
||||
),
|
||||
// Bare body (no wrapper) — what `expected_error_kind` would
|
||||
// see if the body got extracted from the envelope upstream.
|
||||
(
|
||||
"bare",
|
||||
r#"{"error":{"message":"phi3.5:mini does not support tools","type":"invalid_request_error"}}"#,
|
||||
),
|
||||
// TAURI-RUST-4Z0 — verbatim from issue 5664 (model=`deepseek-r1:8b`,
|
||||
// provider=ollama). The envelope carries `"type":"api_error"`
|
||||
// rather than `"invalid_request_error"` — pin it so the matcher
|
||||
// can never be narrowed to require a specific `type` token; the
|
||||
// `"does not support tools"` body substring is the only anchor.
|
||||
(
|
||||
"4Z0",
|
||||
r#"ollama streaming API error (400 Bad Request): {"error":{"message":"registry.ollama.ai/library/deepseek-r1:8b does not support tools","type":"api_error","param":null,"code":null}}"#,
|
||||
),
|
||||
] {
|
||||
assert!(
|
||||
is_provider_config_rejection_message(body),
|
||||
"TAURI-RUST-{sentry_id} body must classify as provider config-rejection: {body:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Polarity guard for the does-not-support-tools arm. The phrase is
|
||||
/// scoped enough that no real bug-class body should accidentally
|
||||
/// match — but pin a few near-miss shapes so a future loosening of
|
||||
/// the matcher can't silently re-classify them.
|
||||
#[test]
|
||||
fn does_not_classify_unrelated_tools_phrases_as_config_rejection() {
|
||||
for body in [
|
||||
// Tool-call dispatch failure (real bug) — must reach Sentry.
|
||||
"tool execution failed: shell returned exit 1",
|
||||
// Generic "tools" mention without the does-not-support phrase.
|
||||
"agent ran with 0 tools available",
|
||||
// Reversed phrasing — provider says they DO support tools but
|
||||
// the call shape is wrong. Still actionable for triage.
|
||||
"supports tools but received malformed tool_calls array",
|
||||
// Empty body.
|
||||
"",
|
||||
] {
|
||||
assert!(
|
||||
!is_provider_config_rejection_message(body),
|
||||
"{body:?} must NOT classify as a provider config-rejection"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_reliable_aggregate_no_fallbacks_envelope() {
|
||||
// OPENHUMAN-TAURI-4JS — `reliable::format_failure_aggregate`
|
||||
@@ -724,64 +597,4 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// TAURI-RUST-4K7 — Ollama models that don't support tool calling
|
||||
/// (e.g. `gemma3:1b-it-qat`, `huihui_ai/deepseek-r1-abliterated:8b`)
|
||||
/// return HTTP 400 with one of several tool-rejection phrases.
|
||||
/// The compatible provider retries without tools, so the 400 is expected
|
||||
/// capability-discovery rather than a product bug. These phrases must be
|
||||
/// classified as config-rejections so Sentry is not flooded on every turn.
|
||||
#[test]
|
||||
fn detects_ollama_tool_unsupported_bodies() {
|
||||
for (sentry_id, body) in [
|
||||
(
|
||||
"4K7-a",
|
||||
r#"{"error":"gemma3:1b-it-qat does not support tools"}"#,
|
||||
),
|
||||
(
|
||||
"4K7-b",
|
||||
r#"{"error":"huihui_ai/deepseek-r1-abliterated:8b does not support tools"}"#,
|
||||
),
|
||||
(
|
||||
"4K7-c",
|
||||
r#"ollama streaming API error (400 Bad Request): {"error":"phi3:mini does not support tools"}"#,
|
||||
),
|
||||
(
|
||||
"4K7-d",
|
||||
r#"{"error":"function calling is not supported by this model"}"#,
|
||||
),
|
||||
(
|
||||
"4K7-e",
|
||||
r#"{"error":{"message":"unknown parameter: tools","type":"invalid_request_error"}}"#,
|
||||
),
|
||||
(
|
||||
"4K7-f",
|
||||
r#"{"error":"unrecognized field `tools` in request body"}"#,
|
||||
),
|
||||
(
|
||||
"4K7-g",
|
||||
r#"{"error":{"message":"unsupported parameter: tools","type":"invalid_request_error"}}"#,
|
||||
),
|
||||
] {
|
||||
assert!(
|
||||
is_provider_config_rejection_message(body),
|
||||
"TAURI-RUST-{sentry_id} body must classify as provider config-rejection (tool-unsupported): {body:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_ollama_tool_unsupported_bodies_case_insensitive() {
|
||||
// Ollama error messages should match regardless of casing.
|
||||
for body in [
|
||||
"Model 'gemma3:1b-it-qat' DOES NOT SUPPORT TOOLS",
|
||||
"Function Calling Is Not Supported By This Model",
|
||||
"Unknown Parameter: Tools",
|
||||
] {
|
||||
assert!(
|
||||
is_provider_config_rejection_message(body),
|
||||
"{body:?} must classify as config-rejection regardless of case"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -144,22 +144,6 @@ async fn list_configured_models_from_config(
|
||||
|
||||
let status = response.status();
|
||||
if !status.is_success() {
|
||||
// A 404 from the /models endpoint means the provider does not support model
|
||||
// listing — this is expected for many OpenAI-compatible providers (e.g. DeepSeek,
|
||||
// Moonshot, Kimi, custom proxies). Return an empty model list so the caller can
|
||||
// proceed normally instead of surfacing a spurious error / Sentry event.
|
||||
// (Sentry issue TAURI-RUST-1Z — 819 events from this path alone.)
|
||||
if status == reqwest::StatusCode::NOT_FOUND {
|
||||
log::debug!(
|
||||
"[providers][list_models] slug={} returned 404 — provider does not support /models listing; returning empty list",
|
||||
entry.slug
|
||||
);
|
||||
return Ok(crate::rpc::RpcOutcome::new(
|
||||
serde_json::json!({ "models": serde_json::Value::Array(vec![]), "unsupported": true }),
|
||||
vec!["provider does not support model listing (404)".to_string()],
|
||||
));
|
||||
}
|
||||
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let sanitized = sanitize_api_error(&body);
|
||||
let truncated = crate::openhuman::util::truncate_with_ellipsis(&sanitized, 300);
|
||||
@@ -985,38 +969,16 @@ pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::E
|
||||
} else if is_context_window_exceeded {
|
||||
log_context_window_exceeded("api_error", provider, None, status);
|
||||
} else if should_report_provider_http_failure(status) {
|
||||
// Defense-in-depth: some backends (e.g. OpenHuman) wrap an upstream
|
||||
// provider 429 as an HTTP 500 with a rate-limit phrase in the body
|
||||
// (`"429 rate limit exceeded"`, `"upstream rate limit exceeded"`).
|
||||
// `should_report_provider_http_failure(500)` would otherwise let this
|
||||
// through to Sentry — suppress it here before the report fires so the
|
||||
// noise stays off Sentry (OPENHUMAN-TAURI-S: ~6 984 events).
|
||||
// The `expected_error_kind` classifier in `report_error_or_expected`
|
||||
// catches the same shape at re-report sites (agent / web_channel).
|
||||
let lower_body = body.to_ascii_lowercase();
|
||||
let is_rate_limit_body =
|
||||
crate::core::observability::is_upstream_rate_limit_message(&lower_body);
|
||||
if is_rate_limit_body {
|
||||
tracing::warn!(
|
||||
domain = "llm_provider",
|
||||
operation = "api_error",
|
||||
provider = provider,
|
||||
status = status_str.as_str(),
|
||||
"[llm_provider] api_error: skipping Sentry report — rate-limit body in \
|
||||
non-429 response ({status})"
|
||||
);
|
||||
} else {
|
||||
crate::core::observability::report_error(
|
||||
message.as_str(),
|
||||
"llm_provider",
|
||||
"api_error",
|
||||
&[
|
||||
("provider", provider),
|
||||
("status", status_str.as_str()),
|
||||
("failure", "non_2xx"),
|
||||
],
|
||||
);
|
||||
}
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -288,69 +288,6 @@ async fn openrouter_key_is_trimmed_for_validation_and_catalog_probe() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Spawn a minimal axum server that always returns 404 for the /models endpoint.
|
||||
/// Used to verify that providers without model listing return an empty list,
|
||||
/// not an error (Sentry issue TAURI-RUST-1Z).
|
||||
async fn spawn_models_404_server() -> String {
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("bind");
|
||||
let addr = listener.local_addr().expect("local_addr");
|
||||
let app = axum::Router::new().route(
|
||||
"/models",
|
||||
axum::routing::get(|| async {
|
||||
(axum::http::StatusCode::NOT_FOUND, "Not Found").into_response()
|
||||
}),
|
||||
);
|
||||
tokio::spawn(async move {
|
||||
axum::serve(listener, app).await.expect("serve");
|
||||
});
|
||||
format!("http://{addr}")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn models_404_returns_empty_list_not_error() {
|
||||
// Providers that return 404 on /models (e.g. DeepSeek, Kimi, custom proxies)
|
||||
// must yield an empty model list, not an Err. Returning an Err was firing a
|
||||
// Sentry error for every `inference_list_models` call (TAURI-RUST-1Z, 819 events).
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let endpoint = spawn_models_404_server().await;
|
||||
|
||||
let mut config = Config {
|
||||
config_path: tmp.path().join("config.toml"),
|
||||
workspace_dir: tmp.path().join("workspace"),
|
||||
action_dir: tmp.path().join("workspace"),
|
||||
..Config::default()
|
||||
};
|
||||
config.secrets.encrypt = false;
|
||||
config.cloud_providers.push(CloudProviderCreds {
|
||||
id: "p_custom_test".to_string(),
|
||||
slug: "custom-no-models".to_string(),
|
||||
label: "Custom (no /models)".to_string(),
|
||||
endpoint,
|
||||
auth_style: AuthStyle::Bearer,
|
||||
legacy_type: None,
|
||||
default_model: None,
|
||||
});
|
||||
|
||||
let outcome = list_configured_models_from_config("custom-no-models", &config)
|
||||
.await
|
||||
.expect("404 from /models must succeed with an empty list");
|
||||
|
||||
let models = outcome.value["models"]
|
||||
.as_array()
|
||||
.expect("response must have a `models` array");
|
||||
assert!(
|
||||
models.is_empty(),
|
||||
"expected empty model list for a 404 /models endpoint, got: {models:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.value["unsupported"],
|
||||
serde_json::Value::Bool(true),
|
||||
"unsupported flag must be set to true for 404 providers"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn factory_backend() {
|
||||
assert!(create_backend_inference_provider(
|
||||
@@ -455,61 +392,6 @@ mod budget_exhausted_suppression {
|
||||
}
|
||||
}
|
||||
|
||||
// Tests for the rate-limit body suppression guard added to `api_error`.
|
||||
// Exercises `is_upstream_rate_limit_message` with the exact body shapes that
|
||||
// produced OPENHUMAN-TAURI-S (~6 984 events from HTTP 500 wrapping a
|
||||
// "429 rate limit exceeded" body) and OPENHUMAN-TAURI-6Y / -2E.
|
||||
mod rate_limit_body_suppression {
|
||||
use crate::core::observability::is_upstream_rate_limit_message;
|
||||
|
||||
/// HTTP 500 with a `"429 rate limit exceeded"` body must be detected
|
||||
/// as a rate-limit signal so the guard in `api_error` can skip the
|
||||
/// Sentry report (OPENHUMAN-TAURI-S).
|
||||
#[test]
|
||||
fn http_500_with_429_body_phrase_is_rate_limited() {
|
||||
let body = r#"{"success":false,"error":"429 rate limit exceeded, please try again later"}"#
|
||||
.to_ascii_lowercase();
|
||||
assert!(
|
||||
is_upstream_rate_limit_message(&body),
|
||||
"500-body with '429 rate limit exceeded' must be detected as rate-limited"
|
||||
);
|
||||
}
|
||||
|
||||
/// HTTP 500 with an `"upstream rate limit exceeded"` body
|
||||
/// (OPENHUMAN-TAURI-6Y shape).
|
||||
#[test]
|
||||
fn http_500_with_upstream_rate_limit_body_is_rate_limited() {
|
||||
let body = r#"{"success":false,"error":"Upstream rate limit exceeded for model 'summarization-v1'. Please retry shortly.","details":{"provider":"gmi"}}"#
|
||||
.to_ascii_lowercase();
|
||||
assert!(
|
||||
is_upstream_rate_limit_message(&body),
|
||||
"500-body with 'upstream rate limit exceeded' must be detected"
|
||||
);
|
||||
}
|
||||
|
||||
/// OpenAI / Anthropic `"rate_limit_error"` type body.
|
||||
#[test]
|
||||
fn body_with_rate_limit_error_type_is_rate_limited() {
|
||||
let body = r#"{"error":{"message":"Rate limit exceeded. Please retry after a brief wait.","type":"rate_limit_error"}}"#
|
||||
.to_ascii_lowercase();
|
||||
assert!(
|
||||
is_upstream_rate_limit_message(&body),
|
||||
"body with 'rate_limit_error' type must be detected"
|
||||
);
|
||||
}
|
||||
|
||||
/// Unrelated 500 body must NOT be detected as rate-limited.
|
||||
#[test]
|
||||
fn http_500_unrelated_body_is_not_rate_limited() {
|
||||
let body = r#"{"success":false,"error":"internal server error: database unavailable"}"#
|
||||
.to_ascii_lowercase();
|
||||
assert!(
|
||||
!is_upstream_rate_limit_message(&body),
|
||||
"unrelated 500 body must not be detected as rate-limited"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
mod provider_access_policy_suppression {
|
||||
use super::*;
|
||||
|
||||
|
||||
@@ -51,13 +51,13 @@ fn report_ollama_health_gate_once(base_url: &str, model: &str) -> bool {
|
||||
let sentry_message = format!(
|
||||
"ollama embeddings opted-in but daemon unreachable at {base_url}; falling back to cloud embeddings for this session"
|
||||
);
|
||||
// Route through `report_error_or_expected` so the `expected_error_kind`
|
||||
// classifier runs. The wire shape `"ollama embeddings opted-in but daemon
|
||||
// unreachable at …"` matches `is_ollama_user_config_rejection` → routes to
|
||||
// `ExpectedErrorKind::ProviderUserState` → demoted to a warn breadcrumb,
|
||||
// NOT a Sentry error event. Using `report_error_message` directly would
|
||||
// bypass the classifier and fire `sentry::capture_message(…, Level::Error)`
|
||||
// unconditionally — the root cause of TAURI-RUST-B (472 events).
|
||||
// Route through `report_error_or_expected` so the GX arm of
|
||||
// `is_ollama_user_config_rejection` in `expected_error_kind` demotes
|
||||
// the message to an info breadcrumb (user-state: ollama daemon not
|
||||
// running). Direct `report_error_message` here bypassed the classifier
|
||||
// and produced TAURI-RUST-B (~409 events). The `&str` input avoids
|
||||
// the `format!("{:#}")` round-trip that `report_error` would do on an
|
||||
// anyhow chain — the wire shape stays bit-identical.
|
||||
crate::core::observability::report_error_or_expected(
|
||||
sentry_message.as_str(),
|
||||
"memory",
|
||||
@@ -766,25 +766,4 @@ mod tests {
|
||||
"different URL also suppressed — gate is process-scoped, not per-URL"
|
||||
);
|
||||
}
|
||||
|
||||
/// TAURI-RUST-B (issue #2921): the exact wire shape produced by
|
||||
/// `report_ollama_health_gate_once` must classify as
|
||||
/// `ExpectedErrorKind::ProviderUserState` so `report_error_or_expected`
|
||||
/// routes it to a warn breadcrumb rather than a Sentry error event.
|
||||
///
|
||||
/// Previously the gate called `report_error_message` directly, bypassing
|
||||
/// the classifier and firing `sentry::capture_message(…, Level::Error)`
|
||||
/// unconditionally for every process restart where Ollama was opted-in but
|
||||
/// not running (472 events at time of fix). The fix routes through
|
||||
/// `report_error_or_expected` which checks `expected_error_kind` first.
|
||||
#[test]
|
||||
fn tauri_rust_b_wire_shape_classifies_as_expected() {
|
||||
// Canonical format produced by `report_ollama_health_gate_once`.
|
||||
let msg = "ollama embeddings opted-in but daemon unreachable at http://localhost:11434; falling back to cloud embeddings for this session";
|
||||
assert_eq!(
|
||||
crate::core::observability::expected_error_kind(msg),
|
||||
Some(crate::core::observability::ExpectedErrorKind::ProviderUserState),
|
||||
"TAURI-RUST-B — daemon-unreachable health-gate message must demote to ProviderUserState, not fire as Sentry error"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,17 +2,6 @@
|
||||
//!
|
||||
//! Follows the cron module's `with_connection` pattern: opens the database,
|
||||
//! runs DDL on every connection, and provides pure functions.
|
||||
//!
|
||||
//! ## Init-failure noise suppression (TAURI-RUST-A)
|
||||
//!
|
||||
//! `with_connection` runs the schema DDL on every call. Transient
|
||||
//! `SQLITE_BUSY` / `SQLITE_LOCKED` errors (e.g. concurrent in-process RPC
|
||||
//! calls, antivirus hold, network-drive WAL rejection) are handled by a
|
||||
//! per-connection busy timeout (5 s) plus an application-level retry loop
|
||||
//! (3 retries, 100 / 300 / 900 ms backoff). Only `DatabaseBusy` /
|
||||
//! `DatabaseLocked` errors are retried — schema or corruption errors fail
|
||||
//! through immediately so Sentry captures a real root cause rather than a
|
||||
//! transient noise event.
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use rusqlite::{Connection, OptionalExtension};
|
||||
@@ -72,8 +61,9 @@ pub fn with_connection<T>(
|
||||
let db_path = workspace_dir.join("subconscious").join("subconscious.db");
|
||||
if let Some(parent) = db_path.parent() {
|
||||
std::fs::create_dir_all(parent)
|
||||
.with_context(|| format!("create subconscious dir: {}", parent.display()))?;
|
||||
.with_context(|| format!("failed to create subconscious dir: {}", parent.display()))?;
|
||||
}
|
||||
|
||||
let conn = open_and_initialize_with_retry(&db_path)?;
|
||||
f(&conn)
|
||||
}
|
||||
|
||||
@@ -1971,7 +1971,6 @@ async fn inference_provider_factory_and_classifiers_cover_user_state_edges() {
|
||||
"The supported API model names are native-a or native-b",
|
||||
"ModelNotAllowed",
|
||||
"invalid_authentication_error",
|
||||
"unknown parameter: tools",
|
||||
"requires a subscription, upgrade for access",
|
||||
"No active credentials for provider: openai",
|
||||
] {
|
||||
@@ -1983,6 +1982,12 @@ async fn inference_provider_factory_and_classifiers_cover_user_state_edges() {
|
||||
assert!(is_openai_compatible_unknown_model_message(
|
||||
"Model `gpt-unknown` is not available. Use GET /openai/v1/models to list available models."
|
||||
));
|
||||
// PR #2959 reverted the "unknown parameter: tools" suppression: this shape
|
||||
// is no longer demoted to user-config state, so it fires to Sentry again
|
||||
// (root cause to be fixed separately).
|
||||
assert!(!is_provider_config_rejection_message(
|
||||
"unknown parameter: tools"
|
||||
));
|
||||
assert!(!is_provider_config_rejection_message(
|
||||
"internal server error while streaming tokens"
|
||||
));
|
||||
|
||||
@@ -340,11 +340,17 @@ async fn provider_ops_leftovers_cover_model_listing_error_shapes_and_auth_styles
|
||||
.expect_err("unknown provider id");
|
||||
assert!(unknown.contains("no cloud provider"));
|
||||
|
||||
let unsupported = list_configured_models("not-found")
|
||||
// PR #2959 reverted the list_models 404 suppression: a 404 from the
|
||||
// /models endpoint no longer returns a synthetic `{models: [], unsupported:
|
||||
// true}` success — it surfaces as a real error so the failure fires to
|
||||
// Sentry and gets a root-cause fix (e.g. a wrong base URL).
|
||||
let not_found_err = list_configured_models("not-found")
|
||||
.await
|
||||
.expect("404 models unsupported");
|
||||
assert_eq!(unsupported.value["models"].as_array().unwrap().len(), 0);
|
||||
assert_eq!(unsupported.value["unsupported"], true);
|
||||
.expect_err("404 list_models now surfaces as an error");
|
||||
assert!(
|
||||
not_found_err.contains("provider returned 404"),
|
||||
"404 list_models error should surface the status: {not_found_err:?}"
|
||||
);
|
||||
|
||||
let missing_data = list_configured_models("missing-data")
|
||||
.await
|
||||
|
||||
@@ -310,11 +310,16 @@ async fn provider_factory_and_model_listing_cover_cloud_local_and_invalid_shapes
|
||||
.value;
|
||||
assert_eq!(local_listed["models"][0]["id"], "demo-chat");
|
||||
|
||||
let unsupported = list_configured_models("missing")
|
||||
// PR #2959 reverted the list_models 404 suppression: a 404 from /models
|
||||
// now surfaces as a real error instead of a synthetic `unsupported: true`
|
||||
// success, so the failure fires to Sentry for a root-cause fix.
|
||||
let missing_err = list_configured_models("missing")
|
||||
.await
|
||||
.expect("404 unsupported")
|
||||
.value;
|
||||
assert_eq!(unsupported["unsupported"], true);
|
||||
.expect_err("404 list_models now surfaces as an error");
|
||||
assert!(
|
||||
missing_err.contains("provider returned 404"),
|
||||
"404 list_models error should surface the status: {missing_err:?}"
|
||||
);
|
||||
|
||||
let openrouter = list_configured_models("openrouter")
|
||||
.await
|
||||
|
||||
@@ -393,18 +393,19 @@ async fn inference_provider_success_paths_use_mock_models_and_chat() {
|
||||
"mock model should round-trip through provider /models: {models}"
|
||||
);
|
||||
|
||||
let unsupported = rpc(
|
||||
// PR #2959 reverted the list_models 404 suppression: a 404 from /models
|
||||
// now surfaces as a JSON-RPC error instead of a synthetic `unsupported:
|
||||
// true` success, so the failure fires to Sentry for a root-cause fix.
|
||||
let models_404 = rpc(
|
||||
&harness.rpc_base,
|
||||
102,
|
||||
"openhuman.inference_list_models",
|
||||
json!({ "provider_id": "mock-404" }),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
payload(&unsupported, "inference_list_models 404")
|
||||
.get("unsupported")
|
||||
.and_then(Value::as_bool),
|
||||
Some(true)
|
||||
assert!(
|
||||
error_message(&models_404, "inference_list_models 404").contains("provider returned 404"),
|
||||
"404 list_models should surface as an error: {models_404}"
|
||||
);
|
||||
|
||||
let reply = rpc(
|
||||
|
||||
Reference in New Issue
Block a user