mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
fix(cron): stop cron billing-state Sentry floods — 402 credits + 400 budget (TAURI-RUST-514 / -BMW) (#3913)
This commit is contained in:
oxoxDev
@@ -2223,6 +2223,25 @@ pub fn run() {
|
||||
);
|
||||
return None;
|
||||
}
|
||||
// Drop provider insufficient-credits 402s — the user's own BYO
|
||||
// account (e.g. OpenRouter) is out of balance, a billing state
|
||||
// OpenHuman has no lever over once the request already caps
|
||||
// max_tokens. The core binary's main.rs before_send already
|
||||
// filters these; since #1061 the core runs in-process inside this
|
||||
// shell, so the cron `agent_job` retries-exhausted report (and any
|
||||
// other compatible-provider path) lands in THIS Sentry client and
|
||||
// must be filtered identically. Closes the #3617 drift that wired
|
||||
// the filter only into the standalone-CLI chain (TAURI-RUST-514 /
|
||||
// -C62).
|
||||
if openhuman_core::core::observability::is_insufficient_credits_event(&event) {
|
||||
// Metadata-only log shape — `event.message` carries the raw
|
||||
// provider 402 body which CLAUDE.md forbids from local logs.
|
||||
log::debug!(
|
||||
"[sentry-insufficient-credits-filter] dropping insufficient-credits 402 event_id={:?}",
|
||||
event.event_id
|
||||
);
|
||||
return None;
|
||||
}
|
||||
// Strip server_name (hostname) to avoid leaking machine identity.
|
||||
event.server_name = None;
|
||||
// Attach the cached account uid so Sentry can count unique users
|
||||
|
||||
+96
-14
@@ -2233,6 +2233,43 @@ pub fn is_budget_event(event: &sentry::protocol::Event<'_>) -> bool {
|
||||
event_contains_budget_exhausted_message(event)
|
||||
}
|
||||
|
||||
/// Whether a raw error / message string is a provider **insufficient-credits
|
||||
/// 402** — the BYO account (e.g. OpenRouter) genuinely lacks the balance to
|
||||
/// satisfy the request. Anchored on BOTH a 402-status shape AND a credit
|
||||
/// phrase, so a bare `402` (or a non-402 error whose body merely contains the
|
||||
/// digits `402`) is not swallowed and keeps reaching Sentry.
|
||||
///
|
||||
/// Single source of truth shared by the message-level cron halt
|
||||
/// (`openhuman::cron::scheduler`'s `is_insufficient_credits_failure`, which
|
||||
/// stops retrying a permanent 402 and skips its `report_error`) and the
|
||||
/// event-level `before_send` filter [`is_insufficient_credits_event`] — the
|
||||
/// same split as [`is_session_expired_message`] ↔ [`is_session_expired_event`].
|
||||
/// TAURI-RUST-514 / -C62.
|
||||
pub fn is_insufficient_credits_message(text: &str) -> bool {
|
||||
let lower = text.to_ascii_lowercase();
|
||||
// Anchor the 402 to a status shape — the emit sites format the message
|
||||
// as "<provider> API error (402 Payment Required): <body>". Matching a
|
||||
// bare "402" would false-positive on body digits (e.g. a 400 error
|
||||
// whose body says "can only afford 402 tokens"), which is NOT this
|
||||
// user-state and must keep reaching Sentry.
|
||||
let is_402_status = lower.contains("(402") || lower.contains("402 payment required");
|
||||
if !is_402_status {
|
||||
return false;
|
||||
}
|
||||
// Check the credit/balance signal against the BODY only. The status prefix
|
||||
// "(402 Payment Required)" itself contains the phrase "payment required",
|
||||
// which `body_indicates_insufficient_credits` matches — so feeding it the
|
||||
// whole string would classify ANY 402 (even one whose body is an unrelated
|
||||
// condition) as insufficient-credits and suppress it (codex P2 on #3913).
|
||||
// Slice off everything up to and including the formatted "): " status
|
||||
// separator first; fall back to the whole text when the separator is
|
||||
// absent (non-standard shape) so a credit phrase there still matches.
|
||||
let body = lower
|
||||
.split_once("): ")
|
||||
.map_or(lower.as_str(), |(_, body)| body);
|
||||
crate::openhuman::inference::provider::body_indicates_insufficient_credits(body)
|
||||
}
|
||||
|
||||
/// Defense-in-depth `before_send` filter for **insufficient-credits 402**
|
||||
/// provider events (TAURI-RUST-C62): the user's own BYO provider account
|
||||
/// (e.g. OpenRouter) is out of balance — a billing state OpenHuman has no
|
||||
@@ -2252,22 +2289,10 @@ pub fn is_budget_event(event: &sentry::protocol::Event<'_>) -> bool {
|
||||
/// - that same text carries an insufficient-credits phrase
|
||||
/// (`provider::body_indicates_insufficient_credits`).
|
||||
pub fn is_insufficient_credits_event(event: &sentry::protocol::Event<'_>) -> bool {
|
||||
fn text_is_insufficient_credits_402(text: &str) -> bool {
|
||||
let lower = text.to_ascii_lowercase();
|
||||
// Anchor the 402 to a status shape — the emit sites format the message
|
||||
// as "<provider> API error (402 Payment Required): <body>". Matching a
|
||||
// bare "402" would false-positive on body digits (e.g. a 400 error
|
||||
// whose body says "can only afford 402 tokens"), which is NOT this
|
||||
// user-state and must keep reaching Sentry.
|
||||
let is_402_status = lower.contains("(402") || lower.contains("402 payment required");
|
||||
is_402_status
|
||||
&& crate::openhuman::inference::provider::body_indicates_insufficient_credits(text)
|
||||
}
|
||||
|
||||
if event
|
||||
.message
|
||||
.as_deref()
|
||||
.is_some_and(text_is_insufficient_credits_402)
|
||||
.is_some_and(is_insufficient_credits_message)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
@@ -2275,7 +2300,7 @@ pub fn is_insufficient_credits_event(event: &sentry::protocol::Event<'_>) -> boo
|
||||
exception
|
||||
.value
|
||||
.as_deref()
|
||||
.is_some_and(text_is_insufficient_credits_402)
|
||||
.is_some_and(is_insufficient_credits_message)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -5345,6 +5370,63 @@ mod tests {
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_insufficient_credits_message_matches_verbatim_cron_402() {
|
||||
// Verbatim TAURI-RUST-514 body as it reaches the cron `report_error`
|
||||
// call site (`domain=cron`, `operation=agent_job`): the message-level
|
||||
// matcher must catch it so the cron halt skips the leaking report.
|
||||
assert!(is_insufficient_credits_message(
|
||||
"openrouter 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 5081.\"}}",
|
||||
));
|
||||
assert!(is_insufficient_credits_message(
|
||||
"custom_openai API error (402 Payment Required): insufficient balance",
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_insufficient_credits_message_requires_402_and_credit_phrase() {
|
||||
// A 402 without a credit phrase, and a credit phrase without a 402
|
||||
// status, must both stay reportable (could be a real defect).
|
||||
assert!(!is_insufficient_credits_message(
|
||||
"provider API error (402): some unrelated condition"
|
||||
));
|
||||
assert!(!is_insufficient_credits_message(
|
||||
"provider API error (500): internal error, insufficient memory"
|
||||
));
|
||||
// The status must be the 402, not a digit in the body.
|
||||
assert!(!is_insufficient_credits_message(
|
||||
"provider API error (400): can only afford 402 tokens"
|
||||
));
|
||||
// codex P2: the status prefix "(402 Payment Required)" itself contains
|
||||
// the phrase "payment required". An unrelated body behind a real 402
|
||||
// must NOT be classified as insufficient-credits (else the cron halt
|
||||
// would suppress a genuine 402 defect). The credit signal must live in
|
||||
// the BODY, not the formatted status prefix.
|
||||
assert!(!is_insufficient_credits_message(
|
||||
"provider API error (402 Payment Required): some unrelated condition"
|
||||
));
|
||||
// But a body that literally carries the credit signal still matches,
|
||||
// even when the status prefix also says "Payment Required".
|
||||
assert!(is_insufficient_credits_message(
|
||||
"provider API error (402 Payment Required): your account has insufficient balance"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_insufficient_credits_event_delegates_to_message_matcher() {
|
||||
// Parity: the event-level filter is now a thin wrapper over the
|
||||
// message-level matcher across both the message and exception paths.
|
||||
let body = "myopenrouter API error (402 Payment Required): This request requires \
|
||||
more credits, or fewer max_tokens.";
|
||||
assert!(is_insufficient_credits_message(body));
|
||||
assert!(is_insufficient_credits_event(&event_with_message(body)));
|
||||
assert!(is_insufficient_credits_event(&event_with_exception_value(
|
||||
body
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_expired_before_send_matches_core_401_events() {
|
||||
let msg = "SESSION_EXPIRED: backend session not active — sign in to resume LLM work";
|
||||
|
||||
@@ -337,6 +337,61 @@ fn is_session_expired_failure(
|
||||
crate::core::observability::is_session_expired_message(signal)
|
||||
}
|
||||
|
||||
/// Did this failed agent-job attempt hit a provider **insufficient-credits
|
||||
/// 402** state (BYO account out of balance, e.g. OpenRouter)?
|
||||
///
|
||||
/// Same shape as [`is_session_expired_failure`], for the same reason: the
|
||||
/// condition is a deterministic, permanent user-state error with no local
|
||||
/// lever — retrying it across the backoff loop cannot recover, it only burns
|
||||
/// cycles and (pre-this-guard) multiplied the per-attempt
|
||||
/// `report_error` events that flooded Sentry (TAURI-RUST-514: the residual
|
||||
/// after #3617 capped the extraction path, surfacing here via the cron
|
||||
/// `agent_job` `report_error` which the desktop `before_send` chain did not
|
||||
/// yet filter). So we halt after the first occurrence and skip the report,
|
||||
/// matching the source demotion already applied at the provider emit site
|
||||
/// (`is_provider_insufficient_credits_402`).
|
||||
///
|
||||
/// Routes on `last_agent_error` first (the raw anyhow chain carrying the
|
||||
/// provider's 402 wire body), falling back to `last_output`, identical to
|
||||
/// [`is_session_expired_failure`]. Restricted to `JobType::Agent`.
|
||||
fn is_insufficient_credits_failure(
|
||||
job_type: &JobType,
|
||||
last_agent_error: Option<&str>,
|
||||
last_output: &str,
|
||||
) -> bool {
|
||||
if !matches!(job_type, JobType::Agent) {
|
||||
return false;
|
||||
}
|
||||
let signal = last_agent_error.unwrap_or(last_output);
|
||||
crate::core::observability::is_insufficient_credits_message(signal)
|
||||
}
|
||||
|
||||
/// Did this failed agent-job attempt hit a managed-backend **budget-exhausted
|
||||
/// 400** state (`USER_INSUFFICIENT_CREDITS` — the OpenHuman account is out of
|
||||
/// its spend budget)?
|
||||
///
|
||||
/// The sibling of [`is_insufficient_credits_failure`] for the managed-backend
|
||||
/// billing 400 instead of the BYO provider 402. Same rationale: a permanent
|
||||
/// user-state error with no local lever, so retrying across the backoff loop
|
||||
/// cannot recover and the per-attempt `report_error` floods Sentry
|
||||
/// (TAURI-RUST-BMW). The existing `before_send` filter
|
||||
/// [`crate::core::observability::is_budget_event`] is **tag-gated**
|
||||
/// (`failure=non_2xx` + `status=400`), tags the cron `agent_job` re-report
|
||||
/// does not carry — so the residual leaks here. Halt on the first occurrence
|
||||
/// and skip the report, reusing the same body classifier as that filter
|
||||
/// (`provider::is_budget_exhausted_message`). Restricted to `JobType::Agent`.
|
||||
fn is_budget_exhausted_failure(
|
||||
job_type: &JobType,
|
||||
last_agent_error: Option<&str>,
|
||||
last_output: &str,
|
||||
) -> bool {
|
||||
if !matches!(job_type, JobType::Agent) {
|
||||
return false;
|
||||
}
|
||||
let signal = last_agent_error.unwrap_or(last_output);
|
||||
crate::openhuman::inference::provider::is_budget_exhausted_message(signal)
|
||||
}
|
||||
|
||||
async fn execute_job_with_retry(
|
||||
config: &Config,
|
||||
security: &SecurityPolicy,
|
||||
@@ -347,6 +402,8 @@ async fn execute_job_with_retry(
|
||||
let retries = config.reliability.scheduler_retries;
|
||||
let mut backoff_ms = config.reliability.provider_backoff_ms.max(200);
|
||||
let mut session_expired = false;
|
||||
let mut credits_exhausted = false;
|
||||
let mut budget_exhausted = false;
|
||||
|
||||
for attempt in 0..=retries {
|
||||
let (success, output, agent_error) = match job.job_type {
|
||||
@@ -384,6 +441,49 @@ async fn execute_job_with_retry(
|
||||
break;
|
||||
}
|
||||
|
||||
if is_insufficient_credits_failure(
|
||||
&job.job_type,
|
||||
last_agent_error.as_deref(),
|
||||
last_output.as_str(),
|
||||
) {
|
||||
// Halt on the first occurrence — a BYO provider 402 (out of
|
||||
// balance) is permanent across the backoff loop, and the
|
||||
// provider emit site already demoted it from Sentry. Skipping
|
||||
// the retries-exhausted `report_error` below keeps the residual
|
||||
// off Sentry at source, independent of the `before_send` chain
|
||||
// (TAURI-RUST-514). See `is_insufficient_credits_failure`.
|
||||
// Metadata-only log (no raw provider body — see CLAUDE.md).
|
||||
log::debug!(
|
||||
"[cron] action=halt_on_insufficient_credits_402 job_id={} attempt={} retries={}",
|
||||
job.id.as_str(),
|
||||
attempt,
|
||||
retries
|
||||
);
|
||||
credits_exhausted = true;
|
||||
break;
|
||||
}
|
||||
|
||||
if is_budget_exhausted_failure(
|
||||
&job.job_type,
|
||||
last_agent_error.as_deref(),
|
||||
last_output.as_str(),
|
||||
) {
|
||||
// Halt on the first occurrence — a managed-backend budget 400
|
||||
// (USER_INSUFFICIENT_CREDITS) is permanent across the backoff
|
||||
// loop. The tag-gated `is_budget_event` before_send filter never
|
||||
// matches this cron re-report, so suppressing the report here
|
||||
// keeps it off Sentry at source (TAURI-RUST-BMW). See
|
||||
// `is_budget_exhausted_failure`. Metadata-only log (no raw body).
|
||||
log::debug!(
|
||||
"[cron] action=halt_on_budget_exhausted_400 job_id={} attempt={} retries={}",
|
||||
job.id.as_str(),
|
||||
attempt,
|
||||
retries
|
||||
);
|
||||
budget_exhausted = true;
|
||||
break;
|
||||
}
|
||||
|
||||
if attempt < retries {
|
||||
let jitter_ms = u64::from(Utc::now().timestamp_subsec_millis() % 250);
|
||||
time::sleep(Duration::from_millis(backoff_ms + jitter_ms)).await;
|
||||
@@ -391,7 +491,12 @@ async fn execute_job_with_retry(
|
||||
}
|
||||
}
|
||||
|
||||
if matches!(job.job_type, JobType::Agent) && !session_expired {
|
||||
// Permanent billing user-states (BYO 402 out-of-credit / managed-backend
|
||||
// 400 out-of-budget) are demoted at source: halt the loop and skip the
|
||||
// retries-exhausted report, independent of the tag-gated before_send
|
||||
// filters that the cron re-report does not match (TAURI-RUST-514 / -BMW).
|
||||
let billing_halt = credits_exhausted || budget_exhausted;
|
||||
if matches!(job.job_type, JobType::Agent) && !session_expired && !billing_halt {
|
||||
let report_message = last_agent_error
|
||||
.as_deref()
|
||||
.unwrap_or_else(|| last_output.as_str());
|
||||
@@ -409,6 +514,20 @@ async fn execute_job_with_retry(
|
||||
("failure", "retries_exhausted"),
|
||||
],
|
||||
);
|
||||
} else if matches!(job.job_type, JobType::Agent) && billing_halt {
|
||||
// Suppressed the retries-exhausted Sentry report for a permanent
|
||||
// billing user-state. Metadata-only breadcrumb so the suppression is
|
||||
// diagnosable in production without the raw provider body.
|
||||
let reason = if credits_exhausted {
|
||||
"insufficient_credits_402"
|
||||
} else {
|
||||
"budget_exhausted_400"
|
||||
};
|
||||
log::debug!(
|
||||
"[cron] action=suppress_retries_exhausted_report reason={reason} job_id={} retries={}",
|
||||
job.id.as_str(),
|
||||
retries
|
||||
);
|
||||
}
|
||||
|
||||
(false, last_output)
|
||||
|
||||
@@ -383,6 +383,130 @@ fn is_session_expired_failure_does_not_halt_shell_jobs() {
|
||||
);
|
||||
}
|
||||
|
||||
// TAURI-RUST-514 — a BYO provider insufficient-credits 402 ("requires more
|
||||
// credits") leaks from a cron-fired agent job through `last_agent_error`.
|
||||
// `is_insufficient_credits_failure` must consult the message classifier so the
|
||||
// retry loop halts on the first occurrence (a permanent billing state) instead
|
||||
// of retrying N times and reporting `failure=retries_exhausted` to Sentry.
|
||||
#[test]
|
||||
fn is_insufficient_credits_failure_matches_verbatim_402_in_agent_error() {
|
||||
let wire = r#"openrouter 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 5081."}}"#;
|
||||
assert!(
|
||||
is_insufficient_credits_failure(
|
||||
&JobType::Agent,
|
||||
Some(wire),
|
||||
AGENT_JOB_USER_FAILURE_MESSAGE
|
||||
),
|
||||
"raw agent error carrying the 402 credit body must trip the halt"
|
||||
);
|
||||
}
|
||||
|
||||
// Defense-in-depth: classify even if a future path surfaces the raw 402 in
|
||||
// `last_output` rather than `last_agent_error`.
|
||||
#[test]
|
||||
fn is_insufficient_credits_failure_matches_when_only_output_carries_signal() {
|
||||
let wire = r#"openrouter API error (402 Payment Required): insufficient balance — add credits"#;
|
||||
assert!(is_insufficient_credits_failure(&JobType::Agent, None, wire));
|
||||
}
|
||||
|
||||
// Negative guard: the canned user-facing message carries no 402 signal, and an
|
||||
// ordinary provider error (500, or a 400 whose body merely names a token
|
||||
// count) must NOT halt — those are exactly what the retry loop +
|
||||
// `failure=retries_exhausted` capture exist for.
|
||||
#[test]
|
||||
fn is_insufficient_credits_failure_does_not_match_non_credit_errors() {
|
||||
assert!(!is_insufficient_credits_failure(
|
||||
&JobType::Agent,
|
||||
Some(AGENT_JOB_USER_FAILURE_MESSAGE),
|
||||
AGENT_JOB_USER_FAILURE_MESSAGE,
|
||||
));
|
||||
let server_err =
|
||||
r#"OpenHuman API error (500 Internal Server Error): {"error":"Internal server error"}"#;
|
||||
assert!(!is_insufficient_credits_failure(
|
||||
&JobType::Agent,
|
||||
Some(server_err),
|
||||
""
|
||||
));
|
||||
let digit_in_body = r#"provider API error (400): can only afford 402 tokens"#;
|
||||
assert!(
|
||||
!is_insufficient_credits_failure(&JobType::Agent, Some(digit_in_body), ""),
|
||||
"the 402 must be the status, not an arbitrary token count in a 400 body"
|
||||
);
|
||||
}
|
||||
|
||||
// Scope guard: shell jobs that echo a 402-shaped string keep their retry
|
||||
// semantics — only agent jobs route through the inference layer.
|
||||
#[test]
|
||||
fn is_insufficient_credits_failure_does_not_halt_shell_jobs() {
|
||||
let wire = r#"openrouter API error (402 Payment Required): requires more credits"#;
|
||||
assert!(!is_insufficient_credits_failure(
|
||||
&JobType::Shell,
|
||||
None,
|
||||
wire
|
||||
));
|
||||
assert!(!is_insufficient_credits_failure(
|
||||
&JobType::Shell,
|
||||
Some(wire),
|
||||
wire
|
||||
));
|
||||
}
|
||||
|
||||
// TAURI-RUST-BMW — a managed-backend 400 "Insufficient budget"
|
||||
// (USER_INSUFFICIENT_CREDITS) leaks from a cron-fired agent job through
|
||||
// `last_agent_error`. `is_budget_exhausted_failure` must consult the budget
|
||||
// classifier so the retry loop halts on the first occurrence (a permanent
|
||||
// billing state) instead of retrying N times and reporting
|
||||
// `failure=retries_exhausted` to Sentry — the tag-gated `is_budget_event`
|
||||
// `before_send` filter never matched this cron re-report.
|
||||
#[test]
|
||||
fn is_budget_exhausted_failure_matches_verbatim_400_in_agent_error() {
|
||||
let wire = r#"OpenHuman API error (400 Bad Request): {"success":false,"error":"Insufficient budget","errorCode":"USER_INSUFFICIENT_CREDITS"}"#;
|
||||
assert!(
|
||||
is_budget_exhausted_failure(&JobType::Agent, Some(wire), AGENT_JOB_USER_FAILURE_MESSAGE),
|
||||
"raw agent error carrying the 400 budget body must trip the halt"
|
||||
);
|
||||
}
|
||||
|
||||
// Defense-in-depth: classify even if a future path surfaces the raw 400 in
|
||||
// `last_output` rather than `last_agent_error`.
|
||||
#[test]
|
||||
fn is_budget_exhausted_failure_matches_when_only_output_carries_signal() {
|
||||
let wire = r#"OpenHuman API error (400 Bad Request): budget exceeded — add credits"#;
|
||||
assert!(is_budget_exhausted_failure(&JobType::Agent, None, wire));
|
||||
}
|
||||
|
||||
// Negative guard: the canned user-facing message and an ordinary provider
|
||||
// error must NOT halt — those are what the retry loop +
|
||||
// `failure=retries_exhausted` capture exist for.
|
||||
#[test]
|
||||
fn is_budget_exhausted_failure_does_not_match_non_budget_errors() {
|
||||
assert!(!is_budget_exhausted_failure(
|
||||
&JobType::Agent,
|
||||
Some(AGENT_JOB_USER_FAILURE_MESSAGE),
|
||||
AGENT_JOB_USER_FAILURE_MESSAGE,
|
||||
));
|
||||
let server_err =
|
||||
r#"OpenHuman API error (500 Internal Server Error): {"error":"Internal server error"}"#;
|
||||
assert!(!is_budget_exhausted_failure(
|
||||
&JobType::Agent,
|
||||
Some(server_err),
|
||||
""
|
||||
));
|
||||
}
|
||||
|
||||
// Scope guard: shell jobs that echo a budget-shaped string keep their retry
|
||||
// semantics — only agent jobs route through the inference layer.
|
||||
#[test]
|
||||
fn is_budget_exhausted_failure_does_not_halt_shell_jobs() {
|
||||
let wire = r#"OpenHuman API error (400 Bad Request): {"error":"Insufficient budget"}"#;
|
||||
assert!(!is_budget_exhausted_failure(&JobType::Shell, None, wire));
|
||||
assert!(!is_budget_exhausted_failure(
|
||||
&JobType::Shell,
|
||||
Some(wire),
|
||||
wire
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn run_agent_job_returns_error_without_provider_key() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user