mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(agent): suppress empty-provider-response from Sentry (TAURI-RUST-4JX) (#2790)
This commit is contained in:
@@ -31,6 +31,17 @@ pub enum AgentError {
|
||||
/// Typically indicates an infinite loop in the model's reasoning.
|
||||
MaxIterationsExceeded { max: usize },
|
||||
|
||||
/// The provider's chat completion contained no text, no thinking, and
|
||||
/// no tool calls — a degenerate / poisoned response. Typically observed
|
||||
/// with flaky local model fine-tunes (e.g. community quantizations of
|
||||
/// Qwen/Llama via LM Studio or Ollama). Surfaced as a user-facing
|
||||
/// error instead of a silent blank reply (defense-in-depth from
|
||||
/// `agent/harness/session/turn.rs`) but suppressed from Sentry — it's
|
||||
/// a provider/user-state outcome, not an OpenHuman bug, and a deeper
|
||||
/// fix lives in the model / provider config the user chose. Targets
|
||||
/// Sentry TAURI-RUST-4JX (~33 events, escalating on 0.56.0).
|
||||
EmptyProviderResponse { iteration: usize },
|
||||
|
||||
/// Automated history compaction (summarization) failed.
|
||||
CompactionFailed {
|
||||
message: String,
|
||||
@@ -78,6 +89,12 @@ impl fmt::Display for AgentError {
|
||||
Self::MaxIterationsExceeded { max } => {
|
||||
write!(f, "{MAX_ITERATIONS_ERROR_PREFIX} ({max})")
|
||||
}
|
||||
Self::EmptyProviderResponse { .. } => {
|
||||
// Verbatim user-facing string from the old
|
||||
// `agent/harness/session/turn.rs` emit site — UI / tests
|
||||
// grep for this exact byte sequence.
|
||||
write!(f, "The model returned an empty response. Please try again.")
|
||||
}
|
||||
Self::CompactionFailed {
|
||||
message,
|
||||
consecutive_failures,
|
||||
@@ -111,6 +128,31 @@ impl std::error::Error for AgentError {
|
||||
}
|
||||
}
|
||||
|
||||
impl AgentError {
|
||||
/// User/provider-state outcomes that the UI already surfaces to the
|
||||
/// user and that no developer can act on from Sentry — `run_single`
|
||||
/// suppresses their Sentry emission (`log::info!` only) while still
|
||||
/// returning the `Err` so the existing `AgentError` + `recoverable`
|
||||
/// semantics are preserved.
|
||||
///
|
||||
/// - `MaxIterationsExceeded`: deterministic tool-loop cap, drives
|
||||
/// OPENHUMAN-TAURI-99 / -98 suppression.
|
||||
/// - `EmptyProviderResponse`: degenerate/poisoned chat completion,
|
||||
/// drives TAURI-RUST-4JX suppression.
|
||||
///
|
||||
/// Other variants are real failures (`ProviderError` upstream HTTP /
|
||||
/// network, `ToolExecutionError` callable bug, `ContextLimitExceeded`
|
||||
/// compaction gap, `CostBudgetExceeded`, `CompactionFailed`,
|
||||
/// `PermissionDenied` config bug, `Other` escape hatch) and must
|
||||
/// continue to escalate.
|
||||
pub fn skips_sentry(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::MaxIterationsExceeded { .. } | Self::EmptyProviderResponse { .. }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<anyhow::Error> for AgentError {
|
||||
fn from(e: anyhow::Error) -> Self {
|
||||
// Attempt to recover a typed AgentError that was wrapped in anyhow.
|
||||
@@ -252,4 +294,76 @@ mod tests {
|
||||
assert!(matches!(other, AgentError::Other(_)));
|
||||
assert!(other.source().is_some());
|
||||
}
|
||||
|
||||
// ── AgentError::EmptyProviderResponse (TAURI-RUST-4JX) ──────────────────
|
||||
//
|
||||
// `agent::harness::session::turn` returns this variant when the provider's
|
||||
// chat completion contains no text, no thinking, and no tool calls (a
|
||||
// degenerate/poisoned response — typically a flaky local model). The
|
||||
// variant was added so `run_single` can route it through `skips_sentry()`
|
||||
// and demote like `MaxIterationsExceeded`, keeping TAURI-RUST-4JX off
|
||||
// Sentry while preserving the user-visible error and the `Err` propagation
|
||||
// contract.
|
||||
|
||||
#[test]
|
||||
fn empty_provider_response_display_matches_user_facing_string() {
|
||||
// The exact wire string is anchored: the UI surfaces it verbatim to
|
||||
// the user, and the emit-site comment at
|
||||
// `agent/harness/session/turn.rs:801` (the warn breadcrumb) explicitly
|
||||
// calls out the "surfacing as error instead of a silent blank reply"
|
||||
// contract. Any change to this byte string is a user-visible message
|
||||
// change and a Sentry-fingerprint change.
|
||||
let err = AgentError::EmptyProviderResponse { iteration: 1 };
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"The model returned an empty response. Please try again."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_sentry_returns_true_for_known_user_state_variants() {
|
||||
// The two variants that represent user/provider state rather than a
|
||||
// code bug — `run_single` suppresses both from Sentry while still
|
||||
// returning `Err` so the user sees the failure.
|
||||
assert!(AgentError::MaxIterationsExceeded { max: 10 }.skips_sentry());
|
||||
assert!(AgentError::EmptyProviderResponse { iteration: 1 }.skips_sentry());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_sentry_returns_false_for_real_failures() {
|
||||
// Every other variant represents either an actionable bug, an
|
||||
// upstream provider/network failure that triage cares about, or a
|
||||
// CompactionFailed that already has its own follow-up logic — none
|
||||
// of them should silently disappear from Sentry.
|
||||
let real_failures = [
|
||||
AgentError::ProviderError {
|
||||
message: "boom".into(),
|
||||
retryable: true,
|
||||
},
|
||||
AgentError::ContextLimitExceeded {
|
||||
utilization_pct: 98,
|
||||
},
|
||||
AgentError::ToolExecutionError {
|
||||
tool_name: "shell".into(),
|
||||
message: "denied".into(),
|
||||
},
|
||||
AgentError::CostBudgetExceeded {
|
||||
spent_microdollars: 1_000,
|
||||
budget_microdollars: 500,
|
||||
},
|
||||
AgentError::CompactionFailed {
|
||||
message: "summary failed".into(),
|
||||
consecutive_failures: 2,
|
||||
},
|
||||
AgentError::PermissionDenied {
|
||||
tool_name: "shell".into(),
|
||||
required_level: "Execute".into(),
|
||||
channel_max_level: "ReadOnly".into(),
|
||||
},
|
||||
AgentError::Other(anyhow::anyhow!("plain failure")),
|
||||
];
|
||||
for err in real_failures {
|
||||
assert!(!err.skips_sentry(), "must NOT skip Sentry for: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -391,6 +391,7 @@ impl Agent {
|
||||
Some(AgentError::ToolExecutionError { .. }) => Some("tool_execution_error"),
|
||||
Some(AgentError::CostBudgetExceeded { .. }) => Some("cost_budget_exceeded"),
|
||||
Some(AgentError::MaxIterationsExceeded { .. }) => Some("max_iterations_exceeded"),
|
||||
Some(AgentError::EmptyProviderResponse { .. }) => Some("empty_provider_response"),
|
||||
Some(AgentError::CompactionFailed { .. }) => Some("compaction_failed"),
|
||||
Some(AgentError::PermissionDenied { .. }) => Some("permission_denied"),
|
||||
Some(AgentError::Other(_)) | None => None,
|
||||
@@ -519,12 +520,16 @@ impl Agent {
|
||||
}
|
||||
Err(err) => {
|
||||
let sanitized_message = Self::sanitize_event_error_message(&err);
|
||||
// The max-tool-iterations cap is a deterministic agent-state
|
||||
// outcome, not a bug — the UI already surfaces the failure
|
||||
// to the user via the chat-rendered "Error: Agent exceeded
|
||||
// maximum tool iterations" message. Skip the Sentry funnel
|
||||
// for this variant entirely and emit a structured
|
||||
// `log::info!` (OPENHUMAN-TAURI-99 / -98).
|
||||
// Some typed `AgentError` variants represent agent / user /
|
||||
// provider state that the UI already surfaces — the
|
||||
// max-tool-iterations cap (OPENHUMAN-TAURI-99 / -98,
|
||||
// chat-rendered "Error: Agent exceeded maximum tool
|
||||
// iterations") and the empty-provider-response degeneracy
|
||||
// (TAURI-RUST-4JX, "The model returned an empty response.
|
||||
// Please try again."). Skip the Sentry funnel for both
|
||||
// and emit a structured `log::info!` instead. The
|
||||
// suppressed set is owned by `AgentError::skips_sentry()`
|
||||
// so the policy stays in one place.
|
||||
//
|
||||
// Other agent errors go through `report_error_or_expected`
|
||||
// so OPENHUMAN-TAURI-5Z and the budget-noise cluster —
|
||||
@@ -534,14 +539,13 @@ impl Agent {
|
||||
// warn/info-level breadcrumb without losing genuine bugs.
|
||||
// `Err` propagation, the `AgentError` domain event, and
|
||||
// downstream `recoverable=false` semantics are preserved.
|
||||
let is_max_iter = matches!(
|
||||
err.downcast_ref::<AgentError>(),
|
||||
Some(AgentError::MaxIterationsExceeded { .. })
|
||||
);
|
||||
if is_max_iter {
|
||||
let skips_sentry = err
|
||||
.downcast_ref::<AgentError>()
|
||||
.is_some_and(AgentError::skips_sentry);
|
||||
if skips_sentry {
|
||||
log::info!(
|
||||
target: "agent",
|
||||
"[agent.run_single] suppressed Sentry emission for max-iteration cap \
|
||||
"[agent.run_single] suppressed Sentry emission for user-state agent error \
|
||||
session_id={} channel={} error_kind={} message={}",
|
||||
self.event_session_id(),
|
||||
self.event_channel(),
|
||||
|
||||
@@ -21,6 +21,7 @@ use super::transcript;
|
||||
use super::types::Agent;
|
||||
use crate::core::event_bus::{publish_global, DomainEvent};
|
||||
use crate::openhuman::agent::dispatcher::{ParsedToolCall, ToolExecutionResult};
|
||||
use crate::openhuman::agent::error::AgentError;
|
||||
use crate::openhuman::agent::harness;
|
||||
use crate::openhuman::agent::hooks::{self, ToolCallRecord, TurnContext};
|
||||
use crate::openhuman::agent::memory_loader::collect_recall_citations;
|
||||
@@ -811,9 +812,17 @@ impl Agent {
|
||||
"[agent_loop] provider returned an empty final response (i={}, no text, no tool calls) — surfacing as error instead of a silent blank reply",
|
||||
iteration + 1
|
||||
);
|
||||
return Err(anyhow::anyhow!(
|
||||
"The model returned an empty response. Please try again."
|
||||
));
|
||||
// Typed variant so `run_single` can route this
|
||||
// through `AgentError::skips_sentry()` and demote
|
||||
// to a `log::info!` instead of escalating to
|
||||
// Sentry (TAURI-RUST-4JX). The `Display` impl
|
||||
// still renders the canonical user-facing string
|
||||
// for UI surfaces, so the user behaviour is
|
||||
// unchanged.
|
||||
return Err(AgentError::EmptyProviderResponse {
|
||||
iteration: iteration + 1,
|
||||
}
|
||||
.into());
|
||||
}
|
||||
log::info!(
|
||||
"[agent] no tool calls — returning final response after {} iteration(s)",
|
||||
|
||||
@@ -51,6 +51,9 @@ fn agent_error_to_user_message(err: &AgentError) -> &'static str {
|
||||
AgentError::MaxIterationsExceeded { .. } => {
|
||||
"The agent stopped after too many tool iterations. Raise the iteration cap in Settings \u{2192} AI \u{2192} LLM or simplify the task."
|
||||
}
|
||||
AgentError::EmptyProviderResponse { .. } => {
|
||||
"The model returned an empty response. Try a different model or check your local provider in Settings \u{2192} AI \u{2192} LLM."
|
||||
}
|
||||
AgentError::CompactionFailed { .. } => {
|
||||
"Automatic history compaction failed. The next run will start with a fresh context."
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user