Skip to content

Commit 9486f94

Browse files
committed
[fix] Update to use a new RecvErrorKind instead of custom error handling.
1 parent acde38f commit 9486f94

File tree

4 files changed

+99
-69
lines changed

4 files changed

+99
-69
lines changed

crates/chat-cli/src/cli/chat/consts.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ pub const MAX_USER_MESSAGE_SIZE: usize = 400_000;
1313

1414
pub const DUMMY_TOOL_NAME: &str = "dummy";
1515

16-
/// Marker key used to identify invalid tool arguments (non-JSON objects)
17-
pub const INVALID_TOOL_ARGS_MARKER: &str = "__error__invalid_args_json__";
18-
1916
pub const MAX_NUMBER_OF_IMAGES_PER_REQUEST: usize = 10;
2017

2118
/// In bytes - 10 MB

crates/chat-cli/src/cli/chat/mod.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,6 +2694,53 @@ impl ChatSession {
26942694
.await?,
26952695
));
26962696
},
2697+
RecvErrorKind::ToolValidationError {
2698+
tool_use_id,
2699+
name,
2700+
message,
2701+
error_message,
2702+
} => {
2703+
self.send_chat_telemetry(
2704+
os,
2705+
TelemetryResult::Failed,
2706+
Some(reason),
2707+
Some(reason_desc),
2708+
status_code,
2709+
false, // We retry the request, so don't end the current turn yet.
2710+
)
2711+
.await;
2712+
2713+
error!(
2714+
recv_error.request_metadata.request_id,
2715+
tool_use_id, name, error_message, "Tool validation failed"
2716+
);
2717+
self.conversation
2718+
.push_assistant_message(os, *message, Some(recv_error.request_metadata));
2719+
let tool_results = vec![ToolUseResult {
2720+
tool_use_id,
2721+
content: vec![ToolUseResultBlock::Text(format!(
2722+
"Tool validation failed: {}. Please ensure tool arguments are provided as a valid JSON object.",
2723+
error_message
2724+
))],
2725+
status: ToolResultStatus::Error,
2726+
}];
2727+
// User hint of what happened
2728+
let _ = queue!(
2729+
self.stdout,
2730+
style::Print("\n\n"),
2731+
style::SetForegroundColor(Color::Yellow),
2732+
style::Print(format!("Tool validation failed: {}", error_message)),
2733+
style::ResetColor,
2734+
style::Print("\n"),
2735+
);
2736+
self.conversation.add_tool_results(tool_results);
2737+
self.send_tool_use_telemetry(os).await;
2738+
return Ok(ChatState::HandleResponseStream(
2739+
self.conversation
2740+
.as_sendable_conversation_state(os, &mut self.stderr, false)
2741+
.await?,
2742+
));
2743+
},
26972744
_ => {
26982745
self.send_chat_telemetry(
26992746
os,

crates/chat-cli/src/cli/chat/parser.rs

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use tracing::{
2525
warn,
2626
};
2727

28-
use super::consts::INVALID_TOOL_ARGS_MARKER;
2928
use super::message::{
3029
AssistantMessage,
3130
AssistantToolUse,
@@ -92,6 +91,7 @@ impl RecvError {
9291
RecvErrorKind::StreamTimeout { .. } => None,
9392
RecvErrorKind::UnexpectedToolUseEos { .. } => None,
9493
RecvErrorKind::Cancelled => None,
94+
RecvErrorKind::ToolValidationError { .. } => None,
9595
}
9696
}
9797
}
@@ -104,6 +104,7 @@ impl ReasonCode for RecvError {
104104
RecvErrorKind::StreamTimeout { .. } => "RecvErrorStreamTimeout".to_string(),
105105
RecvErrorKind::UnexpectedToolUseEos { .. } => "RecvErrorUnexpectedToolUseEos".to_string(),
106106
RecvErrorKind::Cancelled => "Interrupted".to_string(),
107+
RecvErrorKind::ToolValidationError { .. } => "RecvErrorToolValidation".to_string(),
107108
}
108109
}
109110
}
@@ -152,6 +153,14 @@ pub enum RecvErrorKind {
152153
/// The stream processing task was cancelled
153154
#[error("Stream handling was cancelled")]
154155
Cancelled,
156+
/// Tool validation failed due to invalid arguments
157+
#[error("Tool validation failed for tool: {} with id: {}", .name, .tool_use_id)]
158+
ToolValidationError {
159+
tool_use_id: String,
160+
name: String,
161+
message: Box<AssistantMessage>,
162+
error_message: String,
163+
},
155164
}
156165

157166
/// Represents a response stream from a call to the SendMessage API.
@@ -479,9 +488,34 @@ impl ResponseParser {
479488
serde_json::Value::Object(_) => args,
480489
_ => {
481490
error!("Received non-object JSON for tool arguments: {:?}", args);
482-
serde_json::json!({
483-
INVALID_TOOL_ARGS_MARKER: format!("Expected JSON object, got: {:?}", args)
484-
})
491+
let warning_args = serde_json::Value::Object(
492+
[(
493+
"key".to_string(),
494+
serde_json::Value::String(
495+
"WARNING: the actual tool use arguments were not a valid JSON object".to_string(),
496+
),
497+
)]
498+
.into_iter()
499+
.collect(),
500+
);
501+
self.tool_uses.push(AssistantToolUse {
502+
id: id.clone(),
503+
name: name.clone(),
504+
orig_name: name.clone(),
505+
args: warning_args.clone(),
506+
orig_args: warning_args.clone(),
507+
});
508+
let message = Box::new(AssistantMessage::new_tool_use(
509+
Some(self.message_id.clone()),
510+
std::mem::take(&mut self.assistant_text),
511+
self.tool_uses.clone().into_iter().collect(),
512+
));
513+
return Err(self.error(RecvErrorKind::ToolValidationError {
514+
tool_use_id: id,
515+
name,
516+
message,
517+
error_message: format!("Expected JSON object, got: {:?}", args),
518+
}));
485519
},
486520
}
487521
},
@@ -812,16 +846,18 @@ mod tests {
812846
);
813847

814848
let mut output = String::new();
815-
let mut found_invalid_marker = false;
849+
let mut found_validation_error = false;
816850
for _ in 0..5 {
817-
let event = parser.recv().await.unwrap();
818-
output.push_str(&format!("{:?}", event));
819-
820-
// Check for invalid args marker in ToolUse events
821-
if let ResponseEvent::ToolUse(tool_use) = event {
822-
if tool_use.args.get(INVALID_TOOL_ARGS_MARKER).is_some() {
823-
found_invalid_marker = true;
824-
}
851+
match parser.recv().await {
852+
Ok(event) => {
853+
output.push_str(&format!("{:?}", event));
854+
},
855+
Err(recv_error) => {
856+
if matches!(recv_error.source, RecvErrorKind::ToolValidationError { .. }) {
857+
found_validation_error = true;
858+
}
859+
break;
860+
},
825861
}
826862
}
827863

@@ -830,8 +866,8 @@ mod tests {
830866
"assistant text preceding a code reference should be ignored as this indicates licensed code is being returned"
831867
);
832868
assert!(
833-
found_invalid_marker,
834-
"Expected to find invalid args marker for non-object JSON"
869+
found_validation_error,
870+
"Expected to find tool validation error for non-object JSON"
835871
);
836872
}
837873
}

crates/chat-cli/src/cli/chat/tool_manager.rs

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,7 @@ use crate::cli::agent::{
6464
McpServerConfig,
6565
};
6666
use crate::cli::chat::cli::prompts::GetPromptError;
67-
use crate::cli::chat::consts::{
68-
DUMMY_TOOL_NAME,
69-
INVALID_TOOL_ARGS_MARKER,
70-
};
67+
use crate::cli::chat::consts::DUMMY_TOOL_NAME;
7168
use crate::cli::chat::message::AssistantToolUse;
7269
use crate::cli::chat::server_messenger::{
7370
ServerMessengerBuilder,
@@ -849,54 +846,7 @@ impl ToolManager {
849846
Ok(self.schema.clone())
850847
}
851848

852-
/// Validates that the tool arguments are a valid JSON object and checks for invalid args
853-
/// marker.
854-
///
855-
/// # Arguments
856-
/// * `args` - The JSON value containing tool arguments
857-
/// * `tool_name` - Name of the tool for error reporting
858-
/// * `tool_use_id` - ID of the tool use for error reporting
859-
///
860-
/// # Returns
861-
/// * `Ok(())` if validation passes
862-
/// * `Err(ToolResult)` if validation fails with appropriate error message
863-
fn validate_tool_args(
864-
args: &serde_json::Value,
865-
tool_name: &str,
866-
tool_use_id: &str,
867-
) -> Result<(), ToolResult> {
868-
// Ensure args is an object for safe access
869-
if !args.is_object() {
870-
return Err(ToolResult {
871-
tool_use_id: tool_use_id.to_string(),
872-
content: vec![ToolResultContentBlock::Text(format!(
873-
"The tool \"{}\" requires arguments to be a JSON object, but received: {}",
874-
tool_name,
875-
args
876-
))],
877-
status: ToolResultStatus::Error,
878-
});
879-
}
880-
881-
// Now safely check for invalid args marker (args is guaranteed to be an object)
882-
if let Some(error_msg) = args.get(INVALID_TOOL_ARGS_MARKER).and_then(|v| v.as_str()) {
883-
return Err(ToolResult {
884-
tool_use_id: tool_use_id.to_string(),
885-
content: vec![ToolResultContentBlock::Text(format!(
886-
"The tool \"{}\" is supplied with invalid input format. {}",
887-
tool_name, error_msg
888-
))],
889-
status: ToolResultStatus::Error,
890-
});
891-
}
892-
893-
Ok(())
894-
}
895-
896849
pub async fn get_tool_from_tool_use(&mut self, value: AssistantToolUse) -> Result<Tool, ToolResult> {
897-
// Validate tool arguments
898-
Self::validate_tool_args(&value.args, &value.name, &value.id)?;
899-
900850
let map_err = |parse_error| ToolResult {
901851
tool_use_id: value.id.clone(),
902852
content: vec![ToolResultContentBlock::Text(format!(

0 commit comments

Comments
 (0)