From 694ae4e677d95ebd9f788196ad7903a57c0de426 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Fri, 8 May 2026 00:58:41 +0530 Subject: [PATCH] fix(chat): sanitize agent/cron failures and add user-safe error fallback with Sentry reporting (#1332) --- app/src/providers/ChatRuntimeProvider.tsx | 9 +- .../__tests__/ChatRuntimeProvider.test.tsx | 84 +++++++++++++++++++ src/openhuman/channels/providers/web.rs | 44 +++++++++- src/openhuman/channels/providers/web_tests.rs | 73 +++++++++++++++- src/openhuman/cron/scheduler.rs | 48 ++++++++++- src/openhuman/cron/scheduler_tests.rs | 31 ++++++- 6 files changed, 272 insertions(+), 17 deletions(-) diff --git a/app/src/providers/ChatRuntimeProvider.tsx b/app/src/providers/ChatRuntimeProvider.tsx index 8c9167ac4..b9a5c304e 100644 --- a/app/src/providers/ChatRuntimeProvider.tsx +++ b/app/src/providers/ChatRuntimeProvider.tsx @@ -42,6 +42,8 @@ import { IS_PROD } from '../utils/config'; import { formatTimelineEntry, promptFromArgsBuffer } from '../utils/toolTimelineFormatting'; const logChatRuntime = debug('openhuman:chat-runtime'); +const USER_FACING_AGENT_ERROR_MESSAGE = + 'Something went wrong. Please try again.\nThis error has been reported. You can also report it on Discord.\nReport on Discord'; type SegmentDelivery = { segments: Map }; @@ -781,14 +783,11 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => { const threadMessages = currentState.thread.messagesByThreadId[event.thread_id] ?? []; const lastMsg = threadMessages[threadMessages.length - 1]; if ( - !( - lastMsg?.sender === 'agent' && - lastMsg?.content === 'Something went wrong — please try again.' - ) + !(lastMsg?.sender === 'agent' && lastMsg?.content === USER_FACING_AGENT_ERROR_MESSAGE) ) { void dispatch( addInferenceResponse({ - content: 'Something went wrong — please try again.', + content: USER_FACING_AGENT_ERROR_MESSAGE, threadId: event.thread_id, }) ); diff --git a/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx b/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx index 90cba448d..31a0a04f2 100644 --- a/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx +++ b/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx @@ -436,6 +436,90 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria expect(timeline[0]?.status).toBe('error'); expect(store.getState().chatRuntime.inferenceStatusByThread['t-err']).toBeUndefined(); }); + + it('adds a sanitized user-facing error bubble with Discord report action on chat_error', async () => { + const listeners = renderProvider(); + + act(() => { + listeners.onError?.({ + thread_id: 't-err-sanitized', + request_id: 'r1', + message: + 'agent job failed: error sending request for url (https://staging-api.alphahuman.xyz/openai/v1/chat/completions)', + error_type: 'inference', + round: 0, + }); + }); + + await waitFor(() => + expect(threadApi.appendMessage).toHaveBeenCalledWith( + 't-err-sanitized', + expect.objectContaining({ + sender: 'agent', + content: expect.stringContaining('Something went wrong. Please try again.'), + }) + ) + ); + expect(threadApi.appendMessage).toHaveBeenCalledWith( + 't-err-sanitized', + expect.objectContaining({ + content: expect.stringContaining( + 'Report on Discord' + ), + }) + ); + expect(threadApi.appendMessage).not.toHaveBeenCalledWith( + 't-err-sanitized', + expect.objectContaining({ + content: expect.stringContaining('https://staging-api.alphahuman.xyz'), + }) + ); + }); + + it('does not append duplicate fallback error bubble when the previous message already matches', async () => { + const listeners = renderProvider(); + + act(() => { + listeners.onError?.({ + thread_id: 't-err-dedupe', + request_id: 'r1', + message: 'transport fail one', + error_type: 'inference', + round: 0, + }); + }); + + await waitFor(() => + expect(threadApi.appendMessage).toHaveBeenCalledWith( + 't-err-dedupe', + expect.objectContaining({ + content: expect.stringContaining('Something went wrong. Please try again.'), + }) + ) + ); + + act(() => { + listeners.onError?.({ + thread_id: 't-err-dedupe', + request_id: 'r2', + message: 'transport fail two', + error_type: 'inference', + round: 0, + }); + }); + + await waitFor(() => { + const matchingCalls = vi + .mocked(threadApi.appendMessage) + .mock.calls.filter( + call => + call[0] === 't-err-dedupe' && + typeof call[1]?.content === 'string' && + call[1].content.includes('Something went wrong. Please try again.') + ); + expect(matchingCalls).toHaveLength(1); + }); + }); }); // Live subagent activity (#1122) — the parent thread surfaces a diff --git a/src/openhuman/channels/providers/web.rs b/src/openhuman/channels/providers/web.rs index 0e7c0ea11..10c0e2086 100644 --- a/src/openhuman/channels/providers/web.rs +++ b/src/openhuman/channels/providers/web.rs @@ -86,6 +86,9 @@ static THREAD_SESSIONS: Lazy>> = static IN_FLIGHT: Lazy>> = Lazy::new(|| Mutex::new(HashMap::new())); +#[cfg(test)] +static TEST_FORCED_RUN_CHAT_TASK_ERROR: Lazy>> = + Lazy::new(|| Mutex::new(None)); static BUDGET_ERROR_NORMALIZE_RE: Lazy = Lazy::new(|| Regex::new(r"[-_\s]+").expect("budget normalize regex")); static BUDGET_ERROR_PATTERNS: Lazy> = Lazy::new(|| { @@ -123,6 +126,10 @@ fn inference_budget_exceeded_user_message() -> &'static str { "I don't have any budget available right now. Please top up your credits or choose a plan to continue." } +fn generic_inference_error_user_message() -> &'static str { + "Something went wrong. Please try again.\nThis error has been reported. You can also report it on Discord.\nReport on Discord" +} + fn prompt_guard_user_message(action: PromptEnforcementAction) -> &'static str { match action { PromptEnforcementAction::Allow => "Message accepted.", @@ -135,6 +142,12 @@ fn prompt_guard_user_message(action: PromptEnforcementAction) -> &'static str { } } +#[cfg(test)] +pub(super) async fn set_test_forced_run_chat_task_error(message: Option<&str>) { + let mut slot = TEST_FORCED_RUN_CHAT_TASK_ERROR.lock().await; + *slot = message.map(str::to_string); +} + pub async fn start_chat( client_id: &str, thread_id: &str, @@ -263,13 +276,28 @@ pub async fn start_chat( request_id_task, err ); + let detailed = format!( + "run_chat_task failed client_id={} thread_id={} request_id={} error={}", + client_id_task, thread_id_task, request_id_task, err + ); + crate::core::observability::report_error( + detailed.as_str(), + "web_channel", + "run_chat_task", + &[ + ("channel", "web"), + ("error_type", "inference"), + ("thread_id", thread_id_task.as_str()), + ("request_id", request_id_task.as_str()), + ], + ); publish_web_channel_event(WebChannelEvent { event: "chat_error".to_string(), client_id: client_id_task.clone(), thread_id: thread_id_task.clone(), request_id: request_id_task.clone(), full_response: None, - message: Some(err), + message: Some(generic_inference_error_user_message().to_string()), error_type: Some("inference".to_string()), tool_name: None, skill_id: None, @@ -392,6 +420,20 @@ async fn run_chat_task( model_override: Option, temperature: Option, ) -> Result { + #[cfg(test)] + { + let mut slot = TEST_FORCED_RUN_CHAT_TASK_ERROR.lock().await; + if let Some(forced) = slot.take() { + log::debug!( + "[web-channel][test] forced run_chat_task failure client_id={} thread_id={} request_id={}", + client_id, + thread_id, + request_id + ); + return Err(forced); + } + } + let config = config_rpc::load_config_with_timeout().await?; let map_key = key_for(client_id, thread_id); let model_override = normalize_model_override(model_override); diff --git a/src/openhuman/channels/providers/web_tests.rs b/src/openhuman/channels/providers/web_tests.rs index 23b88b4a7..128aad9ac 100644 --- a/src/openhuman/channels/providers/web_tests.rs +++ b/src/openhuman/channels/providers/web_tests.rs @@ -1,11 +1,24 @@ use super::{ all_web_channel_controller_schemas, all_web_channel_registered_controllers, cancel_chat, - event_session_id_for, inference_budget_exceeded_user_message, - is_inference_budget_exceeded_error, json_output, key_for, normalize_model_override, - optional_f64, optional_string, required_string, schemas, start_chat, - subscribe_web_channel_events, + event_session_id_for, generic_inference_error_user_message, + inference_budget_exceeded_user_message, is_inference_budget_exceeded_error, json_output, + key_for, normalize_model_override, optional_f64, optional_string, required_string, schemas, + set_test_forced_run_chat_task_error, start_chat, subscribe_web_channel_events, }; use crate::core::TypeSchema; +use tokio::time::{timeout, Duration}; + +/// Ensures the test-only forced run_chat_task failure toggle is always reset, +/// even if the test panics before reaching explicit cleanup code. +struct TestForcedRunChatTaskErrorGuard; + +impl Drop for TestForcedRunChatTaskErrorGuard { + fn drop(&mut self) { + tokio::spawn(async { + set_test_forced_run_chat_task_error(None).await; + }); + } +} #[tokio::test] async fn start_chat_validates_required_fields() { @@ -58,6 +71,49 @@ async fn cancel_chat_validates_required_fields() { assert!(err.contains("thread_id is required")); } +#[tokio::test] +async fn start_chat_emits_sanitized_chat_error_on_inference_failure() { + set_test_forced_run_chat_task_error(Some( + "error sending request for url (https://internal-api.example.invalid/openai/v1/chat/completions)", + )) + .await; + let _forced_error_guard = TestForcedRunChatTaskErrorGuard; + + let mut rx = subscribe_web_channel_events(); + let request_id = start_chat( + "coverage-client", + "coverage-thread", + "Please summarize this in one line.", + None, + None, + ) + .await + .expect("start_chat should accept valid request"); + + let expected = generic_inference_error_user_message().to_string(); + let recv = timeout(Duration::from_secs(20), async move { + loop { + let event = rx.recv().await.expect("event stream should stay open"); + if event.event != "chat_error" { + continue; + } + if event.request_id != request_id { + continue; + } + return event; + } + }) + .await + .expect("expected chat_error event for started chat request"); + + let message = recv.message.unwrap_or_default(); + assert_eq!(message, expected); + assert!( + !message.contains("error sending request for url"), + "chat error payload must not expose raw transport details" + ); +} + #[test] fn detects_backend_budget_exhaustion_error() { assert!(is_inference_budget_exceeded_error( @@ -78,6 +134,15 @@ fn budget_exceeded_copy_mentions_top_up() { assert!(message.contains("credits")); } +#[test] +fn generic_error_copy_is_sanitized_and_has_discord_report_action() { + let message = generic_inference_error_user_message(); + assert!(message.contains("Something went wrong. Please try again.")); + assert!(message.contains("This error has been reported.")); + assert!(message + .contains("Report on Discord")); +} + // ── Schema catalog ──────────────────────────────────────────── #[test] diff --git a/src/openhuman/cron/scheduler.rs b/src/openhuman/cron/scheduler.rs index e1f043592..48c8b7ddf 100644 --- a/src/openhuman/cron/scheduler.rs +++ b/src/openhuman/cron/scheduler.rs @@ -15,6 +15,14 @@ use tokio::time::{self, Duration}; const MIN_POLL_SECONDS: u64 = 5; const SHELL_JOB_TIMEOUT_SECS: u64 = 120; +const AGENT_JOB_USER_FAILURE_MESSAGE: &str = "Something went wrong. Please try again.\nThis error has been reported. You can also report it on Discord.\nReport on Discord"; + +fn agent_session_target_tag(target: &SessionTarget) -> &'static str { + match target { + SessionTarget::Main => "main", + SessionTarget::Isolated => "isolated", + } +} pub async fn run(config: Config) -> Result<()> { // Ensure the global event bus is initialized so cron delivery events @@ -77,15 +85,22 @@ async fn execute_job_with_retry( job: &CronJob, ) -> (bool, String) { let mut last_output = String::new(); + let mut last_agent_error: Option = None; let retries = config.reliability.scheduler_retries; let mut backoff_ms = config.reliability.provider_backoff_ms.max(200); for attempt in 0..=retries { - let (success, output) = match job.job_type { - JobType::Shell => run_job_command(config, security, job).await, + let (success, output, agent_error) = match job.job_type { + JobType::Shell => { + let (success, output) = run_job_command(config, security, job).await; + (success, output, None) + } JobType::Agent => run_agent_job(config, job).await, }; last_output = output; + if agent_error.is_some() { + last_agent_error = agent_error; + } if success { return (true, last_output); @@ -103,6 +118,26 @@ async fn execute_job_with_retry( } } + if matches!(job.job_type, JobType::Agent) { + let report_message = last_agent_error + .as_deref() + .unwrap_or_else(|| last_output.as_str()); + crate::core::observability::report_error( + report_message, + "cron", + "agent_job", + &[ + ("job_id", job.id.as_str()), + ("agent_id", job.agent_id.as_deref().unwrap_or("none")), + ( + "session_target", + agent_session_target_tag(&job.session_target), + ), + ("failure", "retries_exhausted"), + ], + ); + } + (false, last_output) } @@ -170,7 +205,7 @@ async fn execute_and_persist_job( (job.id.clone(), success, failure_message) } -async fn run_agent_job(config: &Config, job: &CronJob) -> (bool, String) { +async fn run_agent_job(config: &Config, job: &CronJob) -> (bool, String, Option) { use crate::openhuman::agent::Agent; let name = job.name.clone().unwrap_or_else(|| "cron-job".to_string()); @@ -249,8 +284,13 @@ async fn run_agent_job(config: &Config, job: &CronJob) -> (bool, String) { } else { response }, + None, + ), + Err(e) => ( + false, + AGENT_JOB_USER_FAILURE_MESSAGE.to_string(), + Some(e.to_string()), ), - Err(e) => (false, format!("agent job failed: {e}")), } } diff --git a/src/openhuman/cron/scheduler_tests.rs b/src/openhuman/cron/scheduler_tests.rs index b20efba80..c89b79678 100644 --- a/src/openhuman/cron/scheduler_tests.rs +++ b/src/openhuman/cron/scheduler_tests.rs @@ -45,6 +45,22 @@ fn test_job(command: &str) -> CronJob { } } +#[test] +fn agent_failure_copy_mentions_retry_reporting_and_discord() { + assert!(AGENT_JOB_USER_FAILURE_MESSAGE.contains("Something went wrong. Please try again.")); + assert!(AGENT_JOB_USER_FAILURE_MESSAGE.contains("This error has been reported.")); + assert!(AGENT_JOB_USER_FAILURE_MESSAGE.contains("Report on Discord")); +} + +#[test] +fn agent_session_target_tag_matches_expected_values() { + assert_eq!(agent_session_target_tag(&SessionTarget::Main), "main"); + assert_eq!( + agent_session_target_tag(&SessionTarget::Isolated), + "isolated" + ); +} + #[cfg(not(windows))] #[tokio::test] async fn run_job_command_success() { @@ -203,11 +219,20 @@ async fn run_agent_job_returns_error_without_provider_key() { job.job_type = JobType::Agent; job.prompt = Some("Say hello".into()); - let (success, output) = run_agent_job(&config, &job).await; + let (success, output, raw_error) = run_agent_job(&config, &job).await; assert!(!success, "Agent job without provider key should fail"); + assert!(output.contains("Something went wrong. Please try again.")); + assert!(output.contains("This error has been reported.")); + assert!(output.contains("Report on Discord")); assert!( - !output.is_empty(), - "Expected non-empty error output from failed agent job" + raw_error + .as_deref() + .is_some_and(|value| !value.trim().is_empty()), + "Expected raw agent error for observability after retries are exhausted" + ); + assert!( + !output.contains("error sending request for url"), + "Expected sanitized output without raw transport details" ); }