fix: 增强 panic 安全与计算器健壮性
- agent_loop/processor: catch_unwind 隔离 panic,防止消息静默丢失 - calculator: 拒绝 NaN/Infinity 输入,修复阶乘溢出(上限 34),拒绝非有限表达式结果 - message: 修复 sanitize 两阶段删除索引未排序导致的越界 panic - utils: 新增 panic_payload_message 提取可读 panic 消息 - .gitignore: 忽略 artifacts/ 测试产物目录
This commit is contained in:
parent
f65fb0167f
commit
e0313ab8f3
@ -4,7 +4,7 @@
|
||||
|
||||
## [0.3.5] - 2026-08-12
|
||||
|
||||
较 [0.3.4] 的 5 个 commit 迭代,聚焦 **可观测性**、**并发性能** 与 **外部集成健壮性** 三大方向。
|
||||
较 [0.3.4] 的 7 个 commit 迭代,聚焦 **可观测性**、**并发性能** 与 **外部集成健壮性** 三大方向。
|
||||
|
||||
### 新增功能
|
||||
|
||||
@ -43,6 +43,17 @@
|
||||
- `ChatContainer`:新增 `topicId` prop 透传给 `MessageInput`。
|
||||
- `MessageInput`:监听 `topicId` 变化清空草稿,替代原 key remount 重置机制。
|
||||
|
||||
#### Panic 安全增强
|
||||
- `agent_loop.rs` / `processor.rs`:用 `catch_unwind` 隔离工具执行和消息处理的 panic,归一化为错误返回,LLM 可见错误并自我纠正,防止用户消息被静默丢弃。
|
||||
- `utils.rs`:新增 `panic_payload_message()` 从 panic payload 中提取可读消息,支持 `&str` / `String` / 其他类型降级。
|
||||
- `bus/message.rs`:修复 `sanitize_incomplete_tool_call_sequences` 两阶段删除索引未全局排序导致的越界 panic(Phase 1 降序 + Phase 1.5 升序 → 合并后必须重新排序)。
|
||||
|
||||
#### 计算器健壮性
|
||||
- 拒绝 `"NaN"` / `"inf"` 等非有限输入,防止 `sort_by` 中 `partial_cmp().unwrap()` panic。
|
||||
- 修复阶乘溢出:上限从 170 降至 34(35! 超出 `u128::MAX`),改用 `checked_mul` 替代 unchecked 乘法。
|
||||
- `evaluate` 表达式拒绝非有限结果(如 `1/0` → inf、`0/0` → NaN)。
|
||||
- 新增 7 个 tokio 测试覆盖所有边界场景。
|
||||
|
||||
### 测试
|
||||
- MCP 超时路径新增 2 个 tokio 单元测试。
|
||||
|
||||
|
||||
@ -15,6 +15,7 @@ use crate::text::{char_count, take_prefix_chars, take_suffix_chars};
|
||||
use crate::tools::{ToolContext, ToolRegistry};
|
||||
use crate::utils::format_error_chain;
|
||||
use async_trait::async_trait;
|
||||
use futures_util::FutureExt;
|
||||
use std::borrow::Cow;
|
||||
use std::collections::{HashMap, VecDeque};
|
||||
use std::hash::{Hash, Hasher};
|
||||
@ -2120,18 +2121,36 @@ impl AgentLoop {
|
||||
}
|
||||
};
|
||||
|
||||
match tool
|
||||
.execute_with_context(
|
||||
&{
|
||||
let tool_context = {
|
||||
let mut ctx = self.tool_context.clone();
|
||||
ctx.tool_call_id = Some(tool_call.id.clone());
|
||||
ctx
|
||||
},
|
||||
normalized_arguments.clone(),
|
||||
};
|
||||
// catch_unwind 隔离单个工具的 panic:否则一个工具崩溃会终止整个 turn,
|
||||
// 用户消息被静默丢弃。归一化为工具级失败后 LLM 还能看到错误并自我纠正。
|
||||
let execution = std::panic::AssertUnwindSafe(
|
||||
tool.execute_with_context(&tool_context, normalized_arguments.clone()),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(result) => {
|
||||
.catch_unwind()
|
||||
.await;
|
||||
|
||||
match execution {
|
||||
Err(payload) => {
|
||||
let error = format!(
|
||||
"Tool '{}' panicked: {}",
|
||||
tool_call.name,
|
||||
crate::utils::panic_payload_message(&payload)
|
||||
);
|
||||
tracing::error!(
|
||||
tool = %tool_call.name,
|
||||
args = %truncate_args(&tool_call.arguments, 4_000),
|
||||
normalized_args = %truncate_args(&normalized_arguments, 4_000),
|
||||
error = %error,
|
||||
"Tool execution panicked"
|
||||
);
|
||||
ToolExecutionOutcome::failure(format!("Error: {}", error), Some(error))
|
||||
}
|
||||
Ok(Ok(result)) => {
|
||||
if result.success {
|
||||
if let Some(pending_output) = parse_pending_tool_output(&result.output) {
|
||||
ToolExecutionOutcome::pending(pending_output)
|
||||
@ -2158,7 +2177,7 @@ impl AgentLoop {
|
||||
ToolExecutionOutcome::failure(failure_output, Some(error))
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
Ok(Err(e)) => {
|
||||
tracing::error!(
|
||||
tool = %tool_call.name,
|
||||
args = %truncate_args(&tool_call.arguments, 4_000),
|
||||
@ -3111,6 +3130,122 @@ mod tests {
|
||||
assert_eq!(messages.len(), 3);
|
||||
}
|
||||
|
||||
/// 良构不变量校验:sanitize 的输出必须满足
|
||||
/// 1. 每个带 tool_calls 的 assistant 之后紧邻其全部 tool 结果(无其他消息隔断);
|
||||
/// 2. 每个 tool 消息都有存活的父 assistant。
|
||||
fn assert_well_formed(messages: &[ChatMessage]) {
|
||||
let mut pending: Vec<String> = Vec::new();
|
||||
for (i, m) in messages.iter().enumerate() {
|
||||
if m.role == "assistant" {
|
||||
if let Some(calls) = m.tool_calls.as_ref().filter(|c| !c.is_empty()) {
|
||||
assert!(
|
||||
pending.is_empty(),
|
||||
"assistant at {i} starts tool_calls while previous results are pending"
|
||||
);
|
||||
pending = calls.iter().map(|tc| tc.id.clone()).collect();
|
||||
}
|
||||
} else if m.role == "tool" {
|
||||
let tc_id = m.tool_call_id.clone().unwrap_or_default();
|
||||
let pos = pending
|
||||
.iter()
|
||||
.position(|id| *id == tc_id)
|
||||
.unwrap_or_else(|| panic!("tool at {i} has no pending parent (id={tc_id})"));
|
||||
pending.remove(pos);
|
||||
} else if !pending.is_empty() {
|
||||
panic!("non-tool message at {i} interrupts pending tool results");
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
pending.is_empty(),
|
||||
"trailing assistant tool_calls without results"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sanitize_mixed_removal_order_does_not_panic_or_corrupt() {
|
||||
// Phase 1(反向扫描)按降序收集孤儿 assistant 索引 [2,1,0],
|
||||
// Phase 1.5(正向扫描)随后按升序追加索引 3 → remove_indices=[2,1,0,3]。
|
||||
// 若不全局排序就逐个 Vec::remove,第 4 次删除时越界 panic。
|
||||
let mut messages = vec![
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"orphan 1",
|
||||
vec![ToolCall {
|
||||
id: "call_x".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"orphan 2",
|
||||
vec![ToolCall {
|
||||
id: "call_y".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"orphan 3",
|
||||
vec![ToolCall {
|
||||
id: "call_z".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"resolved but interrupted",
|
||||
vec![ToolCall {
|
||||
id: "call_w".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::user("next question"),
|
||||
ChatMessage::tool("call_w", "bash", "result"),
|
||||
];
|
||||
|
||||
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
|
||||
|
||||
assert_eq!(removed, 5, "4 assistants + 1 orphaned tool result");
|
||||
assert_eq!(messages.len(), 1);
|
||||
assert_eq!(messages[0].role, "user");
|
||||
assert_eq!(messages[0].content, "next question");
|
||||
assert_well_formed(&messages);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sanitize_mixed_removal_order_deletes_correct_messages() {
|
||||
// remove_indices=[0(Phase 1), 1(Phase 1.5)]:先 remove(0) 后索引漂移,
|
||||
// 未排序时 remove(1) 会误删 user 消息而非第二个 assistant。
|
||||
let mut messages = vec![
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"orphan",
|
||||
vec![ToolCall {
|
||||
id: "call_x".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::assistant_with_tool_calls(
|
||||
"resolved but interrupted",
|
||||
vec![ToolCall {
|
||||
id: "call_y".to_string(),
|
||||
name: "bash".to_string(),
|
||||
arguments: serde_json::json!({}),
|
||||
}],
|
||||
),
|
||||
ChatMessage::user("interrupting question"),
|
||||
ChatMessage::tool("call_y", "bash", "result"),
|
||||
];
|
||||
|
||||
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
|
||||
|
||||
assert_eq!(removed, 3, "2 assistants + 1 orphaned tool result");
|
||||
assert_eq!(messages.len(), 1);
|
||||
assert_eq!(messages[0].role, "user");
|
||||
assert_eq!(messages[0].content, "interrupting question");
|
||||
assert_well_formed(&messages);
|
||||
}
|
||||
|
||||
// ===== LLM 重试机制测试 =====
|
||||
|
||||
#[test]
|
||||
|
||||
@ -443,7 +443,12 @@ pub(crate) fn sanitize_incomplete_tool_call_sequences(messages: &mut Vec<ChatMes
|
||||
}
|
||||
}
|
||||
|
||||
// Remove in descending index order to avoid shifting
|
||||
// Remove in descending index order to avoid shifting.
|
||||
// 两阶段产出的索引并非全局降序:Phase 1(反向扫描)按降序追加,
|
||||
// Phase 1.5(正向扫描)按升序追加。逐个 Vec::remove 前必须全局排序,
|
||||
// 否则已删除元素会使后续索引漂移(删错消息)甚至越界 panic。
|
||||
remove_indices.sort_unstable_by(|a, b| b.cmp(a));
|
||||
remove_indices.dedup();
|
||||
for &idx in &remove_indices {
|
||||
messages.remove(idx);
|
||||
removed += 1;
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use futures_util::FutureExt;
|
||||
use parking_lot::Mutex;
|
||||
|
||||
use tokio::sync::Semaphore;
|
||||
@ -183,13 +184,29 @@ impl InboundProcessor {
|
||||
&session_id_for_span,
|
||||
async move {
|
||||
let _permit = permit; // 持有 permit 直到任务完成
|
||||
if let Err(e) = processor.process_one(inbound).await {
|
||||
// catch_unwind 将 panic 归一化为错误:否则工具/历史清理中的
|
||||
// panic 只会终止任务并打 panic hook 日志,跳过错误日志与指标,
|
||||
// 用户消息被静默吞掉。参考 channels/wechat.rs 的同类用法。
|
||||
let result = std::panic::AssertUnwindSafe(processor.process_one(inbound))
|
||||
.catch_unwind()
|
||||
.await;
|
||||
match result {
|
||||
Ok(Ok(())) => {}
|
||||
Ok(Err(e)) => {
|
||||
tracing::error!(
|
||||
error = %crate::utils::format_error_chain(&e),
|
||||
"Message processing failed"
|
||||
);
|
||||
crate::observability::metrics::record_message_processing_error();
|
||||
}
|
||||
Err(payload) => {
|
||||
tracing::error!(
|
||||
error = %crate::utils::panic_payload_message(&payload),
|
||||
"Message processing panicked"
|
||||
);
|
||||
crate::observability::metrics::record_message_processing_error();
|
||||
}
|
||||
}
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
@ -158,14 +158,20 @@ fn extract_f64(args: &serde_json::Value, key: &str, name: &str) -> Result<f64, S
|
||||
match args.get(key) {
|
||||
None => Err(format!("Missing required parameter: {name}")),
|
||||
Some(v) => {
|
||||
if let Some(n) = v.as_f64() {
|
||||
Ok(n)
|
||||
let n = if let Some(n) = v.as_f64() {
|
||||
n
|
||||
} else if let Some(s) = v.as_str() {
|
||||
s.parse::<f64>()
|
||||
.map_err(|_| format!("{name} is not a valid number: {s}"))
|
||||
.map_err(|_| format!("{name} is not a valid number: {s}"))?
|
||||
} else {
|
||||
Err(format!("{name} must be a number"))
|
||||
return Err(format!("{name} must be a number"));
|
||||
};
|
||||
// f64::from_str 接受 "NaN"/"inf";非有限值会使 sort_by 的
|
||||
// partial_cmp().unwrap() panic,且算术结果无意义,统一在边界拒绝。
|
||||
if !n.is_finite() {
|
||||
return Err(format!("{name} must be a finite number"));
|
||||
}
|
||||
Ok(n)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -207,6 +213,11 @@ fn extract_values(args: &serde_json::Value, min_len: usize) -> Result<Vec<f64>,
|
||||
} else {
|
||||
return Err(format!("values[{i}] is not a valid number"));
|
||||
};
|
||||
// f64::from_str 接受 "NaN"/"inf";非有限值会使 sort_by 的
|
||||
// partial_cmp().unwrap() panic,且统计结果无意义,统一在边界拒绝。
|
||||
if !n.is_finite() {
|
||||
return Err(format!("values[{i}] is not a finite number"));
|
||||
}
|
||||
nums.push(n);
|
||||
}
|
||||
Ok(nums)
|
||||
@ -251,12 +262,16 @@ fn calc_factorial(args: &serde_json::Value) -> Result<String, String> {
|
||||
}
|
||||
#[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
|
||||
let n = x.round() as u128;
|
||||
if n > 170 {
|
||||
return Err("Factorial result exceeds f64 range (max input: 170)".to_string());
|
||||
// u128::MAX ≈ 3.4e38,34! 是最后一个不溢出的阶乘(35! ≈ 1.03e40)。
|
||||
// 修复前按 f64 范围放行到 170,实际在 n≥35 时 debug panic / release 静默回绕。
|
||||
if n > 34 {
|
||||
return Err("Factorial result exceeds supported integer range (max input: 34)".to_string());
|
||||
}
|
||||
let mut result: u128 = 1;
|
||||
for i in 2..=n {
|
||||
result *= i;
|
||||
result = result
|
||||
.checked_mul(i)
|
||||
.ok_or_else(|| "Factorial result exceeds supported integer range".to_string())?;
|
||||
}
|
||||
Ok(result.to_string())
|
||||
}
|
||||
@ -412,8 +427,15 @@ fn calc_evaluate(args: &serde_json::Value) -> Result<String, String> {
|
||||
.ok_or_else(|| "Missing required parameter: expression".to_string())?;
|
||||
|
||||
meval::eval_str(expression)
|
||||
.map(format_num)
|
||||
.map_err(|e| format!("Expression evaluation error: {e}"))
|
||||
.and_then(|n| {
|
||||
// 表达式可产生非有限结果(如 "1/0" → inf、"0/0" → NaN),
|
||||
// 与 extract_values/extract_f64 的边界策略保持一致:拒绝输出。
|
||||
if !n.is_finite() {
|
||||
return Err(format!("Expression result is not a finite number: {expression}"));
|
||||
}
|
||||
Ok(format_num(n))
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@ -779,4 +801,99 @@ mod tests {
|
||||
.contains("Missing required parameters")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_median_rejects_nan_string_value() {
|
||||
// f64::from_str accepts "NaN"; partial_cmp on NaN is None and would
|
||||
// panic inside sort_by — must surface as a tool error instead.
|
||||
let tool = CalculatorTool::new();
|
||||
let result = tool
|
||||
.execute(json!({"function": "median", "values": ["NaN", 1.0, 2.0]}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!result.success);
|
||||
assert!(
|
||||
result.error.as_ref().unwrap().contains("finite"),
|
||||
"expected finiteness error, got: {:?}",
|
||||
result.error
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_percentile_rejects_infinity_string_value() {
|
||||
let tool = CalculatorTool::new();
|
||||
let result = tool
|
||||
.execute(json!({"function": "percentile", "values": ["inf", 1.0], "p": 50}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!result.success);
|
||||
assert!(result.error.as_ref().unwrap().contains("finite"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_sum_rejects_nan_instead_of_returning_nan() {
|
||||
let tool = CalculatorTool::new();
|
||||
let result = tool
|
||||
.execute(json!({"function": "sum", "values": ["NaN", 1.0]}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!result.success);
|
||||
assert!(result.error.as_ref().unwrap().contains("finite"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_clamp_rejects_non_finite_scalar() {
|
||||
let tool = CalculatorTool::new();
|
||||
let result = tool
|
||||
.execute(json!({"function": "clamp", "x": "NaN", "min_val": 0.0, "max_val": 1.0}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!result.success);
|
||||
assert!(result.error.as_ref().unwrap().contains("finite"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_factorial_large_input_returns_error_not_overflow() {
|
||||
// 35! ≈ 1.03e40 超出 u128::MAX ≈ 3.4e38:修复前 debug 下乘法溢出 panic、
|
||||
// release 下静默回绕。必须返回工具错误。
|
||||
let tool = CalculatorTool::new();
|
||||
let result = tool
|
||||
.execute(json!({"function": "factorial", "x": 35.0}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!result.success);
|
||||
assert!(
|
||||
result.error.as_ref().unwrap().contains("range"),
|
||||
"expected range error, got: {:?}",
|
||||
result.error
|
||||
);
|
||||
// 34! 仍可精确计算
|
||||
let ok = tool
|
||||
.execute(json!({"function": "factorial", "x": 34.0}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(ok.success);
|
||||
assert_eq!(
|
||||
ok.output,
|
||||
"295232799039604140847618609643520000000"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_evaluate_rejects_non_finite_result() {
|
||||
let tool = CalculatorTool::new();
|
||||
let division_by_zero = tool
|
||||
.execute(json!({"function": "evaluate", "expression": "1/0"}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!division_by_zero.success);
|
||||
assert!(division_by_zero.error.as_ref().unwrap().contains("finite"));
|
||||
|
||||
let nan_result = tool
|
||||
.execute(json!({"function": "evaluate", "expression": "0/0"}))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!nan_result.success);
|
||||
assert!(nan_result.error.as_ref().unwrap().contains("finite"));
|
||||
}
|
||||
}
|
||||
|
||||
38
src/utils.rs
38
src/utils.rs
@ -11,6 +11,20 @@ pub fn current_timestamp() -> i64 {
|
||||
.as_millis() as i64
|
||||
}
|
||||
|
||||
/// 从 `catch_unwind` 的 panic payload 中提取可读消息。
|
||||
///
|
||||
/// `panic!` 的 payload 通常是 `&str` 或 `String`;其他类型(如直接
|
||||
/// `panic!(42)`)无法还原原文,返回占位描述。
|
||||
pub fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String {
|
||||
if let Some(s) = payload.downcast_ref::<&str>() {
|
||||
return (*s).to_string();
|
||||
}
|
||||
if let Some(s) = payload.downcast_ref::<String>() {
|
||||
return s.clone();
|
||||
}
|
||||
"<non-string panic payload>".to_string()
|
||||
}
|
||||
|
||||
/// 递归展开 `error.source()` 链,生成 `"顶层错误\ncaused by: 原因\ncaused by: ..."` 格式的字符串。
|
||||
pub fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||
let mut details = vec![error.to_string()];
|
||||
@ -23,3 +37,27 @@ pub fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||
|
||||
details.join("\ncaused by: ")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::any::Any;
|
||||
|
||||
#[test]
|
||||
fn test_panic_payload_message_str() {
|
||||
let payload: Box<dyn Any + Send> = Box::new("boom");
|
||||
assert_eq!(panic_payload_message(&*payload), "boom");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_panic_payload_message_string() {
|
||||
let payload: Box<dyn Any + Send> = Box::new(String::from("boom"));
|
||||
assert_eq!(panic_payload_message(&*payload), "boom");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_panic_payload_message_other_type() {
|
||||
let payload: Box<dyn Any + Send> = Box::new(42u32);
|
||||
assert!(panic_payload_message(&*payload).contains("non-string"));
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user