diff --git a/src/openhuman/channels/providers/web_errors.rs b/src/openhuman/channels/providers/web_errors.rs index 5a9bd0bc7..fa2e61b68 100644 --- a/src/openhuman/channels/providers/web_errors.rs +++ b/src/openhuman/channels/providers/web_errors.rs @@ -579,10 +579,30 @@ pub(crate) fn classify_inference_error(err: &str) -> ClassifiedError { // neither can be shadowed by the broad provider-429 / 5xx arms below. // No `with_provider_detail` — an empty response carries no JSON body // to quote. + // + // Issue #3335: the prior copy ("Try a different model or check your + // local provider in Settings → AI → LLM") sent Managed users in + // exactly the wrong direction — there is no local provider on the + // Managed route, and the common underlying cause is credit + // exhaustion (see #3386). The provider name is not available here + // because `EmptyProviderResponse`'s flattened Display carries no + // `" API error"` infix for `extract_provider_name` to anchor on, + // and routing the typed provider through every layer is out of + // scope for this fix. So the copy is rewritten to be accurate for + // ALL providers: it names the three real remedies (credits, model + // health, configuration) without claiming any single one of them + // applies, and lets the user pick which is relevant to their + // setup. The companion empty-2xx-stream diagnostic in + // `compatible.rs::stream_native_chat` records elapsed_ms, + // chunk_count, and has_usage so the next iteration of this arm + // can be provider-aware once we have evidence on which path is + // dominant in production. ClassifiedError { error_type: "empty_response", - message: "The model returned an empty response. Try a different model or check \ - your local provider in Settings → AI → LLM." + message: "The model returned an empty response. This usually means your inference \ + credits are exhausted (Settings → Billing), the upstream model is temporarily \ + unhealthy, or your provider configuration is rejecting the request \ + (Settings → AI → LLM). Try one of those, or pick a different model." .to_string(), source: "agent_loop", retryable: true, diff --git a/src/openhuman/channels/providers/web_tests.rs b/src/openhuman/channels/providers/web_tests.rs index 34310f32d..5cbdeedfc 100644 --- a/src/openhuman/channels/providers/web_tests.rs +++ b/src/openhuman/channels/providers/web_tests.rs @@ -971,6 +971,60 @@ fn classify_inference_error_empty_response_is_actionable_and_retryable() { ); } +#[test] +fn classify_inference_error_empty_response_copy_names_billing_remedy_and_drops_local_provider_misdirect( +) { + // Issue #3335: the prior copy ("Try a different model or check your + // local provider in Settings → AI → LLM") sent Managed-route users + // toward a remedy that does not exist for them. The common underlying + // cause is credit exhaustion (issue #3386), so the revised copy must + // name the credits / billing path explicitly, must NOT claim a "local + // provider" exists, and must still offer the model-switch path for + // users on self-hosted providers. + let raw = "run_chat_task failed client_id=abc thread_id=t-1 request_id=r-1 \ + error=The model returned an empty response. Please try again."; + let classified = classify_inference_error(raw); + assert_eq!(classified.error_type, "empty_response"); + // New: names the credits / billing remedy (was absent in the old copy, + // so Managed users had no way to self-diagnose credit exhaustion). + assert!( + classified.message.contains("Settings → Billing"), + "must point at the billing surface for credit exhaustion: {}", + classified.message + ); + // New: drops the misleading "local provider" framing — the previous + // copy made a false claim for Managed users where no local provider + // exists. + assert!( + !classified.message.contains("local provider"), + "must not claim a local provider exists: {}", + classified.message + ); + // Preserved: the model-switch remedy and the provider-config + // settings deep link both still apply (some users hit empty response + // because their custom OpenAI-compatible endpoint or local model is + // misconfigured / unhealthy). + assert!( + classified.message.contains("different model"), + "must keep the model-switch remedy: {}", + classified.message + ); + assert!( + classified.message.contains("Settings → AI → LLM"), + "must keep the provider-config deep link: {}", + classified.message + ); + // Preserved: provider is intentionally None until the typed + // `AgentError::EmptyProviderResponse` plumbs through a provider + // identifier (see comment in `web_errors.rs::classify_inference_error` + // empty_response arm). + assert!( + classified.provider.is_none(), + "provider stays None until plumbed through the typed error: {:?}", + classified.provider + ); +} + #[test] fn classify_inference_error_vision_capability_is_non_retryable() { // A multimodal turn sent an image to a text-only model. Retrying the diff --git a/src/openhuman/cron/scheduler.rs b/src/openhuman/cron/scheduler.rs index 6dac36d14..bcb00ae5a 100644 --- a/src/openhuman/cron/scheduler.rs +++ b/src/openhuman/cron/scheduler.rs @@ -57,7 +57,16 @@ fn agent_error_to_user_message(err: &AgentError) -> &'static str { "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." + // Issue #3335: the prior copy named a "local provider" + // remedy that doesn't exist on the Managed route. This + // shorter form (≤120 chars per the + // `agent_error_to_user_message_canned_strings_are_short` + // contract, for clean notification-drawer rendering) names + // the two highest-signal remedies — credits and model + // configuration. The richer three-remedy copy lives on the + // chat-surface side (`channels/providers/web_errors.rs`'s + // empty_response arm) where there's no drawer-width limit. + "Empty model response. Out of credits (Settings \u{2192} Billing) or try a different model in Settings \u{2192} AI \u{2192} LLM." } AgentError::CompactionFailed { .. } => { "Automatic history compaction failed. The next run will start with a fresh context." diff --git a/src/openhuman/cron/scheduler_tests.rs b/src/openhuman/cron/scheduler_tests.rs index ee2a779f3..9c494fa2d 100644 --- a/src/openhuman/cron/scheduler_tests.rs +++ b/src/openhuman/cron/scheduler_tests.rs @@ -846,6 +846,33 @@ fn agent_error_to_user_message_classifies_max_iterations() { assert_ne!(msg, AGENT_JOB_USER_FAILURE_MESSAGE); } +#[test] +fn agent_error_to_user_message_classifies_empty_provider_response_for_3335() { + // Issue #3335: the cron-path copy must stay in lock-step with the + // web-channel `empty_response` arm — names the credits / billing + // remedy explicitly and drops the misleading "local provider" + // misdirect that broke remediation for Managed users. + let err = AgentError::EmptyProviderResponse { iteration: 1 }; + let msg = agent_error_to_user_message(&err); + assert!( + msg.contains("Settings \u{2192} Billing"), + "must point at billing for credit exhaustion: {msg}" + ); + assert!( + !msg.contains("local provider"), + "must not claim a local provider exists: {msg}" + ); + assert!( + msg.contains("different model"), + "must keep the model-switch remedy: {msg}" + ); + assert!( + msg.contains("Settings \u{2192} AI \u{2192} LLM"), + "must keep the provider-config deep link: {msg}" + ); + assert_ne!(msg, AGENT_JOB_USER_FAILURE_MESSAGE); +} + #[test] fn agent_error_to_user_message_classifies_compaction_failed() { let err = AgentError::CompactionFailed { @@ -921,6 +948,13 @@ fn agent_error_to_user_message_canned_strings_are_short() { required_level: "x".into(), channel_max_level: "x".into(), }, + // Issue #3335: EmptyProviderResponse was historically absent from + // this variants list — its old copy happened to fit, but nothing + // enforced it. The fix shipped a new copy that explicitly names + // the credits / billing remedy, which makes the length tradeoff + // active rather than incidental. Lock it in so a future copy + // change can't quietly grow past the drawer-render budget. + AgentError::EmptyProviderResponse { iteration: 0 }, ]; for v in &variants { let msg = agent_error_to_user_message(v); diff --git a/src/openhuman/inference/provider/compatible_stream_native.rs b/src/openhuman/inference/provider/compatible_stream_native.rs index 8310182d1..062140531 100644 --- a/src/openhuman/inference/provider/compatible_stream_native.rs +++ b/src/openhuman/inference/provider/compatible_stream_native.rs @@ -32,6 +32,11 @@ impl OpenAiCompatibleProvider { native_request.tools.as_ref().map_or(0, |t| t.len()), ); + // Captured at request send so the empty-2xx-stream diagnostic + // below can report elapsed_ms — a fast empty stream points at a + // backend reject, a slow one at an upstream stall / timeout. + let stream_started_at = std::time::Instant::now(); + let response = self .apply_auth_header( self.http_client() @@ -157,9 +162,45 @@ impl OpenAiCompatibleProvider { self.name, ); let response_bytes = response.bytes().await?; + let body_bytes_received = response_bytes.len(); dump_response_if_enabled(&self.name, &native_request.model, dump_seq, &response_bytes); let api_resp: ApiChatResponse = serde_json::from_slice(&response_bytes) .map_err(|err| anyhow::anyhow!("{} response parse error: {err}", self.name))?; + + // Mirror the SSE-branch empty-2xx-stream diagnostic (#3335 / + // #3386) on the buffered JSON path. The same upstream + // collapse to `AgentError::EmptyProviderResponse` is + // reachable here when a managed backend returns 200 with a + // content-less JSON payload (credit exhaustion served as + // JSON instead of SSE, or an upstream stall flushed as an + // empty completion). Without this sibling guard the warn + // would only fire on the SSE branch and the buffered case + // would silently miss the very signal we're trying to + // capture. + let buffered_is_empty = api_resp + .choices + .first() + .map(|c| { + let m = &c.message; + let content_empty = m.content.as_deref().is_none_or(str::is_empty); + let reasoning_empty = m.reasoning_content.as_deref().is_none_or(str::is_empty); + let tool_calls_empty = m.tool_calls.as_ref().is_none_or(|t| t.is_empty()); + let function_call_empty = m.function_call.is_none(); + content_empty && reasoning_empty && tool_calls_empty && function_call_empty + }) + .unwrap_or(true); + if buffered_is_empty { + let elapsed_ms = stream_started_at.elapsed().as_millis() as u64; + log::warn!( + "[stream] {} empty 2xx buffered JSON — model={} elapsed_ms={} body_bytes={} has_usage={} has_openhuman_meta={}", + self.name, + native_request.model, + elapsed_ms, + body_bytes_received, + api_resp.usage.is_some(), + api_resp.openhuman.is_some(), + ); + } return Self::parse_native_response(api_resp, &self.name); } @@ -174,9 +215,23 @@ impl OpenAiCompatibleProvider { let mut buffer = String::new(); let mut repeat_detector = StreamRepeatDetector::new(); let mut degenerate_repeat = false; + // Forensic counters for the empty-2xx-stream diagnostic below + // (issue #3335 / #3386). Both are append-only, never read by + // request path logic — strictly observability. + // + // `body_bytes_received` is the count of body bytes yielded by + // `bytes_stream()`. This crate builds reqwest without the + // `gzip` / `brotli` / `zstd` / `deflate` features (see + // root Cargo.toml), so no Content-Encoding decompression happens + // and the count matches what's on the wire. The neutral name + // (rather than `raw_bytes` / `decoded_bytes`) sidesteps the + // ambiguity an operator would otherwise hit reading the log. + let mut sse_chunks_parsed: usize = 0; + let mut body_bytes_received: usize = 0; 'stream: while let Some(item) = bytes_stream.next().await { let bytes = item?; + body_bytes_received += bytes.len(); buffer.push_str(&String::from_utf8_lossy(&bytes)); while let Some(sep_idx) = buffer.find("\n\n") { @@ -214,7 +269,10 @@ impl OpenAiCompatibleProvider { } let chunk: StreamChunkResponse = match serde_json::from_str(data) { - Ok(v) => v, + Ok(v) => { + sse_chunks_parsed += 1; + v + } Err(e) => { log::debug!( "[stream] {} skipping unparseable chunk: {} — data={}", @@ -413,6 +471,42 @@ impl OpenAiCompatibleProvider { tool_call_count, ); + // Issue #3335 / #3386 forensic signal. The streaming chat call + // completed with HTTP 2xx but delivered zero visible text, zero + // thinking, and zero tool calls. This is the upstream shape that + // collapses to `AgentError::EmptyProviderResponse` and gets + // rendered to the user as "The model returned an empty + // response" with the wrong remediation. Most likely causes: + // (a) backend closed the SSE cleanly under credit exhaustion + // (no `[stream] streaming API error` breadcrumb fires + // because status was 200) — common on the OpenHuman + // managed route under #3386, + // (b) backend's upstream LLM provider stalled / timed out + // and the backend forwarded an empty stream instead of + // propagating the upstream error, + // (c) a genuine degenerate model output (rare on hosted + // reasoning models; more common on community quants). + // Logged at warn so it lands in Sentry breadcrumbs even after + // `AgentError::skips_sentry()` (PR #2790) silences the parent + // event. Correlate by elapsed_ms (fast == reject, slow == + // stall), sse_chunks (0 == no SSE at all, >0 == backend + // streamed metadata-only chunks), has_usage (the upstream + // counted tokens but delivered no content), and + // has_openhuman_meta (managed backend reported routing info). + if text_accum.is_empty() && thinking_accum.is_empty() && tool_call_count == 0 { + let elapsed_ms = stream_started_at.elapsed().as_millis() as u64; + log::warn!( + "[stream] {} empty 2xx stream — model={} elapsed_ms={} sse_chunks={} body_bytes={} has_usage={} has_openhuman_meta={}", + self.name, + native_request.model, + elapsed_ms, + sse_chunks_parsed, + body_bytes_received, + last_usage.is_some(), + last_openhuman.is_some(), + ); + } + let tool_calls_for_api: Vec = tool_accum .into_values() .map(|c| ToolCall {