mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(chat): sanitize agent/cron failures and add user-safe error fallback with Sentry reporting (#1332)
This commit is contained in:
@@ -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.\n<openhuman-link path="community/discord">Report on Discord</openhuman-link>';
|
||||
|
||||
type SegmentDelivery = { segments: Map<number, string> };
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
);
|
||||
|
||||
@@ -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(
|
||||
'<openhuman-link path="community/discord">Report on Discord</openhuman-link>'
|
||||
),
|
||||
})
|
||||
);
|
||||
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
|
||||
|
||||
@@ -86,6 +86,9 @@ static THREAD_SESSIONS: Lazy<Mutex<HashMap<String, SessionEntry>>> =
|
||||
|
||||
static IN_FLIGHT: Lazy<Mutex<HashMap<String, InFlightEntry>>> =
|
||||
Lazy::new(|| Mutex::new(HashMap::new()));
|
||||
#[cfg(test)]
|
||||
static TEST_FORCED_RUN_CHAT_TASK_ERROR: Lazy<Mutex<Option<String>>> =
|
||||
Lazy::new(|| Mutex::new(None));
|
||||
static BUDGET_ERROR_NORMALIZE_RE: Lazy<Regex> =
|
||||
Lazy::new(|| Regex::new(r"[-_\s]+").expect("budget normalize regex"));
|
||||
static BUDGET_ERROR_PATTERNS: Lazy<Vec<Regex>> = 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.\n<openhuman-link path=\"community/discord\">Report on Discord</openhuman-link>"
|
||||
}
|
||||
|
||||
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<String>,
|
||||
temperature: Option<f64>,
|
||||
) -> Result<WebChatTaskResult, String> {
|
||||
#[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);
|
||||
|
||||
@@ -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("<openhuman-link path=\"community/discord\">Report on Discord</openhuman-link>"));
|
||||
}
|
||||
|
||||
// ── Schema catalog ────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -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.\n<openhuman-link path=\"community/discord\">Report on Discord</openhuman-link>";
|
||||
|
||||
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<String> = 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<String>) {
|
||||
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}")),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user