diff --git a/Cargo.lock b/Cargo.lock index 620fa5b66c74..b4f3c4aae229 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3519,7 +3519,7 @@ dependencies = [ [[package]] name = "goose-test" -version = "1.1.0" +version = "1.3.0" dependencies = [ "clap", "serde_json", diff --git a/crates/goose-cli/src/scenario_tests/mock_client.rs b/crates/goose-cli/src/scenario_tests/mock_client.rs index 4c5d9c84bb85..4dd337769135 100644 --- a/crates/goose-cli/src/scenario_tests/mock_client.rs +++ b/crates/goose-cli/src/scenario_tests/mock_client.rs @@ -2,11 +2,10 @@ //! add a tool you want to have around and then add the client to the extension router use mcp_client::client::{Error, McpClientTrait}; -use mcp_core::ToolError; use rmcp::{ model::{ - CallToolResult, Content, GetPromptResult, ListPromptsResult, ListResourcesResult, - ListToolsResult, ReadResourceResult, ServerNotification, Tool, + CallToolResult, Content, ErrorData, GetPromptResult, ListPromptsResult, + ListResourcesResult, ListToolsResult, ReadResourceResult, ServerNotification, Tool, }, object, }; @@ -17,7 +16,7 @@ use tokio_util::sync::CancellationToken; pub struct MockClient { tools: HashMap, - handlers: HashMap Result, ToolError> + Send + Sync>>, + handlers: HashMap Result, ErrorData> + Send + Sync>>, } impl MockClient { @@ -30,7 +29,7 @@ impl MockClient { pub(crate) fn add_tool(mut self, tool: Tool, handler: F) -> Self where - F: Fn(&Value) -> Result, ToolError> + Send + Sync + 'static, + F: Fn(&Value) -> Result, ErrorData> + Send + Sync + 'static, { let tool_name = tool.name.to_string(); self.tools.insert(tool_name.clone(), tool); diff --git a/crates/goose-cli/src/session/mod.rs b/crates/goose-cli/src/session/mod.rs index 82f5328bf4f2..5b9b75f7f922 100644 --- a/crates/goose-cli/src/session/mod.rs +++ b/crates/goose-cli/src/session/mod.rs @@ -34,9 +34,9 @@ use goose::config::Config; use goose::providers::pricing::initialize_pricing_cache; use goose::session; use input::InputResult; -use mcp_core::handler::ToolError; use rmcp::model::PromptMessage; use rmcp::model::ServerNotification; +use rmcp::model::{ErrorCode, ErrorData}; use goose::conversation::message::{Message, MessageContent}; use rand::{distributions::Alphanumeric, Rng}; @@ -927,7 +927,7 @@ impl Session { let mut response_message = Message::user(); response_message.content.push(MessageContent::tool_response( confirmation.id.clone(), - Err(ToolError::ExecutionError("Tool call cancelled by user".to_string())) + Err(ErrorData { code: ErrorCode::INVALID_REQUEST, message: std::borrow::Cow::from("Tool call cancelled by user".to_string()), data: None }) )); self.messages.push(response_message); if let Some(session_file) = &self.session_file { @@ -1294,7 +1294,11 @@ impl Session { for (req_id, _) in &tool_requests { response_message.content.push(MessageContent::tool_response( req_id.clone(), - Err(ToolError::ExecutionError(notification.clone())), + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: std::borrow::Cow::from(notification.clone()), + data: None, + }), )); } self.push_message(response_message); diff --git a/crates/goose-mcp/src/computercontroller/docx_tool.rs b/crates/goose-mcp/src/computercontroller/docx_tool.rs index e88564e465b8..9347d796cae0 100644 --- a/crates/goose-mcp/src/computercontroller/docx_tool.rs +++ b/crates/goose-mcp/src/computercontroller/docx_tool.rs @@ -1,7 +1,7 @@ use docx_rs::*; use image::{self, ImageFormat}; -use mcp_core::ToolError; -use rmcp::model::Content; +use rmcp::model::{Content, ErrorCode, ErrorData}; +use std::borrow::Cow; use std::{fs, io::Cursor}; #[derive(Debug)] @@ -90,15 +90,19 @@ pub async fn docx_tool( operation: &str, content: Option<&str>, params: Option<&serde_json::Value>, -) -> Result, ToolError> { +) -> Result, ErrorData> { match operation { "extract_text" => { - let file = fs::read(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read DOCX file: {}", e)) + let file = fs::read(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read DOCX file: {}", e)), + data: None, })?; - let docx = read_docx(&file).map_err(|e| { - ToolError::ExecutionError(format!("Failed to parse DOCX file: {}", e)) + let docx = read_docx(&file).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to parse DOCX file: {}", e)), + data: None, })?; let mut text = String::new(); @@ -169,10 +173,10 @@ pub async fn docx_tool( } "update_doc" => { - let content = content.ok_or_else(|| { - ToolError::InvalidParameters( - "Content parameter required for update_doc".to_string(), - ) + let content = content.ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Content parameter required for update_doc"), + data: None, })?; // Parse update mode and style from params @@ -186,15 +190,14 @@ pub async fn docx_tool( let mode = match mode { "append" => UpdateMode::Append, "replace" => { - let old_text = - params - .get("old_text") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "old_text parameter required for replace mode".to_string(), - ) - })?; + let old_text = params + .get("old_text") + .and_then(|v| v.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("old_text parameter required for replace mode"), + data: None, + })?; UpdateMode::Replace { old_text: old_text.to_string(), } @@ -213,10 +216,10 @@ pub async fn docx_tool( let image_path = params .get("image_path") .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "image_path parameter required for add_image mode".to_string(), - ) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("image_path parameter required for add_image mode"), + data: None, })? .to_string(); @@ -236,10 +239,11 @@ pub async fn docx_tool( height, } } - _ => return Err(ToolError::InvalidParameters( - "Invalid mode. Must be 'append', 'replace', 'structured', or 'add_image'" - .to_string(), - )), + _ => return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Invalid mode. Must be 'append', 'replace', 'structured', or 'add_image'"), + data: None, + }), }; (mode, style) } else { @@ -250,11 +254,15 @@ pub async fn docx_tool( UpdateMode::Append => { // Read existing document if it exists, or create new one let mut doc = if std::path::Path::new(path).exists() { - let file = fs::read(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read DOCX file: {}", e)) + let file = fs::read(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read DOCX file: {}", e)), + data: None, })?; - read_docx(&file).map_err(|e| { - ToolError::ExecutionError(format!("Failed to parse DOCX file: {}", e)) + read_docx(&file).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to parse DOCX file: {}", e)), + data: None, })? } else { Docx::new() @@ -278,13 +286,17 @@ pub async fn docx_tool( let mut buf = Vec::new(); { let mut cursor = Cursor::new(&mut buf); - doc.build().pack(&mut cursor).map_err(|e| { - ToolError::ExecutionError(format!("Failed to build DOCX: {}", e)) + doc.build().pack(&mut cursor).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to build DOCX: {}", e)), + data: None, })?; } - fs::write(path, &buf).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write DOCX file: {}", e)) + fs::write(path, &buf).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write DOCX file: {}", e)), + data: None, })?; Ok(vec![Content::text(format!( @@ -295,12 +307,16 @@ pub async fn docx_tool( UpdateMode::Replace { old_text } => { // Read existing document - let file = fs::read(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read DOCX file: {}", e)) + let file = fs::read(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read DOCX file: {}", e)), + data: None, })?; - let docx = read_docx(&file).map_err(|e| { - ToolError::ExecutionError(format!("Failed to parse DOCX file: {}", e)) + let docx = read_docx(&file).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to parse DOCX file: {}", e)), + data: None, })?; let mut new_doc = Docx::new(); @@ -371,22 +387,30 @@ pub async fn docx_tool( } if !found_text { - return Err(ToolError::ExecutionError(format!( - "Could not find text to replace: {}", - old_text - ))); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Could not find text to replace: {}", + old_text + )), + data: None, + }); } let mut buf = Vec::new(); { let mut cursor = Cursor::new(&mut buf); - new_doc.build().pack(&mut cursor).map_err(|e| { - ToolError::ExecutionError(format!("Failed to build DOCX: {}", e)) + new_doc.build().pack(&mut cursor).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to build DOCX: {}", e)), + data: None, })?; } - fs::write(path, &buf).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write DOCX file: {}", e)) + fs::write(path, &buf).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write DOCX file: {}", e)), + data: None, })?; Ok(vec![Content::text(format!( @@ -397,11 +421,15 @@ pub async fn docx_tool( UpdateMode::InsertStructured { level, style } => { let mut doc = if std::path::Path::new(path).exists() { - let file = fs::read(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read DOCX file: {}", e)) + let file = fs::read(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read DOCX file: {}", e)), + data: None, })?; - read_docx(&file).map_err(|e| { - ToolError::ExecutionError(format!("Failed to parse DOCX file: {}", e)) + read_docx(&file).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to parse DOCX file: {}", e)), + data: None, })? } else { Docx::new() @@ -431,13 +459,17 @@ pub async fn docx_tool( let mut buf = Vec::new(); { let mut cursor = Cursor::new(&mut buf); - doc.build().pack(&mut cursor).map_err(|e| { - ToolError::ExecutionError(format!("Failed to build DOCX: {}", e)) + doc.build().pack(&mut cursor).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to build DOCX: {}", e)), + data: None, })?; } - fs::write(path, &buf).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write DOCX file: {}", e)) + fs::write(path, &buf).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write DOCX file: {}", e)), + data: None, })?; Ok(vec![Content::text(format!( @@ -452,43 +484,55 @@ pub async fn docx_tool( height, } => { let mut doc = if std::path::Path::new(path).exists() { - let file = fs::read(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read DOCX file: {}", e)) + let file = fs::read(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read DOCX file: {}", e)), + data: None, })?; - read_docx(&file).map_err(|e| { - ToolError::ExecutionError(format!("Failed to parse DOCX file: {}", e)) + read_docx(&file).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to parse DOCX file: {}", e)), + data: None, })? } else { Docx::new() }; // Read the image file - let image_data = fs::read(&image_path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read image file: {}", e)) + let image_data = fs::read(&image_path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read image file: {}", e)), + data: None, })?; // Get image format and extension let extension = std::path::Path::new(&image_path) .extension() .and_then(|e| e.to_str()) - .ok_or_else(|| { - ToolError::ExecutionError("Invalid image file extension".to_string()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Invalid image file extension".to_string()), + data: None, })? .to_lowercase(); // Convert to PNG if not already PNG let image_data = if extension != "png" { // Try to convert to PNG using the image crate - let img = image::load_from_memory(&image_data).map_err(|e| { - ToolError::ExecutionError(format!("Failed to load image: {}", e)) + let img = image::load_from_memory(&image_data).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to load image: {}", e)), + data: None, })?; let mut png_data = Vec::new(); img.write_to(&mut Cursor::new(&mut png_data), ImageFormat::Png) - .map_err(|e| { - ToolError::ExecutionError(format!( + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( "Failed to convert image to PNG: {}", e - )) + )), + data: None, })?; png_data } else { @@ -526,13 +570,17 @@ pub async fn docx_tool( let mut buf = Vec::new(); { let mut cursor = Cursor::new(&mut buf); - doc.build().pack(&mut cursor).map_err(|e| { - ToolError::ExecutionError(format!("Failed to build DOCX: {}", e)) + doc.build().pack(&mut cursor).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to build DOCX: {}", e)), + data: None, })?; } - fs::write(path, &buf).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write DOCX file: {}", e)) + fs::write(path, &buf).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write DOCX file: {}", e)), + data: None, })?; Ok(vec![Content::text(format!( @@ -543,10 +591,14 @@ pub async fn docx_tool( } } - _ => Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}. Valid operations are: 'extract_text', 'update_doc'", - operation - ))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Invalid operation: {}. Valid operations are: 'extract_text', 'update_doc'", + operation + )), + data: None, + }), } } diff --git a/crates/goose-mcp/src/computercontroller/mod.rs b/crates/goose-mcp/src/computercontroller/mod.rs index 19c172552134..8edd4cd59ee8 100644 --- a/crates/goose-mcp/src/computercontroller/mod.rs +++ b/crates/goose-mcp/src/computercontroller/mod.rs @@ -3,6 +3,7 @@ use etcetera::{choose_app_strategy, AppStrategy}; use indoc::{formatdoc, indoc}; use reqwest::{Client, Url}; use serde_json::Value; +use std::borrow::Cow; use std::{ collections::HashMap, fs, future::Future, path::PathBuf, pin::Pin, sync::Arc, sync::Mutex, }; @@ -12,15 +13,14 @@ use tokio::{process::Command, sync::mpsc}; use std::os::unix::fs::PermissionsExt; use mcp_core::{ - handler::{ - require_str_parameter, require_u64_parameter, PromptError, ResourceError, ToolError, - }, + handler::{require_str_parameter, require_u64_parameter, PromptError, ResourceError}, protocol::ServerCapabilities, }; use mcp_server::router::CapabilitiesBuilder; use mcp_server::Router; use rmcp::model::{ - AnnotateAble, Content, JsonRpcMessage, Prompt, RawResource, Resource, Tool, ToolAnnotations, + AnnotateAble, Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, RawResource, Resource, + Tool, ToolAnnotations, }; use rmcp::object; @@ -570,17 +570,24 @@ impl ComputerControllerRouter { content: &[u8], prefix: &str, extension: &str, - ) -> Result { + ) -> Result { let cache_path = self.get_cache_path(prefix, extension); - fs::write(&cache_path, content) - .map_err(|e| ToolError::ExecutionError(format!("Failed to write to cache: {}", e)))?; + fs::write(&cache_path, content).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write to cache: {}", e)), + data: None, + })?; Ok(cache_path) } // Helper function to register a file as a resource - fn register_as_resource(&self, cache_path: &PathBuf, mime_type: &str) -> Result<(), ToolError> { + fn register_as_resource(&self, cache_path: &PathBuf, mime_type: &str) -> Result<(), ErrorData> { let uri = Url::from_file_path(cache_path) - .map_err(|_| ToolError::ExecutionError("Invalid cache path".into()))? + .map_err(|_| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Invalid cache path"), + data: None, + })? .to_string(); let mut resource = RawResource::new(uri.clone(), cache_path.to_string_lossy().into_owned()); @@ -596,8 +603,16 @@ impl ComputerControllerRouter { Ok(()) } - async fn web_scrape(&self, params: Value) -> Result, ToolError> { - let url = require_str_parameter(¶ms, "url")?; + async fn web_scrape(&self, params: Value) -> Result, ErrorData> { + let url = params + .get("url") + .and_then(|v| v.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'url' parameter"), + data: None, + })?; + let save_as = params .get("save_as") .and_then(|v| v.as_str()) @@ -609,48 +624,64 @@ impl ComputerControllerRouter { .get(url) .send() .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to fetch URL: {}", e)))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to fetch URL: {}", e)), + data: None, + })?; let status = response.status(); if !status.is_success() { - return Err(ToolError::ExecutionError(format!( - "HTTP request failed with status: {}", - status - ))); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("HTTP request failed with status: {}", status)), + data: None, + }); } // Process based on save_as parameter - let (content, extension) = - match save_as { - "text" => { - let text = response.text().await.map_err(|e| { - ToolError::ExecutionError(format!("Failed to get text: {}", e)) - })?; - (text.into_bytes(), "txt") - } - "json" => { - let text = response.text().await.map_err(|e| { - ToolError::ExecutionError(format!("Failed to get text: {}", e)) - })?; - // Verify it's valid JSON - serde_json::from_str::(&text).map_err(|e| { - ToolError::ExecutionError(format!("Invalid JSON response: {}", e)) - })?; - (text.into_bytes(), "json") - } - "binary" => { - let bytes = response.bytes().await.map_err(|e| { - ToolError::ExecutionError(format!("Failed to get bytes: {}", e)) - })?; - (bytes.to_vec(), "bin") - } - _ => { - return Err(ToolError::InvalidParameters(format!( - "Invalid 'save_as' parameter: {}. Valid options are: 'text', 'json', 'binary'", - save_as - ))); - } - }; + let (content, extension) = match save_as { + "text" => { + let text = response.text().await.map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get text: {}", e)), + data: None, + })?; + (text.into_bytes(), "txt") + } + "json" => { + let text = response.text().await.map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get text: {}", e)), + data: None, + })?; + // Verify it's valid JSON + serde_json::from_str::(&text).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Invalid JSON response: {}", e)), + data: None, + })?; + (text.into_bytes(), "json") + } + "binary" => { + let bytes = response.bytes().await.map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get bytes: {}", e)), + data: None, + })?; + (bytes.to_vec(), "bin") + } + _ => { + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Invalid 'save_as' parameter: {}. Valid options are: 'text', 'json', 'binary'", + save_as + )), + data: None, + }); + } + }; // Save to cache let cache_path = self.save_to_cache(&content, "web", extension).await?; @@ -665,16 +696,24 @@ impl ComputerControllerRouter { } // Implement quick_script tool functionality - async fn quick_script(&self, params: Value) -> Result, ToolError> { + async fn quick_script(&self, params: Value) -> Result, ErrorData> { let language = params .get("language") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'language' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'language' parameter"), + data: None, + })?; let script = params .get("script") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'script' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'script' parameter"), + data: None, + })?; let save_output = params .get("save_output") @@ -682,8 +721,10 @@ impl ComputerControllerRouter { .unwrap_or(false); // Create a temporary directory for the script - let script_dir = tempfile::tempdir().map_err(|e| { - ToolError::ExecutionError(format!("Failed to create temporary directory: {}", e)) + let script_dir = tempfile::tempdir().map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to create temporary directory: {}", e)), + data: None, })?; let (shell, shell_arg) = self.system_automation.get_shell_command(); @@ -694,24 +735,27 @@ impl ComputerControllerRouter { "script.{}", if cfg!(windows) { "bat" } else { "sh" } )); - fs::write(&script_path, script).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write script: {}", e)) + fs::write(&script_path, script).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write script: {}", e)), + data: None, })?; // Set execute permissions on Unix systems #[cfg(unix)] { let mut perms = fs::metadata(&script_path) - .map_err(|e| { - ToolError::ExecutionError(format!("Failed to get file metadata: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get file metadata: {}", e)), + data: None, })? .permissions(); perms.set_mode(0o755); // rwxr-xr-x - fs::set_permissions(&script_path, perms).map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to set execute permissions: {}", - e - )) + fs::set_permissions(&script_path, perms).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to set execute permissions: {}", e)), + data: None, })?; } @@ -719,24 +763,30 @@ impl ComputerControllerRouter { } "ruby" => { let script_path = script_dir.path().join("script.rb"); - fs::write(&script_path, script).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write script: {}", e)) + fs::write(&script_path, script).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write script: {}", e)), + data: None, })?; format!("ruby {}", script_path.display()) } "powershell" => { let script_path = script_dir.path().join("script.ps1"); - fs::write(&script_path, script).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write script: {}", e)) + fs::write(&script_path, script).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to write script: {}", e)), + data: None, })?; script_path.display().to_string() } _ => { - return Err( ToolError::InvalidParameters( - format!("Invalid 'language' parameter: {}. Valid options are: 'shell', 'batch', 'ruby', 'powershell", language) - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid 'language' parameter: {}. Valid options are: 'shell', 'batch', 'ruby', 'powershell'", language)), + data: None, + }); } }; @@ -752,8 +802,10 @@ impl ComputerControllerRouter { .env("GOOSE_TERMINAL", "1") .output() .await - .map_err(|e| { - ToolError::ExecutionError(format!("Failed to run script: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to run script: {}", e)), + data: None, })? } _ => Command::new(shell) @@ -762,7 +814,11 @@ impl ComputerControllerRouter { .env("GOOSE_TERMINAL", "1") .output() .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to run script: {}", e)))?, + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to run script: {}", e)), + data: None, + })?, }; let output_str = String::from_utf8_lossy(&output.stdout).into_owned(); @@ -792,11 +848,15 @@ impl ComputerControllerRouter { } // Implement computer control functionality - async fn computer_control(&self, params: Value) -> Result, ToolError> { + async fn computer_control(&self, params: Value) -> Result, ErrorData> { let script = params .get("script") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'script' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'script' parameter"), + data: None, + })?; let save_output = params .get("save_output") @@ -807,7 +867,11 @@ impl ComputerControllerRouter { let output = self .system_automation .execute_system_script(script) - .map_err(|e| ToolError::ExecutionError(format!("Failed to execute script: {}", e)))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute script: {}", e)), + data: None, + })?; let mut result = format!("Script completed successfully.\n\nOutput:\n{}", output); @@ -825,71 +889,110 @@ impl ComputerControllerRouter { Ok(vec![Content::text(result)]) } - async fn xlsx_tool(&self, params: Value) -> Result, ToolError> { + async fn xlsx_tool(&self, params: Value) -> Result, ErrorData> { let path = params .get("path") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'path' parameter"), + data: None, + })?; let operation = params .get("operation") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'operation' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'operation' parameter"), + data: None, + })?; match operation { "list_worksheets" => { - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; - let worksheets = xlsx - .list_worksheets() - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; + let worksheets = xlsx.list_worksheets().map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!("{:#?}", worksheets))]) } "get_columns" => { - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; let worksheet = if let Some(name) = params.get("worksheet").and_then(|v| v.as_str()) { - xlsx.get_worksheet_by_name(name) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_name(name).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? } else { - xlsx.get_worksheet_by_index(0) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_index(0).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? }; - let columns = xlsx - .get_column_names(worksheet) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let columns = xlsx.get_column_names(worksheet).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!("{:#?}", columns))]) } "get_range" => { let range = params .get("range") .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'range' parameter".into()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'range' parameter"), + data: None, })?; - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; let worksheet = if let Some(name) = params.get("worksheet").and_then(|v| v.as_str()) { - xlsx.get_worksheet_by_name(name) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_name(name).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? } else { - xlsx.get_worksheet_by_index(0) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_index(0).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? }; - let range_data = xlsx - .get_range(worksheet, range) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let range_data = xlsx.get_range(worksheet, range).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!("{:#?}", range_data))]) } "find_text" => { let search_text = params .get("search_text") .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'search_text' parameter".into()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'search_text' parameter"), + data: None, })?; let case_sensitive = params @@ -897,19 +1000,32 @@ impl ComputerControllerRouter { .and_then(|v| v.as_bool()) .unwrap_or(false); - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; let worksheet = if let Some(name) = params.get("worksheet").and_then(|v| v.as_str()) { - xlsx.get_worksheet_by_name(name) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_name(name).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? } else { - xlsx.get_worksheet_by_index(0) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_index(0).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? }; let matches = xlsx .find_in_worksheet(worksheet, search_text, case_sensitive) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!( "Found matches at: {:#?}", matches @@ -925,66 +1041,114 @@ impl ComputerControllerRouter { .and_then(|v| v.as_str()) .unwrap_or("Sheet1"); - let mut xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let mut xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; xlsx.update_cell(worksheet_name, row as u32, col as u32, value) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; - xlsx.save(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; + xlsx.save(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!( "Updated cell ({}, {}) to '{}' in worksheet '{}'", row, col, value, worksheet_name ))]) } "save" => { - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; - xlsx.save(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; + xlsx.save(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text("File saved successfully.")]) } "get_cell" => { - let row = params.get("row").and_then(|v| v.as_u64()).ok_or_else(|| { - ToolError::InvalidParameters("Missing 'row' parameter".into()) - })?; + let row = params + .get("row") + .and_then(|v| v.as_u64()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'row' parameter"), + data: None, + })?; - let col = params.get("col").and_then(|v| v.as_u64()).ok_or_else(|| { - ToolError::InvalidParameters("Missing 'col' parameter".into()) - })?; + let col = params + .get("col") + .and_then(|v| v.as_u64()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'col' parameter"), + data: None, + })?; - let xlsx = xlsx_tool::XlsxTool::new(path) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + let xlsx = xlsx_tool::XlsxTool::new(path).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; let worksheet = if let Some(name) = params.get("worksheet").and_then(|v| v.as_str()) { - xlsx.get_worksheet_by_name(name) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_name(name).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? } else { - xlsx.get_worksheet_by_index(0) - .map_err(|e| ToolError::ExecutionError(e.to_string()))? + xlsx.get_worksheet_by_index(0).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })? }; let cell_value = xlsx .get_cell_value(worksheet, row as u32, col as u32) - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })?; Ok(vec![Content::text(format!("{:#?}", cell_value))]) } - _ => Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}", - operation - ))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid operation: {}", operation)), + data: None, + }), } } // Implement cache tool functionality - async fn docx_tool(&self, params: Value) -> Result, ToolError> { + async fn docx_tool(&self, params: Value) -> Result, ErrorData> { let path = params .get("path") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'path' parameter"), + data: None, + })?; let operation = params .get("operation") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'operation' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'operation' parameter"), + data: None, + })?; crate::computercontroller::docx_tool::docx_tool( path, @@ -995,34 +1159,50 @@ impl ComputerControllerRouter { .await } - async fn pdf_tool(&self, params: Value) -> Result, ToolError> { + async fn pdf_tool(&self, params: Value) -> Result, ErrorData> { let path = params .get("path") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'path' parameter"), + data: None, + })?; let operation = params .get("operation") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'operation' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'operation' parameter"), + data: None, + })?; crate::computercontroller::pdf_tool::pdf_tool(path, operation, &self.cache_dir).await } - async fn cache(&self, params: Value) -> Result, ToolError> { + async fn cache(&self, params: Value) -> Result, ErrorData> { let command = params .get("command") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'command' parameter".into()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'command' parameter"), + data: None, + })?; match command { "list" => { let mut files = Vec::new(); - for entry in fs::read_dir(&self.cache_dir).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read cache directory: {}", e)) + for entry in fs::read_dir(&self.cache_dir).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read cache directory: {}", e)), + data: None, })? { - let entry = entry.map_err(|e| { - ToolError::ExecutionError(format!("Failed to read directory entry: {}", e)) + let entry = entry.map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read directory entry: {}", e)), + data: None, })?; files.push(format!("{}", entry.path().display())); } @@ -1033,13 +1213,19 @@ impl ComputerControllerRouter { ))]) } "view" => { - let path = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| { - ToolError::InvalidParameters("Missing 'path' parameter for view".into()) - })?; + let path = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| + ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'path' parameter for view"), + data: None, + })?; - let content = fs::read_to_string(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to read file: {}", e)) - })?; + let content = fs::read_to_string(path).map_err(|e| + ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to read file: {}", e)), + data: None, + })?; Ok(vec![Content::text(format!( "Content of {}:\n\n{}", @@ -1047,13 +1233,19 @@ impl ComputerControllerRouter { ))]) } "delete" => { - let path = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| { - ToolError::InvalidParameters("Missing 'path' parameter for delete".into()) - })?; + let path = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| + ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'path' parameter for delete"), + data: None, + })?; - fs::remove_file(path).map_err(|e| { - ToolError::ExecutionError(format!("Failed to delete file: {}", e)) - })?; + fs::remove_file(path).map_err(|e| + ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to delete file: {}", e)), + data: None, + })?; // Remove from active resources if present if let Ok(url) = Url::from_file_path(path) { @@ -1066,22 +1258,31 @@ impl ComputerControllerRouter { Ok(vec![Content::text(format!("Deleted file: {}", path))]) } "clear" => { - fs::remove_dir_all(&self.cache_dir).map_err(|e| { - ToolError::ExecutionError(format!("Failed to clear cache directory: {}", e)) - })?; - fs::create_dir_all(&self.cache_dir).map_err(|e| { - ToolError::ExecutionError(format!("Failed to recreate cache directory: {}", e)) - })?; + fs::remove_dir_all(&self.cache_dir).map_err(|e| + ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to clear cache directory: {}", e)), + data: None, + })?; + fs::create_dir_all(&self.cache_dir).map_err(|e| + ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to recreate cache directory: {}", e)), + data: None, + })?; // Clear active resources self.active_resources.lock().unwrap().clear(); Ok(vec![Content::text("Cache cleared successfully.")]) } - _ => Err(ToolError::InvalidParameters(format!( + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( "Invalid 'command' parameter: {}. Valid options are: 'list', 'view', 'delete', 'clear'", - command - ))) + command)), + data: None, + }), } } } @@ -1111,7 +1312,7 @@ impl Router for ComputerControllerRouter { tool_name: &str, arguments: Value, _notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); Box::pin(async move { @@ -1123,7 +1324,11 @@ impl Router for ComputerControllerRouter { "pdf_tool" => this.pdf_tool(arguments).await, "docx_tool" => this.docx_tool(arguments).await, "xlsx_tool" => this.xlsx_tool(arguments).await, - _ => Err(ToolError::NotFound(format!("Tool {} not found", tool_name))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!("Tool {} not found", tool_name)), + data: None, + }), } }) } diff --git a/crates/goose-mcp/src/computercontroller/pdf_tool.rs b/crates/goose-mcp/src/computercontroller/pdf_tool.rs index 424d1f9e190f..28834b6dc291 100644 --- a/crates/goose-mcp/src/computercontroller/pdf_tool.rs +++ b/crates/goose-mcp/src/computercontroller/pdf_tool.rs @@ -1,16 +1,20 @@ use lopdf::{content::Content as PdfContent, Document, Object}; -use mcp_core::ToolError; -use rmcp::model::Content; +use rmcp::model::{Content, ErrorCode, ErrorData}; use std::{fs, path::Path}; pub async fn pdf_tool( path: &str, operation: &str, cache_dir: &Path, -) -> Result, ToolError> { +) -> Result, ErrorData> { // Open and parse the PDF file - let doc = Document::load(path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to open PDF file: {}", e)))?; + let doc = Document::load(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to open PDF file: {}", e), + None, + ) + })?; let result = match operation { "extract_text" => { @@ -115,7 +119,11 @@ pub async fn pdf_tool( "extract_images" => { let cache_dir = cache_dir.join("pdf_images"); fs::create_dir_all(&cache_dir).map_err(|e| { - ToolError::ExecutionError(format!("Failed to create image cache directory: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to create image cache directory: {}", e), + None, + ) })?; let mut images = Vec::new(); @@ -167,14 +175,19 @@ pub async fn pdf_tool( // Process each page for (page_num, page_id) in doc.get_pages() { let page = doc.get_object(page_id).map_err(|e| { - ToolError::ExecutionError(format!("Failed to get page {}: {}", page_num, e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get page {}: {}", page_num, e), + None, + ) })?; let page_dict = page.as_dict().map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to get page dict {}: {}", - page_num, e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get page dict {}: {}", page_num, e), + None, + ) })?; // Get page resources - handle both direct dict and reference @@ -184,27 +197,32 @@ pub async fn pdf_tool( Object::Reference(id) => doc .get_object(*id) .map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to get resource reference: {}", - e - )) + ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + format!("Failed to get resource reference: {}", e), + None, + ) }) .and_then(|obj| { obj.as_dict().map_err(|e| { - ToolError::ExecutionError(format!( - "Resource reference is not a dictionary: {}", - e - )) + ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + format!("Resource reference is not a dictionary: {}", e), + None, + ) }) }), - _ => Err(ToolError::ExecutionError( + _ => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "Resources is neither dictionary nor reference".to_string(), + None, )), }, - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to get Resources: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get Resources: {}", e), + None, + )), }?; // Look for XObject dictionary - handle both direct dict and reference @@ -214,37 +232,50 @@ pub async fn pdf_tool( Object::Reference(id) => doc .get_object(*id) .map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to get XObject reference: {}", - e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get XObject reference: {}", e), + None, + ) }) .and_then(|obj| { obj.as_dict().map_err(|e| { - ToolError::ExecutionError(format!( - "XObject reference is not a dictionary: {}", - e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("XObject reference is not a dictionary: {}", e), + None, + ) }) }), - _ => Err(ToolError::ExecutionError( + _ => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "XObject is neither dictionary nor reference".to_string(), + None, )), }, - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to get XObject: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get XObject: {}", e), + None, + )), }; if let Ok(xobjects) = xobjects { for (name, xobject) in xobjects.iter() { let xobject_id = xobject.as_reference().map_err(|_| { - ToolError::ExecutionError("Failed to get XObject reference".to_string()) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Failed to get XObject reference".to_string(), + None, + ) })?; let xobject = doc.get_object(xobject_id).map_err(|e| { - ToolError::ExecutionError(format!("Failed to get XObject: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get XObject: {}", e), + None, + ) })?; if let Ok(stream) = xobject.as_stream() { @@ -283,10 +314,11 @@ pub async fn pdf_tool( )); fs::write(&image_path, &data).map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to write image: {}", - e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write image: {}", e), + None, + ) })?; images.push(format!( @@ -313,10 +345,14 @@ pub async fn pdf_tool( } _ => { - return Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}. Valid operations are: 'extract_text', 'extract_images'", - operation - ))) + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Invalid operation: {}. Valid operations are: 'extract_text', 'extract_images'", + operation + ), + None, + )) } }; diff --git a/crates/goose-mcp/src/developer/mod.rs b/crates/goose-mcp/src/developer/mod.rs index e5cded56faa8..8b986d9fdb06 100644 --- a/crates/goose-mcp/src/developer/mod.rs +++ b/crates/goose-mcp/src/developer/mod.rs @@ -1,4 +1,5 @@ mod editor_models; + mod lang; mod shell; @@ -26,17 +27,16 @@ use url::Url; use include_dir::{include_dir, Dir}; use mcp_core::{ - handler::{require_str_parameter, PromptError, ResourceError, ToolError}, + handler::{require_str_parameter, PromptError, ResourceError}, protocol::ServerCapabilities, }; -use once_cell::sync::Lazy; - use mcp_server::router::CapabilitiesBuilder; use mcp_server::Router; +use once_cell::sync::Lazy; use rmcp::model::{ - Content, JsonRpcMessage, JsonRpcNotification, JsonRpcVersion2_0, Notification, Prompt, - PromptArgument, Resource, Role, Tool, ToolAnnotations, + Content, ErrorCode, ErrorData, JsonRpcMessage, JsonRpcNotification, JsonRpcVersion2_0, + Notification, Prompt, PromptArgument, Resource, Role, Tool, ToolAnnotations, }; use rmcp::object; @@ -737,7 +737,7 @@ impl DeveloperRouter { } // shell output can be large, this will help manage that - fn process_shell_output(&self, output_str: &str) -> Result<(String, String), ToolError> { + fn process_shell_output(&self, output_str: &str) -> Result<(String, String), ErrorData> { let lines: Vec<&str> = output_str.lines().collect(); let line_count = lines.len(); @@ -746,15 +746,27 @@ impl DeveloperRouter { let final_output = if line_count > 100 { let tmp_file = tempfile::NamedTempFile::new().map_err(|e| { - ToolError::ExecutionError(format!("Failed to create temporary file: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to create temporary file: {}", e), + None, + ) })?; std::fs::write(tmp_file.path(), output_str).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write to temporary file: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write to temporary file: {}", e), + None, + ) })?; let (_, path) = tmp_file.keep().map_err(|e| { - ToolError::ExecutionError(format!("Failed to persist temporary file: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to persist temporary file: {}", e), + None, + ) })?; format!( @@ -777,7 +789,7 @@ impl DeveloperRouter { } // Helper method to resolve a path relative to cwd with platform-specific handling - fn resolve_path(&self, path_str: &str) -> Result { + fn resolve_path(&self, path_str: &str) -> Result { let cwd = std::env::current_dir().expect("should have a current working dir"); let expanded = expand_path(path_str); let path = Path::new(&expanded); @@ -786,11 +798,15 @@ impl DeveloperRouter { match is_absolute_path(&expanded) { true => Ok(path.to_path_buf()), - false => Err(ToolError::InvalidParameters(format!( - "The path {} is not an absolute path, did you possibly mean {}?", - path_str, - suggestion.to_string_lossy(), - ))), + false => Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "The path {} is not an absolute path, did you possibly mean {}?", + path_str, + suggestion.to_string_lossy(), + ), + None, + )), } } @@ -799,14 +815,17 @@ impl DeveloperRouter { &self, params: Value, notifier: mpsc::Sender, - ) -> Result, ToolError> { - let command = - params - .get("command") - .and_then(|v| v.as_str()) - .ok_or(ToolError::InvalidParameters( + ) -> Result, ErrorData> { + let command = params + .get("command") + .and_then(|v| v.as_str()) + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, "The command string is required".to_string(), - ))?; + None, + ) + })?; // Check if command might access ignored files and return early if it does let cmd_parts: Vec<&str> = command.split_whitespace().collect(); @@ -822,10 +841,14 @@ impl DeveloperRouter { } if self.is_ignored(path) { - return Err(ToolError::ExecutionError(format!( - "The command attempts to access '{}' which is restricted by .gooseignore", - arg - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "The command attempts to access '{}' which is restricted by .gooseignore", + arg + ), + None, + )); } } @@ -842,7 +865,7 @@ impl DeveloperRouter { .args(&shell_config.args) .arg(command) .spawn() - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None))?; let stdout = BufReader::new(child.stdout.take().unwrap()); let stderr = BufReader::new(child.stderr.take().unwrap()); @@ -890,23 +913,30 @@ impl DeveloperRouter { child .wait() .await - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None))?; let output_str = match output_task.await { - Ok(result) => result.map_err(|e| ToolError::ExecutionError(e.to_string()))?, - Err(e) => return Err(ToolError::ExecutionError(e.to_string())), + Ok(result) => result + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None))?, + Err(e) => { + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + e.to_string(), + None, + )) + } }; // Check the character count of the output const MAX_CHAR_COUNT: usize = 400_000; // 409600 chars = 400KB let char_count = output_str.chars().count(); if char_count > MAX_CHAR_COUNT { - return Err(ToolError::ExecutionError(format!( + return Err(ErrorData::new(ErrorCode::INTERNAL_ERROR, format!( "Shell output from command '{}' has too many characters ({}). Maximum character count is {}.", command, char_count, MAX_CHAR_COUNT - ))); + ), None)); } let (final_output, user_output) = self.process_shell_output(&output_str)?; @@ -919,27 +949,39 @@ impl DeveloperRouter { ]) } - async fn text_editor(&self, params: Value) -> Result, ToolError> { + #[allow(clippy::too_many_lines)] + async fn text_editor(&self, params: Value) -> Result, ErrorData> { let command = params .get("command") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'command' parameter".to_string()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'command' parameter".to_string(), + None, + ) })?; - let path_str = params - .get("path") - .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?; + let path_str = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'path' parameter".to_string(), + None, + ) + })?; let path = self.resolve_path(path_str)?; // Check if file is ignored before proceeding with any text editor operation if self.is_ignored(&path) { - return Err(ToolError::ExecutionError(format!( - "Access to '{}' is restricted by .gooseignore", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Access to '{}' is restricted by .gooseignore", + path.display() + ), + None, + )); } match command { @@ -968,13 +1010,21 @@ impl DeveloperRouter { .get("old_str") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'old_str' parameter".into()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'old_str' parameter".to_string(), + None, + ) })?; let new_str = params .get("new_str") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'new_str' parameter".into()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'new_str' parameter".to_string(), + None, + ) })?; self.text_editor_replace(&path, old_str, new_str).await @@ -984,22 +1034,31 @@ impl DeveloperRouter { .get("insert_line") .and_then(|v| v.as_i64()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'insert_line' parameter".into()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'insert_line' parameter".to_string(), + None, + ) })? as usize; let new_str = params .get("new_str") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'new_str' parameter".into()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'new_str' parameter".to_string(), + None, + ) })?; self.text_editor_insert(&path, insert_line, new_str).await } "undo_edit" => self.text_editor_undo(&path).await, - _ => Err(ToolError::InvalidParameters(format!( - "Unknown command '{}'", - command - ))), + _ => Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("Unknown command '{}'", command), + None, + )), } } @@ -1008,7 +1067,7 @@ impl DeveloperRouter { &self, view_range: Option<(usize, i64)>, total_lines: usize, - ) -> Result<(usize, usize), ToolError> { + ) -> Result<(usize, usize), ErrorData> { if let Some((start_line, end_line)) = view_range { // Convert 1-indexed line numbers to 0-indexed let start_idx = if start_line > 0 { start_line - 1 } else { 0 }; @@ -1019,17 +1078,25 @@ impl DeveloperRouter { }; if start_idx >= total_lines { - return Err(ToolError::InvalidParameters(format!( - "Start line {} is beyond the end of the file (total lines: {})", - start_line, total_lines - ))); + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Start line {} is beyond the end of the file (total lines: {})", + start_line, total_lines + ), + None, + )); } if start_idx >= end_idx { - return Err(ToolError::InvalidParameters(format!( - "Start line {} must be less than end line {}", - start_line, end_line - ))); + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Start line {} must be less than end line {}", + start_line, end_line + ), + None, + )); } Ok((start_idx, end_idx)) @@ -1091,42 +1158,72 @@ impl DeveloperRouter { &self, path: &PathBuf, view_range: Option<(usize, i64)>, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { if !path.is_file() { - return Err(ToolError::ExecutionError(format!( - "The path '{}' does not exist or is not a file.", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "The path '{}' does not exist or is not a file.", + path.display() + ), + None, + )); } const MAX_FILE_SIZE: u64 = 400 * 1024; // 400KB - let f = File::open(path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to open file: {}", e)))?; + let f = File::open(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to open file: {}", e), + None, + ) + })?; let file_size = f .metadata() - .map_err(|e| ToolError::ExecutionError(format!("Failed to get file metadata: {}", e)))? + .map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get file metadata: {}", e), + None, + ) + })? .len(); if file_size > MAX_FILE_SIZE { - return Err(ToolError::ExecutionError(format!( + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( "File '{}' is too large ({:.2}KB). Maximum size is 400KB to prevent memory issues.", path.display(), file_size as f64 / 1024.0 - ))); + ), + None, + )); } // Ensure we never read over that limit even if the file is being concurrently mutated let mut f = f.take(MAX_FILE_SIZE); let uri = Url::from_file_path(path) - .map_err(|_| ToolError::ExecutionError("Invalid file path".into()))? + .map_err(|_| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Invalid file path".to_string(), + None, + ) + })? .to_string(); let mut content = String::new(); - f.read_to_string(&mut content) - .map_err(|e| ToolError::ExecutionError(format!("Failed to read file: {}", e)))?; + f.read_to_string(&mut content).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read file: {}", e), + None, + ) + })?; let lines: Vec<&str> = content.lines().collect(); let total_lines = lines.len(); @@ -1154,7 +1251,7 @@ impl DeveloperRouter { &self, path: &PathBuf, file_text: &str, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { // Normalize line endings based on platform let mut normalized_text = normalize_line_endings(file_text); // Make mutable @@ -1165,7 +1262,13 @@ impl DeveloperRouter { // Write to the file std::fs::write(path, &normalized_text) // Write the potentially modified text - .map_err(|e| ToolError::ExecutionError(format!("Failed to write file: {}", e)))?; + .map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) + })?; // Try to detect the language from the file extension let language = lang::get_language_identifier(path); @@ -1191,23 +1294,33 @@ impl DeveloperRouter { ]) } + #[allow(clippy::too_many_lines)] async fn text_editor_replace( &self, path: &PathBuf, old_str: &str, new_str: &str, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { // Check if file exists and is active if !path.exists() { - return Err(ToolError::InvalidParameters(format!( - "File '{}' does not exist, you can write a new file with the `write` command", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "File '{}' does not exist, you can write a new file with the `write` command", + path.display() + ), + None, + )); } // Read content - let content = std::fs::read_to_string(path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to read file: {}", e)))?; + let content = std::fs::read_to_string(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read file: {}", e), + None, + ) + })?; // Check if Editor API is configured and use it as the primary path if let Some(ref editor) = self.editor_model { @@ -1219,7 +1332,11 @@ impl DeveloperRouter { // Write the updated content directly let normalized_content = normalize_line_endings(&updated_content); std::fs::write(path, &normalized_content).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write file: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) })?; // Simple success message for Editor API @@ -1244,15 +1361,15 @@ impl DeveloperRouter { // Traditional string replacement path (original logic) // Ensure 'old_str' appears exactly once if content.matches(old_str).count() > 1 { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "'old_str' must appear exactly once in the file, but it appears multiple times" - .into(), + .to_string(), + None, )); } if content.matches(old_str).count() == 0 { - return Err(ToolError::InvalidParameters( - "'old_str' must appear exactly once in the file, but it does not appear in the file. Make sure the string exactly matches existing file content, including whitespace!".into(), - )); + return Err(ErrorData::new(ErrorCode::INVALID_PARAMS, "'old_str' must appear exactly once in the file, but it does not appear in the file. Make sure the string exactly matches existing file content, including whitespace!".to_string(), None)); } // Save history for undo (original behavior - after validation) @@ -1260,8 +1377,13 @@ impl DeveloperRouter { let new_content = content.replace(old_str, new_str); let normalized_content = normalize_line_endings(&new_content); - std::fs::write(path, &normalized_content) - .map_err(|e| ToolError::ExecutionError(format!("Failed to write file: {}", e)))?; + std::fs::write(path, &normalized_content).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) + })?; // Try to detect the language from the file extension let language = lang::get_language_identifier(path); @@ -1322,18 +1444,27 @@ impl DeveloperRouter { path: &PathBuf, insert_line: usize, new_str: &str, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { // Check if file exists if !path.exists() { - return Err(ToolError::InvalidParameters(format!( - "File '{}' does not exist, you can write a new file with the `write` command", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "File '{}' does not exist, you can write a new file with the `write` command", + path.display() + ), + None, + )); } // Read content - let content = std::fs::read_to_string(path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to read file: {}", e)))?; + let content = std::fs::read_to_string(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read file: {}", e), + None, + ) + })?; // Save history for undo self.save_file_history(path)?; @@ -1343,10 +1474,10 @@ impl DeveloperRouter { // Validate insert_line parameter if insert_line > total_lines { - return Err(ToolError::InvalidParameters(format!( + return Err(ErrorData::new(ErrorCode::INVALID_PARAMS, format!( "Insert line {} is beyond the end of the file (total lines: {}). Use 0 to insert at the beginning or {} to insert at the end.", insert_line, total_lines, total_lines - ))); + ), None)); } // Create new content with inserted text @@ -1376,8 +1507,13 @@ impl DeveloperRouter { normalized_content }; - std::fs::write(path, &final_content) - .map_err(|e| ToolError::ExecutionError(format!("Failed to write file: {}", e)))?; + std::fs::write(path, &final_content).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) + })?; // Try to detect the language from the file extension let language = lang::get_language_identifier(path); @@ -1426,32 +1562,45 @@ impl DeveloperRouter { ]) } - async fn text_editor_undo(&self, path: &PathBuf) -> Result, ToolError> { + async fn text_editor_undo(&self, path: &PathBuf) -> Result, ErrorData> { let mut history = self.file_history.lock().unwrap(); if let Some(contents) = history.get_mut(path) { if let Some(previous_content) = contents.pop() { // Write previous content back to file std::fs::write(path, previous_content).map_err(|e| { - ToolError::ExecutionError(format!("Failed to write file: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) })?; Ok(vec![Content::text("Undid the last edit")]) } else { - Err(ToolError::InvalidParameters( - "No edit history available to undo".into(), + Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "No edit history available to undo".to_string(), + None, )) } } else { - Err(ToolError::InvalidParameters( - "No edit history available to undo".into(), + Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "No edit history available to undo".to_string(), + None, )) } } - fn save_file_history(&self, path: &PathBuf) -> Result<(), ToolError> { + fn save_file_history(&self, path: &PathBuf) -> Result<(), ErrorData> { let mut history = self.file_history.lock().unwrap(); let content = if path.exists() { - std::fs::read_to_string(path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to read file: {}", e)))? + std::fs::read_to_string(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read file: {}", e), + None, + ) + })? } else { String::new() }; @@ -1459,9 +1608,14 @@ impl DeveloperRouter { Ok(()) } - async fn list_windows(&self, _params: Value) -> Result, ToolError> { - let windows = Window::all() - .map_err(|_| ToolError::ExecutionError("Failed to list windows".into()))?; + async fn list_windows(&self, _params: Value) -> Result, ErrorData> { + let windows = Window::all().map_err(|_| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Failed to list windows".to_string(), + None, + ) + })?; let window_titles: Vec = windows.into_iter().map(|w| w.title().to_string()).collect(); @@ -1511,11 +1665,14 @@ impl DeveloperRouter { path.to_path_buf() } - async fn image_processor(&self, params: Value) -> Result, ToolError> { - let path_str = params - .get("path") - .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?; + async fn image_processor(&self, params: Value) -> Result, ErrorData> { + let path_str = params.get("path").and_then(|v| v.as_str()).ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'path' parameter".to_string(), + None, + ) + })?; let path = { let p = self.resolve_path(path_str)?; @@ -1528,37 +1685,57 @@ impl DeveloperRouter { // Check if file is ignored before proceeding if self.is_ignored(&path) { - return Err(ToolError::ExecutionError(format!( - "Access to '{}' is restricted by .gooseignore", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Access to '{}' is restricted by .gooseignore", + path.display() + ), + None, + )); } // Check if file exists if !path.exists() { - return Err(ToolError::ExecutionError(format!( - "File '{}' does not exist", - path.display() - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("File '{}' does not exist", path.display()), + None, + )); } // Check file size (10MB limit for image files) const MAX_FILE_SIZE: u64 = 10 * 1024 * 1024; // 10MB in bytes let file_size = std::fs::metadata(&path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to get file metadata: {}", e)))? + .map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get file metadata: {}", e), + None, + ) + })? .len(); if file_size > MAX_FILE_SIZE { - return Err(ToolError::ExecutionError(format!( - "File '{}' is too large ({:.2}MB). Maximum size is 10MB.", - path.display(), - file_size as f64 / (1024.0 * 1024.0) - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "File '{}' is too large ({:.2}MB). Maximum size is 10MB.", + path.display(), + file_size as f64 / (1024.0 * 1024.0) + ), + None, + )); } // Open and decode the image - let image = xcap::image::open(&path) - .map_err(|e| ToolError::ExecutionError(format!("Failed to open image file: {}", e)))?; + let image = xcap::image::open(&path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to open image file: {}", e), + None, + ) + })?; // Resize if necessary (same logic as screen_capture) let mut processed_image = image; @@ -1579,7 +1756,11 @@ impl DeveloperRouter { processed_image .write_to(&mut Cursor::new(&mut bytes), xcap::image::ImageFormat::Png) .map_err(|e| { - ToolError::ExecutionError(format!("Failed to write image buffer: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write image buffer: {}", e), + None, + ) })?; let data = base64::prelude::BASE64_STANDARD.encode(bytes); @@ -1594,48 +1775,67 @@ impl DeveloperRouter { ]) } - async fn screen_capture(&self, params: Value) -> Result, ToolError> { - let mut image = if let Some(window_title) = - params.get("window_title").and_then(|v| v.as_str()) - { - // Try to find and capture the specified window - let windows = Window::all() - .map_err(|_| ToolError::ExecutionError("Failed to list windows".into()))?; - - let window = windows - .into_iter() - .find(|w| w.title() == window_title) - .ok_or_else(|| { - ToolError::ExecutionError(format!( - "No window found with title '{}'", - window_title - )) + async fn screen_capture(&self, params: Value) -> Result, ErrorData> { + let mut image = + if let Some(window_title) = params.get("window_title").and_then(|v| v.as_str()) { + // Try to find and capture the specified window + let windows = Window::all().map_err(|_| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Failed to list windows".to_string(), + None, + ) })?; - window.capture_image().map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to capture window '{}': {}", - window_title, e - )) - })? - } else { - // Default to display capture if no window title is specified - let display = params.get("display").and_then(|v| v.as_u64()).unwrap_or(0) as usize; - - let monitors = Monitor::all() - .map_err(|_| ToolError::ExecutionError("Failed to access monitors".into()))?; - let monitor = monitors.get(display).ok_or_else(|| { - ToolError::ExecutionError(format!( - "{} was not an available monitor, {} found.", - display, - monitors.len() - )) - })?; + let window = windows + .into_iter() + .find(|w| w.title() == window_title) + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("No window found with title '{}'", window_title), + None, + ) + })?; - monitor.capture_image().map_err(|e| { - ToolError::ExecutionError(format!("Failed to capture display {}: {}", display, e)) - })? - }; + window.capture_image().map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to capture window '{}': {}", window_title, e), + None, + ) + })? + } else { + // Default to display capture if no window title is specified + let display = params.get("display").and_then(|v| v.as_u64()).unwrap_or(0) as usize; + + let monitors = Monitor::all().map_err(|_| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Failed to access monitors".to_string(), + None, + ) + })?; + let monitor = monitors.get(display).ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "{} was not an available monitor, {} found.", + display, + monitors.len() + ), + None, + ) + })?; + + monitor.capture_image().map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to capture display {}: {}", display, e), + None, + ) + })? + }; // Resize the image to a reasonable width while maintaining aspect ratio let max_width = 768; @@ -1654,7 +1854,11 @@ impl DeveloperRouter { image .write_to(&mut Cursor::new(&mut bytes), xcap::image::ImageFormat::Png) .map_err(|e| { - ToolError::ExecutionError(format!("Failed to write image buffer {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write image buffer {}", e), + None, + ) })?; // Convert to base64 @@ -1667,13 +1871,13 @@ impl DeveloperRouter { } } -fn recommend_read_range(path: &Path, total_lines: usize) -> Result, ToolError> { - Err(ToolError::ExecutionError(format!( +fn recommend_read_range(path: &Path, total_lines: usize) -> Result, ErrorData> { + Err(ErrorData::new(ErrorCode::INTERNAL_ERROR, format!( "File '{}' is {} lines long, recommended to read in with view_range (or searching) to get bite size content. If you do wish to read all the file, please pass in view_range with [1, {}] to read it all at once", path.display(), total_lines, total_lines - ))) + ), None)) } impl Router for DeveloperRouter { @@ -1701,7 +1905,7 @@ impl Router for DeveloperRouter { tool_name: &str, arguments: Value, notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); Box::pin(async move { @@ -1711,7 +1915,11 @@ impl Router for DeveloperRouter { "list_windows" => this.list_windows(arguments).await, "screen_capture" => this.screen_capture(arguments).await, "image_processor" => this.image_processor(arguments).await, - _ => Err(ToolError::NotFound(format!("Tool {} not found", tool_name))), + _ => Err(ErrorData::new( + ErrorCode::METHOD_NOT_FOUND, + format!("Tool {} not found", tool_name), + None, + )), } }) } @@ -1865,7 +2073,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); temp_dir.close().unwrap(); } @@ -1961,7 +2169,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::ExecutionError(_))); + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); assert!(err.to_string().contains("too large")); } @@ -1987,7 +2195,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::ExecutionError(_))); + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); assert!(err.to_string().contains("is too large")); } @@ -2305,7 +2513,7 @@ mod tests { result.is_err(), "Should not be able to write to ignored file" ); - assert!(matches!(result.unwrap_err(), ToolError::ExecutionError(_))); + assert_eq!(result.unwrap_err().code, ErrorCode::INTERNAL_ERROR); // Try to write to a non-ignored file let result = router @@ -2364,7 +2572,7 @@ mod tests { .await; assert!(result.is_err(), "Should not be able to cat ignored file"); - assert!(matches!(result.unwrap_err(), ToolError::ExecutionError(_))); + assert_eq!(result.unwrap_err().code, ErrorCode::INTERNAL_ERROR); // Try to cat a non-ignored file let allowed_file_path = temp_dir.path().join("allowed.txt"); @@ -2547,7 +2755,7 @@ mod tests { result.is_err(), "Should not be able to write to file ignored by .gitignore fallback" ); - assert!(matches!(result.unwrap_err(), ToolError::ExecutionError(_))); + assert_eq!(result.unwrap_err().code, ErrorCode::INTERNAL_ERROR); // Try to write to a non-ignored file let result = router @@ -2600,7 +2808,7 @@ mod tests { result.is_err(), "Should not be able to cat file ignored by .gitignore fallback" ); - assert!(matches!(result.unwrap_err(), ToolError::ExecutionError(_))); + assert_eq!(result.unwrap_err().code, ErrorCode::INTERNAL_ERROR); // Try to cat a non-ignored file let allowed_file_path = temp_dir.path().join("allowed.txt"); @@ -2786,7 +2994,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("beyond the end of the file")); // Test invalid range - start >= end @@ -2804,7 +3012,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("must be less than end line")); temp_dir.close().unwrap(); @@ -3102,7 +3310,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("beyond the end of the file")); temp_dir.close().unwrap(); @@ -3147,7 +3355,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("Missing 'insert_line' parameter")); // Try insert without new_str parameter @@ -3165,7 +3373,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("Missing 'new_str' parameter")); temp_dir.close().unwrap(); @@ -3283,7 +3491,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::InvalidParameters(_))); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); assert!(err.to_string().contains("does not exist")); temp_dir.close().unwrap(); @@ -3332,7 +3540,7 @@ mod tests { assert!(result.is_err()); let err = result.err().unwrap(); - assert!(matches!(err, ToolError::ExecutionError(_))); + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); assert!(err.to_string().contains("2001 lines long")); assert!(err .to_string() diff --git a/crates/goose-mcp/src/google_drive/mod.rs b/crates/goose-mcp/src/google_drive/mod.rs index b3f235dd6ade..adaa297a7e77 100644 --- a/crates/goose-mcp/src/google_drive/mod.rs +++ b/crates/goose-mcp/src/google_drive/mod.rs @@ -8,7 +8,7 @@ use chrono::NaiveDate; use indoc::indoc; use lazy_static::lazy_static; use mcp_core::{ - handler::{PromptError, ResourceError, ToolError}, + handler::{PromptError, ResourceError}, protocol::ServerCapabilities, }; use mcp_server::router::CapabilitiesBuilder; @@ -16,10 +16,12 @@ use mcp_server::Router; use oauth_pkce::PkceOAuth2Client; use regex::Regex; use rmcp::model::{ - AnnotateAble, Content, JsonRpcMessage, Prompt, RawResource, Resource, Tool, ToolAnnotations, + AnnotateAble, Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, RawResource, Resource, + Tool, ToolAnnotations, }; use rmcp::object; use serde_json::{json, Value}; +use std::borrow::Cow; use std::io::Cursor; use std::{env, fs, future::Future, path::Path, pin::Pin, sync::Arc}; use storage::CredentialsManager; @@ -987,23 +989,32 @@ impl GoogleDriveRouter { } // Implement search tool functionality - async fn search(&self, params: Value) -> Result, ToolError> { + async fn search(&self, params: Value) -> Result, ErrorData> { // To minimize tool growth, we search/list for a number of different // objects in Gdrive with sub-funcs. - let drive_type = params.get("driveType").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The type is required".to_string()), - )?; + let drive_type = params + .get("driveType") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The type is required".to_string()), + data: None, + })?; match drive_type { "file" => return self.search_files(params).await, "label" => return self.list_labels(params).await, - t => Err(ToolError::InvalidParameters(format!( - "type must be one of ('file', 'label'), got {}", - t - ))), + t => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "type must be one of (\'file\', \'label\'), got {}", + t + )), + data: None, + }), } } - async fn search_files(&self, params: Value) -> Result, ToolError> { + async fn search_files(&self, params: Value) -> Result, ErrorData> { let name = params.get("name").and_then(|q| q.as_str()); let mime_type = params.get("mimeType").and_then(|q| q.as_str()); let drive_id = params.get("driveId").and_then(|q| q.as_str()); @@ -1017,10 +1028,14 @@ impl GoogleDriveRouter { if ["user", "drive", "allDrives"].contains(&s) { Ok(s) } else { - Err(ToolError::InvalidParameters(format!( - "corpora must be either 'user', 'drive', or 'allDrives', got {}", - s - ))) + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "corpora must be either 'user', 'drive', or 'allDrives', got {}", + s + )), + data: None, + }) } }) .unwrap_or(Ok("user"))?; @@ -1031,15 +1046,23 @@ impl GoogleDriveRouter { .map(|s| { s.as_i64() .and_then(|n| i32::try_from(n).ok()) - .ok_or_else(|| ToolError::InvalidParameters(format!("Invalid pageSize: {}", s))) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid pageSize: {}", s)), + data: None, + }) .and_then(|n| { if (0..=100).contains(&n) { Ok(n) } else { - Err(ToolError::InvalidParameters(format!( - "pageSize must be between 0 and 100, got {}", - n - ))) + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "pageSize must be between 0 and 100, got {}", + n + )), + data: None, + }) } }) }) @@ -1068,10 +1091,11 @@ impl GoogleDriveRouter { } let query_string = query.join(" and "); if query_string.is_empty() { - return Err(ToolError::InvalidParameters( - "No query provided. Please include one of ('name', 'mimeType', 'parent')." - .to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("No query provided. Please include one of (\'name\', \'mimeType\', \'parent\').".to_string()), + data: None + }); } let mut builder = self .drive @@ -1109,10 +1133,14 @@ impl GoogleDriveRouter { let label_results = match label_builder.doit().await { Ok(r) => r.1.labels.unwrap_or_default(), Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to execute google drive label list '{}'.", - e - ))) + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to execute google drive label list '{}'.", + e + )), + data: None, + }) } }; let label_ids = label_results @@ -1125,11 +1153,15 @@ impl GoogleDriveRouter { let result = builder.doit().await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute google drive search query '{}', {}.", - query_string.as_str(), - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to execute google drive search query '{}', {}.", + query_string.as_str(), + e + )), + data: None, + }), Ok(r) => { let content = r.1.files @@ -1158,7 +1190,7 @@ impl GoogleDriveRouter { } } - async fn fetch_file_metadata(&self, uri: &str) -> Result { + async fn fetch_file_metadata(&self, uri: &str) -> Result { self.drive .files() .get(uri) @@ -1168,11 +1200,10 @@ impl GoogleDriveRouter { .add_scope(GOOGLE_DRIVE_SCOPES) .doit() .await - .map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to execute Google Drive get query, {}.", - e - )) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Drive get query, {}.", e)), + data: None, }) .map(|r| r.1) } @@ -1250,7 +1281,7 @@ impl GoogleDriveRouter { uri: &str, mime_type: &str, include_images: bool, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let export_mime_type = match mime_type { "application/vnd.google-apps.document" => "text/markdown", "application/vnd.google-apps.spreadsheet" => "text/csv", @@ -1269,10 +1300,14 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute google drive export for {}, {}.", - uri, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to execute google drive export for {}, {}.", + uri, e + )), + data: None, + }), Ok(r) => { if let Ok(body) = r.into_body().collect().await { if let Ok(response) = String::from_utf8(body.to_bytes().to_vec()) { @@ -1280,11 +1315,10 @@ impl GoogleDriveRouter { let content = self.strip_image_body(&response); Ok(vec![Content::text(content).with_priority(0.1)]) } else { - let images = self.resize_images(&response).map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to resize image(s): {}", - e - )) + let images = self.resize_images(&response).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to resize image(s): {}", e)), + data: None, })?; let content = self.strip_image_body(&response); @@ -1293,16 +1327,24 @@ impl GoogleDriveRouter { .collect::>()) } } else { - Err(ToolError::ExecutionError(format!( - "Failed to export google drive to string, {}.", - uri, - ))) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to export google drive to string, {}.", + uri, + )), + data: None, + }) } } else { - Err(ToolError::ExecutionError(format!( - "Failed to export google drive document, {}.", - uri, - ))) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to export google drive document, {}.", + uri + )), + data: None, + }) } } } @@ -1314,7 +1356,7 @@ impl GoogleDriveRouter { uri: &str, mime_type: &str, include_images: bool, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let result = self .drive .files() @@ -1326,10 +1368,14 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute google drive export for {}, {}.", - uri, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to execute google drive export for {}, {}.", + uri, e + )), + data: None, + }), Ok(r) => { if mime_type.starts_with("text/") || mime_type == "application/json" { if let Ok(body) = r.0.into_body().collect().await { @@ -1338,12 +1384,15 @@ impl GoogleDriveRouter { let content = self.strip_image_body(&response); Ok(vec![Content::text(content).with_priority(0.1)]) } else { - let images = self.resize_images(&response).map_err(|e| { - ToolError::ExecutionError(format!( - "Failed to resize image(s): {}", - e - )) - })?; + let images = + self.resize_images(&response).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to resize image(s): {}", + e + )), + data: None, + })?; let content = self.strip_image_body(&response); Ok(std::iter::once(Content::text(content).with_priority(0.1)) @@ -1351,29 +1400,41 @@ impl GoogleDriveRouter { .collect::>()) } } else { - Err(ToolError::ExecutionError(format!( - "Failed to convert google drive to string, {}.", - uri, - ))) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to convert google drive to string, {}.", + uri, + )), + data: None, + }) } } else { - Err(ToolError::ExecutionError(format!( - "Failed to get google drive document, {}.", - uri, - ))) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to get google drive document, {}.", + uri, + )), + data: None, + }) } } else { //TODO: handle base64 image case, see typscript mcp-gdrive - Err(ToolError::ExecutionError(format!( - "Suported mimeType {}, for {}", - mime_type, uri, - ))) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Supported mimeType {}, for {}", + mime_type, uri, + )), + data: None, + }) } } } } - async fn read(&self, params: Value) -> Result, ToolError> { + async fn read(&self, params: Value) -> Result, ErrorData> { let (maybe_uri, maybe_url) = ( params.get("uri").and_then(|q| q.as_str()), params.get("url").and_then(|q| q.as_str()), @@ -1385,10 +1446,14 @@ impl GoogleDriveRouter { // Validation: check for / path separators as invalid uris if drive_uri.contains('/') { - return Err(ToolError::InvalidParameters(format!( - "The uri '{}' contains extra '/'. Only the base URI is allowed.", - uri - ))); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "The uri '{}' contains extra '/'. Only the base URI is allowed.", + uri + )), + data: None, + }); } drive_uri @@ -1397,21 +1462,29 @@ impl GoogleDriveRouter { if let Some(drive_uri) = extract_google_drive_id(url) { drive_uri.to_string() } else { - return Err(ToolError::InvalidParameters(format!( - "Failed to extract valid google drive URI from {}", - url - ))); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Failed to extract valid google drive URI from {}", + url + )), + data: None, + }); } } (Some(_), Some(_)) => { - return Err(ToolError::InvalidParameters( - "Only one of 'uri' or 'url' should be provided".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Only one of 'uri' or 'url' should be provided".to_string()), + data: None, + }); } (None, None) => { - return Err(ToolError::InvalidParameters( - "Either 'uri' or 'url' must be provided".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Either 'uri' or 'url' must be provided".to_string()), + data: None, + }); } }; @@ -1421,11 +1494,13 @@ impl GoogleDriveRouter { .unwrap_or(false); let metadata = self.fetch_file_metadata(&drive_uri).await?; - let mime_type = metadata.mime_type.ok_or_else(|| { - ToolError::ExecutionError(format!( + let mime_type = metadata.mime_type.ok_or_else(|| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( "Missing mime type in file metadata for {}.", drive_uri - )) + )), + data: None, })?; // Handle Google Docs export @@ -1439,14 +1514,24 @@ impl GoogleDriveRouter { } // Implement sheets_tool functionality - async fn sheets_tool(&self, params: Value) -> Result, ToolError> { - let spreadsheet_id = params.get("spreadsheetId").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The spreadsheetId is required".to_string()), - )?; - - let operation = params.get("operation").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The operation is required".to_string()), - )?; + async fn sheets_tool(&self, params: Value) -> Result, ErrorData> { + let spreadsheet_id = params + .get("spreadsheetId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The spreadsheetId is required".to_string()), + data: None, + })?; + + let operation = params + .get("operation") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The operation is required".to_string()), + data: None, + })?; match operation { "list_sheets" => { @@ -1461,10 +1546,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets get query, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets get query, {}.", e)), + data: None, + }), Ok(r) => { let spreadsheet = r.1; let sheets = spreadsheet.sheets.unwrap_or_default(); @@ -1508,10 +1594,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets get_columns query, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets get_columns query, {}.", e)), + data: None, + }), Ok(r) => { let value_range = r.1; // Extract just the headers (first row) @@ -1536,9 +1623,11 @@ impl GoogleDriveRouter { let range = params .get("range") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The range is required for get_values operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The range is required for get_values operation".to_string()), + data: None, + })?; let result = self .sheets @@ -1550,10 +1639,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets values_get query, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets values_get query, {}.", e)), + data: None, + }), Ok(r) => { let value_range = r.1; // Convert the values to a CSV string @@ -1581,16 +1671,20 @@ impl GoogleDriveRouter { let range = params .get("range") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The range is required for update_values operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The range is required for update_values operation".to_string()), + data: None, + })?; let values_csv = params .get("values") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The values parameter is required for update_values operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The values parameter is required for update_values operation".to_string()), + data: None, + })?; // Parse the CSV data into a 2D array of values let mut values: Vec> = Vec::new(); @@ -1629,10 +1723,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets values_update query, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets values_update query, {}.", e)), + data: None, + }), Ok(r) => { let update_response = r.1; let updated_cells = update_response.updated_cells.unwrap_or(0); @@ -1653,16 +1748,20 @@ impl GoogleDriveRouter { let cell = params .get("cell") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The cell parameter is required for update_cell operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The cell parameter is required for update_cell operation".to_string()), + data: None, + })?; let value = params .get("value") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The value parameter is required for update_cell operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The value parameter is required for update_cell operation".to_string()), + data: None, + })?; // Determine the input option (default to USER_ENTERED) let value_input_option = params @@ -1689,10 +1788,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets update_cell operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets update_cell operation, {}.", e)), + data: None, + }), Ok(r) => { let update_response = r.1; let updated_range = update_response.updated_range.unwrap_or_default(); @@ -1708,9 +1808,11 @@ impl GoogleDriveRouter { let title = params .get("title") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The title parameter is required for add_sheet operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The title parameter is required for add_sheet operation".to_string()), + data: None, + })?; // Create the AddSheetReques let add_sheet_request = google_sheets4::api::AddSheetRequest { @@ -1750,10 +1852,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets add_sheet operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets add_sheet operation, {}.", e)), + data: None, + }), Ok(r) => { let response = r.1; let replies = response.replies.unwrap_or_default(); @@ -1786,9 +1889,11 @@ impl GoogleDriveRouter { let range = params .get("range") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The range is required for clear_values operation".to_string(), - ))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The range is required for clear_values operation".to_string()), + data: None, + })?; // Create the ClearValuesReques let clear_values_request = google_sheets4::api::ClearValuesRequest::default(); @@ -1804,10 +1909,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Sheets clear_values operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Sheets clear_values operation, {}.", e)), + data: None, + }), Ok(r) => { let response = r.1; let cleared_range = response.cleared_range.unwrap_or_default(); @@ -1819,10 +1925,11 @@ impl GoogleDriveRouter { } } }, - _ => Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}. Supported operations are: list_sheets, get_columns, get_values, update_values, update_cell, add_sheet, clear_values", - operation - ))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid operation: {}. Supported operations are: list_sheets, get_columns, get_values, update_values, update_cell, add_sheet, clear_values", operation)), + data: None, + }), } } @@ -1905,7 +2012,7 @@ impl GoogleDriveRouter { parent: Option<&str>, support_all_drives: bool, target_id: Option<&str>, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let mut req = File { mime_type: Some(target_mime_type.to_string()), ..Default::default() @@ -1951,10 +2058,14 @@ impl GoogleDriveRouter { }; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to upload google drive file {:?}, {}.", - operation, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to upload google drive file {:?}, {}.", + operation, e + )), + data: None, + }), Ok(r) => Ok(vec![Content::text(format!( "{} ({}) (uri: {})", r.1.name.unwrap_or_default(), @@ -1964,23 +2075,25 @@ impl GoogleDriveRouter { } } - async fn create_file(&self, params: Value) -> Result, ToolError> { + async fn create_file(&self, params: Value) -> Result, ErrorData> { // Extract common parameters - let filename = - params - .get("name") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The name param is required".to_string(), - ))?; - - let mime_type = - params - .get("mimeType") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The mimeType param is required".to_string(), - ))?; + let filename = params + .get("name") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The name param is required".to_string()), + data: None, + })?; + + let mime_type = params + .get("mimeType") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The mimeType param is required".to_string()), + data: None, + })?; let parent_id = params.get("parentId").and_then(|q| q.as_str()); let target_id = params.get("targetId").and_then(|q| q.as_str()); @@ -1997,11 +2110,15 @@ impl GoogleDriveRouter { match mime_type { "application/vnd.google-apps.document" => { if body.is_none() { - return Err(ToolError::InvalidParameters( - "The body param is required for google document file type".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The body param is required for google document file type" + .to_string(), + ), + data: None, + }); } - ( "text/markdown".to_string(), mime_type.to_string(), @@ -2010,12 +2127,15 @@ impl GoogleDriveRouter { } "application/vnd.google-apps.spreadsheet" => { if body.is_none() { - return Err(ToolError::InvalidParameters( - "The body param is required for google spreadsheet file type" - .to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The body param is required for google spreadsheet file type" + .to_string(), + ), + data: None, + }); } - ( "text/csv".to_string(), mime_type.to_string(), @@ -2024,17 +2144,19 @@ impl GoogleDriveRouter { } "application/vnd.google-apps.presentation" => { if path.is_none() { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "The path param is required for google slides file type".to_string(), + None, )); } - let file = std::fs::File::open(path.unwrap()).map_err(|e| { - ToolError::ExecutionError( - format!("Error opening {}: {}", path.unwrap(), e).to_string(), + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Error opening {}: {}", path.unwrap(), e), + None, ) })?; - ( "application/vnd.openxmlformats-officedocument.presentationml.presentation" .to_string(), @@ -2053,9 +2175,14 @@ impl GoogleDriveRouter { } "application/vnd.google-apps.shortcut" => { if target_id.is_none() { - return Err(ToolError::InvalidParameters( - "The targetId param is required when creating a shortcut".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The targetId param is required when creating a shortcut" + .to_string(), + ), + data: None, + }); } let emptybuf: [u8; 0] = []; let empty_stream = Cursor::new(emptybuf); @@ -2069,14 +2196,18 @@ impl GoogleDriveRouter { _ => { let reader: Box = match (body, path) { (None, None) | (Some(_), Some(_)) => { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "Either the body or path param is required".to_string(), + None, )) } (Some(b), None) => Box::new(Cursor::new(b.as_bytes().to_owned())), (None, Some(p)) => Box::new(std::fs::File::open(p).map_err(|e| { - ToolError::ExecutionError( + ErrorData::new( + ErrorCode::INTERNAL_ERROR, format!("Error opening {}: {}", p, e).to_string(), + None, ) })?), }; @@ -2099,23 +2230,31 @@ impl GoogleDriveRouter { .await } - async fn move_file(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; + async fn move_file(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The fileId param is required".to_string()), + data: None, + })?; let current_folder_id = params .get("currentFolderId") .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The currentFolderId param is required".to_string(), - ))?; - let new_folder_id = params.get("newFolderId").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The newFolderId param is required".to_string()), - )?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The currentFolderId param is required".to_string()), + data: None, + })?; + let new_folder_id = params + .get("newFolderId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The newFolderId param is required".to_string()), + data: None, + })?; let req = File::default(); let result = self .drive @@ -2130,10 +2269,14 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to move google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to move google drive file {}, {}.", + file_id, e + )), + data: None, + }), Ok(r) => Ok(vec![Content::text(format!( "{} ({}) (uri: {})", r.1.name.unwrap_or_default(), @@ -2143,14 +2286,15 @@ impl GoogleDriveRouter { } } - async fn update_file(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; + async fn update_file(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The fileId param is required".to_string()), + data: None, + })?; let allow_shared_drives = params .get("allowSharedDrives") @@ -2181,17 +2325,22 @@ impl GoogleDriveRouter { &self, file_id: &str, label_ops: &Vec, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let mut req = ModifyLabelsRequest::default(); let mut label_mods = vec![]; for op in label_ops { if let Some(op) = op.as_object() { - let label_id = op.get("labelId").and_then(|o| o.as_str()).ok_or( - ToolError::InvalidParameters( - "The labelId param is required for label changes".to_string(), - ), - )?; + let label_id = + op.get("labelId") + .and_then(|o| o.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The labelId param is required for label changes".to_string(), + ), + data: None, + })?; match op.get("operation").and_then(|o| o.as_str()) { Some("removeLabel") => { let removal = LabelModification { @@ -2202,11 +2351,17 @@ impl GoogleDriveRouter { label_mods.push(removal); } Some("unsetField") => { - let field_id = op.get("fieldId").and_then(|o| o.as_str()).ok_or( - ToolError::InvalidParameters( - "The fieldId param is required for unsetting a field.".to_string(), - ), - )?; + let field_id = + op.get("fieldId").and_then(|o| o.as_str()).ok_or_else(|| { + ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The fieldId param is required for unsetting a field." + .to_string(), + ), + data: None, + } + })?; let field_mods = LabelFieldModification { field_id: Some(field_id.to_string()), unset_values: Some(true), @@ -2227,15 +2382,19 @@ impl GoogleDriveRouter { } if let Some(date_value) = op.get("dateValue").and_then(|o| o.as_array()) { - let parsed_dates: Result, ToolError> = date_value + let parsed_dates: Result, ErrorData> = date_value .iter() .filter_map(|d| d.as_str()) .map(|d| { NaiveDate::parse_from_str(d, "%Y-%m-%d").map_err(|e| { - ToolError::InvalidParameters(format!( - "Error parsing field date: {}", - e - )) + ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Error parsing field date: {}", + e + )), + data: None, + } }) }) .collect(); @@ -2287,10 +2446,14 @@ impl GoogleDriveRouter { label_mods.push(update_mod); } _ => { - return Err(ToolError::InvalidParameters(format!( - "Label operation invalid: {:?}", - op.get("operation") - ))) + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Label operation invalid: {:?}", + op.get("operation") + )), + data: None, + }) } } }; @@ -2299,10 +2462,14 @@ impl GoogleDriveRouter { let result = self.drive.files().modify_labels(req, file_id).doit().await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to update label for google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to update label for google drive file {}, {}.", + file_id, e + )), + data: None, + }), Ok(r) => Ok(vec![Content::text(format!( "file URI: {}, labels modified: {:?}", file_id, @@ -2318,17 +2485,21 @@ impl GoogleDriveRouter { body: Option<&str>, path: Option<&str>, allow_shared_drives: bool, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { // Determine source and target MIME types based on file_type let (source_mime_type, target_mime_type, reader): (String, String, Box) = match mime_type { "application/vnd.google-apps.document" => { if body.is_none() { - return Err(ToolError::InvalidParameters( - "The body param is required for google document file type".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The body param is required for google document file type" + .to_string(), + ), + data: None, + }); } - ( "text/markdown".to_string(), mime_type.to_string(), @@ -2337,12 +2508,15 @@ impl GoogleDriveRouter { } "application/vnd.google-apps.spreadsheet" => { if body.is_none() { - return Err(ToolError::InvalidParameters( - "The body param is required for google spreadsheet file type" - .to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The body param is required for google spreadsheet file type" + .to_string(), + ), + data: None, + }); } - ( "text/csv".to_string(), mime_type.to_string(), @@ -2351,15 +2525,22 @@ impl GoogleDriveRouter { } "application/vnd.google-apps.presentation" => { if path.is_none() { - return Err(ToolError::InvalidParameters( - "The path param is required for google slides file type".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( + "The path param is required for google slides file type" + .to_string(), + ), + data: None, + }); } - let file = std::fs::File::open(path.unwrap()).map_err(|e| { - ToolError::ExecutionError( + let file = std::fs::File::open(path.unwrap()).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from( format!("Error opening {}: {}", path.unwrap(), e).to_string(), - ) + ), + data: None, })?; ( @@ -2369,17 +2550,22 @@ impl GoogleDriveRouter { Box::new(file), ) } + _ => { let reader: Box = match (body, path) { (None, None) | (Some(_), Some(_)) => { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "Either the body or path param is required".to_string(), + None, )) } (Some(b), None) => Box::new(Cursor::new(b.as_bytes().to_owned())), (None, Some(p)) => Box::new(std::fs::File::open(p).map_err(|e| { - ToolError::ExecutionError( + ErrorData::new( + ErrorCode::INTERNAL_ERROR, format!("Error opening {}: {}", p, e).to_string(), + None, ) })?), }; @@ -2401,14 +2587,15 @@ impl GoogleDriveRouter { .await } - async fn get_comments(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; + async fn get_comments(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The fileId param is required".to_string()), + data: None, + })?; let mut results: Vec = Vec::new(); let mut state = PaginationState::Start; @@ -2428,10 +2615,14 @@ impl GoogleDriveRouter { let result = comment_list.doit().await; match result { Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to execute google drive comment list, {}.", - e - ))) + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to execute google drive comment list, {}.", + e + )), + data: None, + }) } Ok(r) => { let mut content = @@ -2466,24 +2657,34 @@ impl GoogleDriveRouter { Ok(vec![Content::text(results.join("\n"))]) } - async fn manage_comment(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; - let operation = params.get("operation").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The operation is required".to_string()), - )?; - let content = - params - .get("content") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( + #[allow(clippy::too_many_lines)] + async fn manage_comment(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The fileId param is required".to_string()), + data: None, + })?; + let operation = params + .get("operation") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The operation is required".to_string()), + data: None, + })?; + let content = params + .get("content") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from( "The content param is required if the action is create".to_string(), - ))?; + ), + data: None, + })?; match operation { "create" => { @@ -2502,10 +2703,14 @@ impl GoogleDriveRouter { .doit() .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to add comment for google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to add comment for google drive file {}, {}.", + file_id, e + )), + data: None, + }), Ok(r) => Ok(vec![Content::text(format!( "Author: {:?} Content: {} Created: {} uri: {} quoted_content: {:?}", r.1.author.unwrap_or_default(), @@ -2517,11 +2722,14 @@ impl GoogleDriveRouter { } } "reply" => { - let comment_id = params.get("commentId").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters( - "The commentId param is required for reply".to_string(), - ), - )?; + let comment_id = params + .get("commentId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The commentId param is required for reply".to_string()), + data: None, + })?; let resolve_comment = params .get("resolveComment") @@ -2546,10 +2754,14 @@ impl GoogleDriveRouter { .doit() .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to manage reply to comment {} for google drive file {}, {}.", - comment_id, file_id, e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!( + "Failed to manage reply to comment {} for google drive file {}, {}.", + comment_id, file_id, e + )), + data: None, + }), Ok(r) => Ok(vec![Content::text(format!( "Action: {} Author: {:?} Content: {} Created: {} uri: {}", r.1.action.unwrap_or_default(), @@ -2560,21 +2772,35 @@ impl GoogleDriveRouter { ))]), } } - _ => Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}. Supported operations are: create, reply", - operation - ))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Invalid operation: {}. Supported operations are: create, reply", + operation + )), + data: None, + }), } } - async fn docs_tool(&self, params: Value) -> Result, ToolError> { - let document_id = params.get("documentId").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The documentId is required".to_string()), - )?; - - let operation = params.get("operation").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The operation is required".to_string()), - )?; + async fn docs_tool(&self, params: Value) -> Result, ErrorData> { + let document_id = params + .get("documentId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The documentId is required".to_string()), + data: None, + })?; + + let operation = params + .get("operation") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The operation is required".to_string()), + data: None, + })?; match operation { "get_document" => { @@ -2589,10 +2815,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs get query, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs get query, {}.", e)), + data: None, + }), Ok(r) => { let document = r.1; let title = document.title.unwrap_or_default(); @@ -2624,13 +2851,17 @@ impl GoogleDriveRouter { } }, "insert_text" => { - let text = params.get("text").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The text parameter is required for insert_text operation".to_string()), - )?; + let text = params.get("text").and_then(|q| q.as_str()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The text parameter is required for insert_text operation".to_string()), + data: None, + })?; - let position = params.get("position").and_then(|q| q.as_i64()).ok_or( - ToolError::InvalidParameters("The position parameter is required for insert_text operation".to_string()), - )?; + let position = params.get("position").and_then(|q| q.as_i64()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The position parameter is required for insert_text operation".to_string()), + data: None, + })?; // Create the insert text request let insert_text_request = google_docs1::api::InsertTextRequest { @@ -2662,10 +2893,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs insert_text operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs insert_text operation, {}.", e)), + data: None, + }), Ok(_) => { Ok(vec![Content::text(format!( "Successfully inserted text at position {}.", @@ -2675,9 +2907,11 @@ impl GoogleDriveRouter { } }, "append_text" => { - let text = params.get("text").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The text parameter is required for append_text operation".to_string()), - )?; + let text = params.get("text").and_then(|q| q.as_str()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The text parameter is required for append_text operation".to_string()), + data: None, + })?; // First, get the document to find the end position let get_result = self @@ -2691,10 +2925,11 @@ impl GoogleDriveRouter { let end_index = match get_result { Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to get document to determine end position, {}.", - e - ))); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get document to determine end position, {}.", e)), + data: None, + }); }, Ok(r) => { let document = r.1; @@ -2740,23 +2975,28 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs append_text operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs append_text operation, {}.", e)), + data: None, + }), Ok(_) => { Ok(vec![Content::text("Successfully appended text to the document.").with_priority(0.1)]) } } }, "replace_text" => { - let text = params.get("text").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The text parameter is required for replace_text operation".to_string()), - )?; - - let replace_text = params.get("replaceText").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The replaceText parameter is required for replace_text operation".to_string()), - )?; + let text = params.get("text").and_then(|q| q.as_str()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The text parameter is required for replace_text operation".to_string()), + data: None, + })?; + + let replace_text = params.get("replaceText").and_then(|q| q.as_str()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The replaceText parameter is required for replace_text operation".to_string()), + data: None, + })?; // Create the replace all text request let replace_all_text_request = google_docs1::api::ReplaceAllTextRequest { @@ -2787,10 +3027,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs replace_text operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs replace_text operation, {}.", e)), + data: None, + }), Ok(r) => { let response = r.1; let replacements = response @@ -2812,9 +3053,11 @@ impl GoogleDriveRouter { } }, "create_paragraph" => { - let text = params.get("text").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The text parameter is required for create_paragraph operation".to_string()), - )?; + let text = params.get("text").and_then(|q| q.as_str()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The text parameter is required for create_paragraph operation".to_string()), + data: None, + })?; // Get the end position of the document let get_result = self @@ -2828,10 +3071,11 @@ impl GoogleDriveRouter { let end_index = match get_result { Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to get document to determine end position, {}.", - e - ))); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to get document to determine end position, {}.", e)), + data: None, + }); }, Ok(r) => { let document = r.1; @@ -2877,23 +3121,28 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs create_paragraph operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs create_paragraph operation, {}.", e)), + data: None, + }), Ok(_) => { Ok(vec![Content::text("Successfully created a new paragraph.").with_priority(0.1)]) } } }, "delete_content" => { - let start_position = params.get("startPosition").and_then(|q| q.as_i64()).ok_or( - ToolError::InvalidParameters("The startPosition parameter is required for delete_content operation".to_string()), - )?; + let start_position = params.get("startPosition").and_then(|q| q.as_i64()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The startPosition parameter is required for delete_content operation".to_string()), + data: None, + })?; - let end_position = params.get("endPosition").and_then(|q| q.as_i64()).ok_or( - ToolError::InvalidParameters("The endPosition parameter is required for delete_content operation".to_string()), - )?; + let end_position = params.get("endPosition").and_then(|q| q.as_i64()).ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The endPosition parameter is required for delete_content operation".to_string()), + data: None, + })?; // Create the delete content range request let delete_content_range_request = google_docs1::api::DeleteContentRangeRequest { @@ -2924,10 +3173,11 @@ impl GoogleDriveRouter { .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to execute Google Docs delete_content operation, {}.", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute Google Docs delete_content operation, {}.", e)), + data: None, + }), Ok(_) => { Ok(vec![Content::text(format!( "Successfully deleted content from position {} to {}.", @@ -2936,14 +3186,15 @@ impl GoogleDriveRouter { } } }, - _ => Err(ToolError::InvalidParameters(format!( - "Invalid operation: {}. Supported operations are: get_document, insert_text, append_text, replace_text, create_paragraph, delete_content", - operation - ))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid operation: {}. Supported operations are: get_document, insert_text, append_text, replace_text, create_paragraph, delete_content", operation)), + data: None, + }), } } - async fn list_drives(&self, params: Value) -> Result, ToolError> { + async fn list_drives(&self, params: Value) -> Result, ErrorData> { let query = params.get("name_contains").and_then(|q| q.as_str()); let mut results: Vec = Vec::new(); @@ -2966,10 +3217,11 @@ impl GoogleDriveRouter { match result { Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to execute google drive list, {}.", - e - ))) + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute google drive list, {}.", e)), + data: None, + }); } Ok(r) => { let mut content = @@ -3011,14 +3263,15 @@ impl GoogleDriveRouter { p.id.unwrap_or_default()) } - async fn get_permissions(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; + async fn get_permissions(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("The fileId param is required".to_string()), + data: None, + })?; let mut results: Vec = Vec::new(); let mut state = PaginationState::Start; @@ -3039,10 +3292,11 @@ impl GoogleDriveRouter { match result { Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to execute google drive list, {}.", - e - ))) + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to execute google drive list, {}.", e)), + data: None, + }); } Ok(r) => { let mut content = @@ -3062,24 +3316,30 @@ impl GoogleDriveRouter { Ok(vec![Content::text(results.join("\n"))]) } - async fn sharing(&self, params: Value) -> Result, ToolError> { - let file_id = - params - .get("fileId") - .and_then(|q| q.as_str()) - .ok_or(ToolError::InvalidParameters( - "The fileId param is required".to_string(), - ))?; - let operation = params.get("operation").and_then(|q| q.as_str()).ok_or( - ToolError::InvalidParameters("The operation is required".to_string()), - )?; + async fn sharing(&self, params: Value) -> Result, ErrorData> { + let file_id = params + .get("fileId") + .and_then(|q| q.as_str()) + .ok_or(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "The fileId param is required".to_string(), + None, + ))?; + let operation = params + .get("operation") + .and_then(|q| q.as_str()) + .ok_or(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "The operation is required".to_string(), + None, + ))?; let permission_id = params.get("permissionId").and_then(|q| q.as_str()); let role = params.get("role").and_then(|s| { s.as_str().map(|s| { if ROLES.contains(&s) { Ok(s) } else { - Err(ToolError::InvalidParameters("Invalid role: must be one of ('owner', 'organizer', 'fileOrganizer', 'writer', 'commenter', 'reader')".to_string())) + Err(ErrorData::new(ErrorCode::INVALID_PARAMS, "Invalid role: must be one of ('owner', 'organizer', 'fileOrganizer', 'writer', 'commenter', 'reader')".to_string(), None)) } }) }).transpose()?; @@ -3088,7 +3348,7 @@ impl GoogleDriveRouter { if PERMISSIONTYPE.contains(&s) { Ok(s) } else { - Err(ToolError::InvalidParameters("Invalid permission type: must be one of ('user', 'group', 'domain', 'anyone')".to_string())) + Err(ErrorData::new(ErrorCode::INVALID_PARAMS, "Invalid permission type: must be one of ('user', 'group', 'domain', 'anyone')".to_string(), None)) } }) ).transpose()?; @@ -3100,9 +3360,11 @@ impl GoogleDriveRouter { let (role, permission_type) = match (role, permission_type) { (Some(r), Some(t)) => (r, t), _ => { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "The 'create' operation requires the 'role' and 'type' parameters." .to_string(), + None, )) } }; @@ -3118,10 +3380,14 @@ impl GoogleDriveRouter { ("domain", Some(d)) => req.domain = Some(d.to_string()), ("anyone", None) => {} (_, _) => { - return Err(ToolError::InvalidParameters(format!( - "The '{}' operation for type '{}' requires the 'target' parameter.", - operation, permission_type - ))) + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "The '{}' operation for type '{}' requires the 'target' parameter.", + operation, permission_type + ), + None, + )) } } @@ -3138,10 +3404,14 @@ impl GoogleDriveRouter { let result = builder.doit().await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to manage sharing for google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Failed to manage sharing for google drive file {}, {}.", + file_id, e + ), + None, + )), Ok(r) => Ok(vec![Content::text(self.output_permission(r.1))]), } } @@ -3149,9 +3419,11 @@ impl GoogleDriveRouter { let (permission_id, role) = match (permission_id, role) { (Some(p), Some(r)) => (p, r), _ => { - return Err(ToolError::InvalidParameters( + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, "The 'update' operation requires the 'permissionId', and 'role'." .to_string(), + None, )) } }; @@ -3175,16 +3447,22 @@ impl GoogleDriveRouter { .doit() .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to manage sharing for google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Failed to manage sharing for google drive file {}, {}.", + file_id, e + ), + None, + )), Ok(r) => Ok(vec![Content::text(self.output_permission(r.1))]), } } "delete" => { - let permission_id = permission_id.ok_or(ToolError::InvalidParameters( + let permission_id = permission_id.ok_or(ErrorData::new( + ErrorCode::INVALID_PARAMS, "The 'delete' operation requires the 'permissionId'.".to_string(), + None, ))?; let result = self @@ -3197,27 +3475,33 @@ impl GoogleDriveRouter { .doit() .await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to manage sharing for google drive file {}, {}.", - file_id, e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Failed to manage sharing for google drive file {}, {}.", + file_id, e + ), + None, + )), Ok(_) => Ok(vec![Content::text(format!( "Deleted permission: {} from file: {}", file_id, permission_id ))]), } } - s => Err(ToolError::InvalidParameters( + s => Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, format!( "Parameter 'operation' must be one of ('create', 'update', 'delete'); given {}", s ) .to_string(), + None, )), } } - async fn list_labels(&self, _params: Value) -> Result, ToolError> { + async fn list_labels(&self, _params: Value) -> Result, ErrorData> { let builder = self .drive_labels .labels() @@ -3226,10 +3510,11 @@ impl GoogleDriveRouter { let result = builder.doit().await; match result { - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to list labels for Google Drive {}", - e - ))), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to list labels for Google Drive {}", e)), + data: None, + }), Ok(r) => { let content = r.1.labels @@ -3280,7 +3565,7 @@ impl Router for GoogleDriveRouter { tool_name: &str, arguments: Value, _notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); Box::pin(async move { @@ -3297,7 +3582,11 @@ impl Router for GoogleDriveRouter { "list_drives" => this.list_drives(arguments).await, "get_permissions" => this.get_permissions(arguments).await, "sharing" => this.sharing(arguments).await, - _ => Err(ToolError::NotFound(format!("Tool {} not found", tool_name))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!("Tool {} not found", tool_name)), + data: None, + }), } }) } diff --git a/crates/goose-mcp/src/memory/mod.rs b/crates/goose-mcp/src/memory/mod.rs index fee7b117b034..2bd1d20a57b0 100644 --- a/crates/goose-mcp/src/memory/mod.rs +++ b/crates/goose-mcp/src/memory/mod.rs @@ -2,13 +2,15 @@ use async_trait::async_trait; use etcetera::{choose_app_strategy, AppStrategy}; use indoc::formatdoc; use mcp_core::{ - handler::{PromptError, ResourceError, ToolError}, + handler::{PromptError, ResourceError}, protocol::ServerCapabilities, tool::ToolCall, }; use mcp_server::router::CapabilitiesBuilder; use mcp_server::Router; -use rmcp::model::{Content, JsonRpcMessage, Prompt, Resource, Tool, ToolAnnotations}; +use rmcp::model::{ + Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, Resource, Tool, ToolAnnotations, +}; use rmcp::object; use serde_json::Value; use std::{ @@ -523,7 +525,7 @@ impl Router for MemoryRouter { tool_name: &str, arguments: Value, _notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); @@ -534,7 +536,11 @@ impl Router for MemoryRouter { }; match this.execute_tool_call(tool_call).await { Ok(result) => Ok(vec![Content::text(result)]), - Err(err) => Err(ToolError::ExecutionError(err.to_string())), + Err(err) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + err.to_string(), + None, + )), } }) } diff --git a/crates/goose-mcp/src/tutorial/mod.rs b/crates/goose-mcp/src/tutorial/mod.rs index b15e3dca8ac0..0b579085caf8 100644 --- a/crates/goose-mcp/src/tutorial/mod.rs +++ b/crates/goose-mcp/src/tutorial/mod.rs @@ -2,12 +2,14 @@ use anyhow::Result; use include_dir::{include_dir, Dir}; use indoc::formatdoc; use mcp_core::{ - handler::{PromptError, ResourceError, ToolError}, + handler::{PromptError, ResourceError}, protocol::ServerCapabilities, }; use mcp_server::router::CapabilitiesBuilder; use mcp_server::Router; -use rmcp::model::{Content, JsonRpcMessage, Prompt, Resource, Role, Tool, ToolAnnotations}; +use rmcp::model::{ + Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, Resource, Role, Tool, ToolAnnotations, +}; use rmcp::object; use serde_json::Value; use std::{future::Future, pin::Pin}; @@ -92,14 +94,13 @@ impl TutorialRouter { tutorials } - async fn load_tutorial(&self, name: &str) -> Result { + async fn load_tutorial(&self, name: &str) -> Result { let file_name = format!("{}.md", name); - let file = TUTORIALS_DIR - .get_file(&file_name) - .ok_or(ToolError::ExecutionError(format!( - "Could not locate tutorial '{}'", - name - )))?; + let file = TUTORIALS_DIR.get_file(&file_name).ok_or(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Could not locate tutorial '{}'", name), + None, + ))?; Ok(String::from_utf8_lossy(file.contents()).into_owned()) } } @@ -126,7 +127,7 @@ impl Router for TutorialRouter { tool_name: &str, arguments: Value, _notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); @@ -137,7 +138,11 @@ impl Router for TutorialRouter { .get("name") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::InvalidParameters("Missing 'name' parameter".to_string()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'name' parameter".to_string(), + None, + ) })?; let content = this.load_tutorial(name).await?; @@ -145,7 +150,11 @@ impl Router for TutorialRouter { Content::text(content).with_audience(vec![Role::Assistant]) ]) } - _ => Err(ToolError::NotFound(format!("Tool {} not found", tool_name))), + _ => Err(ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + format!("Tool {} not found", tool_name), + None, + )), } }) } diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 9eab644dda75..60699454f860 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -44,9 +44,11 @@ use crate::scheduler_trait::SchedulerTrait; use crate::session; use crate::tool_monitor::{ToolCall, ToolMonitor}; use crate::utils::is_token_cancelled; -use mcp_core::{ToolError, ToolResult}; +use mcp_core::ToolResult; use regex::Regex; -use rmcp::model::{Content, GetPromptResult, Prompt, ServerNotification, Tool}; +use rmcp::model::{ + Content, ErrorCode, ErrorData, GetPromptResult, Prompt, ServerNotification, Tool, +}; use serde_json::Value; use tokio::sync::{mpsc, Mutex, RwLock}; use tokio_util::sync::CancellationToken; @@ -376,7 +378,7 @@ impl Agent { tool_call: mcp_core::tool::ToolCall, request_id: String, cancellation_token: Option, - ) -> (String, Result) { + ) -> (String, Result) { // Check if this tool call should be allowed based on repetition monitoring if let Some(monitor) = self.tool_monitor.lock().await.as_mut() { let tool_call_info = ToolCall::new(tool_call.name.clone(), tool_call.arguments.clone()); @@ -384,8 +386,10 @@ impl Agent { if !monitor.check_tool_call(tool_call_info) { return ( request_id, - Err(ToolError::ExecutionError( + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "Tool call rejected: exceeded maximum allowed repetitions".to_string(), + None, )), ); } @@ -425,8 +429,10 @@ impl Agent { } else { ( request_id, - Err(ToolError::ExecutionError( + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "Final output tool not defined".to_string(), + None, )), ) }; @@ -478,8 +484,10 @@ impl Agent { ToolCallResult::from(extension_manager.search_available_extensions().await) } else if self.is_frontend_tool(&tool_call.name).await { // For frontend tools, return an error indicating we need frontend execution - ToolCallResult::from(Err(ToolError::ExecutionError( + ToolCallResult::from(Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "Frontend tool execution required".to_string(), + None, ))) } else if tool_call.name == TODO_READ_TOOL_NAME { // Handle task planner read tool @@ -505,11 +513,13 @@ impl Agent { if max_chars > 0 && char_count > max_chars { return ( request_id, - Ok(ToolCallResult::from(Err(ToolError::ExecutionError( + Ok(ToolCallResult::from(Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, format!( "Todo list too large: {} chars (max: {})", char_count, max_chars ), + None, )))), ); } @@ -537,7 +547,11 @@ impl Agent { .dispatch_tool_call(tool_call.clone(), cancellation_token.unwrap_or_default()) .await; result.unwrap_or_else(|e| { - ToolCallResult::from(Err(ToolError::ExecutionError(e.to_string()))) + ToolCallResult::from(Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + e.to_string(), + None, + ))) }) }; @@ -554,12 +568,13 @@ impl Agent { ) } + #[allow(clippy::too_many_lines)] pub(super) async fn manage_extensions( &self, action: String, extension_name: String, request_id: String, - ) -> (String, Result, ToolError>) { + ) -> (String, Result, ErrorData>) { let selector = self.tool_route_manager.get_router_tool_selector().await; if ToolRouterIndexManager::is_tool_router_enabled(&selector) { if let Some(selector) = selector { @@ -576,10 +591,11 @@ impl Agent { { return ( request_id, - Err(ToolError::ExecutionError(format!( - "Failed to update vector index: {}", - e - ))), + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to update vector index: {}", e), + None, + )), ); } } @@ -595,7 +611,7 @@ impl Agent { extension_name ))] }) - .map_err(|e| ToolError::ExecutionError(e.to_string())); + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); return (request_id, result); } @@ -604,19 +620,24 @@ impl Agent { Ok(None) => { return ( request_id, - Err(ToolError::ExecutionError(format!( + Err(ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + format!( "Extension '{}' not found. Please check the extension name and try again.", extension_name - ))), + ), + None, + )), ) } Err(e) => { return ( request_id, - Err(ToolError::ExecutionError(format!( - "Failed to get extension config: {}", - e - ))), + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to get extension config: {}", e), + None, + )), ) } }; @@ -629,7 +650,7 @@ impl Agent { extension_name ))] }) - .map_err(|e| ToolError::ExecutionError(e.to_string())); + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); drop(extension_manager); // Update vector index if operation was successful and vector routing is enabled @@ -650,10 +671,11 @@ impl Agent { { return ( request_id, - Err(ToolError::ExecutionError(format!( - "Failed to update vector index: {}", - e - ))), + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to update vector index: {}", e), + None, + )), ); } } diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 1d0f75ea3e1d..5e35c59ac13e 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc}; use futures::stream::{FuturesUnordered, StreamExt}; use futures::{future, FutureExt}; use mcp_core::handler::require_str_parameter; -use mcp_core::{ToolCall, ToolError}; +use mcp_core::ToolCall; use rmcp::service::ClientInitializeError; use rmcp::transport::streamable_http_client::StreamableHttpClientTransportConfig; use rmcp::transport::{ @@ -30,7 +30,7 @@ use crate::config::{Config, ExtensionConfigManager}; use crate::oauth::oauth_flow; use crate::prompt_template; use mcp_client::client::{McpClient, McpClientTrait}; -use rmcp::model::{Content, GetPromptResult, Prompt, ResourceContents, Tool}; +use rmcp::model::{Content, ErrorCode, ErrorData, GetPromptResult, Prompt, ResourceContents, Tool}; use rmcp::transport::auth::AuthClient; use serde_json::Value; @@ -546,7 +546,7 @@ impl ExtensionManager { &self, params: Value, cancellation_token: CancellationToken, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let uri = require_str_parameter(¶ms, "uri")?; let extension_name = params.get("extension_name").and_then(|v| v.as_str()); @@ -588,7 +588,11 @@ impl ExtensionManager { uri, available_extensions ); - Err(ToolError::InvalidParameters(error_msg)) + Err(ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + error_msg, + None, + )) } async fn read_resource_from_extension( @@ -596,7 +600,7 @@ impl ExtensionManager { uri: &str, extension_name: &str, cancellation_token: CancellationToken, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let available_extensions = self .clients .keys() @@ -608,17 +612,22 @@ impl ExtensionManager { extension_name, available_extensions ); - let client = self - .clients - .get(extension_name) - .ok_or(ToolError::InvalidParameters(error_msg))?; + let client = self.clients.get(extension_name).ok_or(ErrorData::new( + ErrorCode::INVALID_PARAMS, + error_msg, + None, + ))?; let client_guard = client.lock().await; let read_result = client_guard .read_resource(uri, cancellation_token) .await .map_err(|_| { - ToolError::ExecutionError(format!("Could not read resource with uri: {}", uri)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Could not read resource with uri: {}", uri), + None, + ) })?; let mut result = Vec::new(); @@ -637,9 +646,13 @@ impl ExtensionManager { &self, extension_name: &str, cancellation_token: CancellationToken, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let client = self.clients.get(extension_name).ok_or_else(|| { - ToolError::InvalidParameters(format!("Extension {} is not valid", extension_name)) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("Extension {} is not valid", extension_name), + None, + ) })?; let client_guard = client.lock().await; @@ -647,10 +660,11 @@ impl ExtensionManager { .list_resources(None, cancellation_token) .await .map_err(|e| { - ToolError::ExecutionError(format!( - "Unable to list resources for {}, {:?}", - extension_name, e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Unable to list resources for {}, {:?}", extension_name, e), + None, + ) }) .map(|lr| { let resource_list = lr @@ -668,7 +682,7 @@ impl ExtensionManager { &self, params: Value, cancellation_token: CancellationToken, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let extension = params.get("extension").and_then(|v| v.as_str()); match extension { @@ -727,16 +741,18 @@ impl ExtensionManager { cancellation_token: CancellationToken, ) -> Result { // Dispatch tool call based on the prefix naming convention - let (client_name, client) = self - .get_client_for_tool(&tool_call.name) - .ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))?; + let (client_name, client) = self.get_client_for_tool(&tool_call.name).ok_or_else(|| { + ErrorData::new(ErrorCode::RESOURCE_NOT_FOUND, tool_call.name.clone(), None) + })?; // rsplit returns the iterator in reverse, tool_name is then at 0 let tool_name = tool_call .name .strip_prefix(client_name) .and_then(|s| s.strip_prefix("__")) - .ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))? + .ok_or_else(|| { + ErrorData::new(ErrorCode::RESOURCE_NOT_FOUND, tool_call.name.clone(), None) + })? .to_string(); let arguments = tool_call.arguments.clone(); @@ -749,7 +765,7 @@ impl ExtensionManager { .call_tool(&tool_name, arguments, cancellation_token) .await .map(|call| call.content.unwrap_or_default()) - .map_err(|e| ToolError::ExecutionError(e.to_string())) + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)) }; Ok(ToolCallResult { @@ -762,9 +778,13 @@ impl ExtensionManager { &self, extension_name: &str, cancellation_token: CancellationToken, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let client = self.clients.get(extension_name).ok_or_else(|| { - ToolError::InvalidParameters(format!("Extension {} is not valid", extension_name)) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("Extension {} is not valid", extension_name), + None, + ) })?; let client_guard = client.lock().await; @@ -772,10 +792,11 @@ impl ExtensionManager { .list_prompts(None, cancellation_token) .await .map_err(|e| { - ToolError::ExecutionError(format!( - "Unable to list prompts for {}, {:?}", - extension_name, e - )) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Unable to list prompts for {}, {:?}", extension_name, e), + None, + ) }) .map(|lp| lp.prompts) } @@ -783,7 +804,7 @@ impl ExtensionManager { pub async fn list_prompts( &self, cancellation_token: CancellationToken, - ) -> Result>, ToolError> { + ) -> Result>, ErrorData> { let mut futures = FuturesUnordered::new(); for extension_name in self.clients.keys() { @@ -846,7 +867,7 @@ impl ExtensionManager { .map_err(|e| anyhow::anyhow!("Failed to get prompt: {}", e)) } - pub async fn search_available_extensions(&self) -> Result, ToolError> { + pub async fn search_available_extensions(&self) -> Result, ErrorData> { let mut output_parts = vec![]; // First get disabled extensions from current config @@ -1140,8 +1161,11 @@ mod tests { .result .await; assert!(matches!( - result.err().unwrap(), - ToolError::ExecutionError(_) + result, + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + .. + }) )); // this should error out, specifically with an ToolError::NotFound @@ -1155,10 +1179,10 @@ mod tests { .dispatch_tool_call(invalid_tool_call, CancellationToken::default()) .await; if let Err(err) = result { - let tool_err = err.downcast_ref::().expect("Expected ToolError"); - assert!(matches!(tool_err, ToolError::NotFound(_))); + let tool_err = err.downcast_ref::().expect("Expected ErrorData"); + assert_eq!(tool_err.code, ErrorCode::RESOURCE_NOT_FOUND); } else { - panic!("Expected ToolError::NotFound"); + panic!("Expected ErrorData with ErrorCode::RESOURCE_NOT_FOUND"); } } } diff --git a/crates/goose/src/agents/final_output_tool.rs b/crates/goose/src/agents/final_output_tool.rs index 8c2f2b969d87..14e5d862cc4b 100644 --- a/crates/goose/src/agents/final_output_tool.rs +++ b/crates/goose/src/agents/final_output_tool.rs @@ -1,9 +1,10 @@ use crate::agents::tool_execution::ToolCallResult; use crate::recipe::Response; use indoc::formatdoc; -use mcp_core::{ToolCall, ToolError}; -use rmcp::model::{Content, Tool, ToolAnnotations}; +use mcp_core::ToolCall; +use rmcp::model::{Content, ErrorCode, ErrorData, Tool, ToolAnnotations}; use serde_json::Value; +use std::borrow::Cow; pub const FINAL_OUTPUT_TOOL_NAME: &str = "recipe__final_output"; pub const FINAL_OUTPUT_CONTINUATION_MESSAGE: &str = @@ -127,13 +128,18 @@ impl FinalOutputTool { "Final output successfully collected.".to_string(), )])) } - Err(error) => ToolCallResult::from(Err(ToolError::InvalidParameters(error))), + Err(error) => ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(error), + data: None, + })), } } - _ => ToolCallResult::from(Err(ToolError::NotFound(format!( - "Unknown tool: {}", - tool_call.name - )))), + _ => ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!("Unknown tool: {}", tool_call.name)), + data: None, + })), } } diff --git a/crates/goose/src/agents/large_response_handler.rs b/crates/goose/src/agents/large_response_handler.rs index ff8066215874..f0b9c176136b 100644 --- a/crates/goose/src/agents/large_response_handler.rs +++ b/crates/goose/src/agents/large_response_handler.rs @@ -1,6 +1,5 @@ use chrono::Utc; -use mcp_core::ToolError; -use rmcp::model::Content; +use rmcp::model::{Content, ErrorData}; use std::fs::File; use std::io::Write; @@ -8,8 +7,8 @@ const LARGE_TEXT_THRESHOLD: usize = 200_000; /// Process tool response and handle large text content pub fn process_tool_response( - response: Result, ToolError>, -) -> Result, ToolError> { + response: Result, ErrorData>, +) -> Result, ErrorData> { match response { Ok(contents) => { let mut processed_contents = Vec::new(); @@ -79,8 +78,8 @@ fn write_large_text_to_file(content: &str) -> Result { #[cfg(test)] mod tests { use super::*; - use mcp_core::ToolError; - use rmcp::model::Content; + use rmcp::model::{Content, ErrorCode, ErrorData}; + use std::borrow::Cow; use std::fs; use std::path::Path; @@ -213,8 +212,12 @@ mod tests { #[test] fn test_error_response_passes_through() { // Create an error response - let error = ToolError::ExecutionError("Test error".to_string()); - let response: Result, ToolError> = Err(error); + let error = ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Test error"), + data: None, + }; + let response: Result, ErrorData> = Err(error); // Process the response let processed = process_tool_response(response); @@ -222,8 +225,9 @@ mod tests { // Verify the error is passed through unchanged assert!(processed.is_err()); match processed { - Err(ToolError::ExecutionError(msg)) => { - assert_eq!(msg, "Test error"); + Err(err) => { + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); + assert_eq!(err.message, "Test error"); } _ => panic!("Expected execution error"), } diff --git a/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs b/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs index 55e11b6fe1c8..823235087da8 100644 --- a/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs +++ b/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs @@ -5,10 +5,10 @@ use crate::agents::subagent_execution_tool::tasks_manager::TasksManager; use crate::agents::subagent_execution_tool::{lib::ExecutionMode, task_types::Task}; use crate::agents::tool_execution::ToolCallResult; -use mcp_core::ToolError; -use rmcp::model::{Content, Tool, ToolAnnotations}; +use rmcp::model::{Content, ErrorCode, ErrorData, Tool, ToolAnnotations}; use rmcp::object; use serde_json::{json, Value}; +use std::borrow::Cow; pub const DYNAMIC_TASK_TOOL_NAME_PREFIX: &str = "dynamic_task__create_task"; @@ -110,9 +110,11 @@ pub async fn create_dynamic_task(params: Value, tasks_manager: &TasksManager) -> let task_params_array = extract_task_parameters(¶ms); if task_params_array.is_empty() { - return ToolCallResult::from(Err(ToolError::ExecutionError( - "No task parameters provided".to_string(), - ))); + return ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("No task parameters provided"), + data: None, + })); } let tasks = create_text_instruction_tasks_from_params(&task_params_array); @@ -129,10 +131,11 @@ pub async fn create_dynamic_task(params: Value, tasks_manager: &TasksManager) -> let tasks_json = match serde_json::to_string(&task_execution_payload) { Ok(json) => json, Err(e) => { - return ToolCallResult::from(Err(ToolError::ExecutionError(format!( - "Failed to serialize task list: {}", - e - )))) + return ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to serialize task list: {}", e)), + data: None, + })) } }; tasks_manager.save_tasks(tasks.clone()).await; diff --git a/crates/goose/src/agents/router_tool_selector.rs b/crates/goose/src/agents/router_tool_selector.rs index d3090d8b7826..f1ec402f0868 100644 --- a/crates/goose/src/agents/router_tool_selector.rs +++ b/crates/goose/src/agents/router_tool_selector.rs @@ -1,11 +1,11 @@ -use mcp_core::ToolError; -use rmcp::model::Content; use rmcp::model::Tool; +use rmcp::model::{Content, ErrorCode, ErrorData}; use anyhow::{Context, Result}; use async_trait::async_trait; use serde::Serialize; use serde_json::Value; +use std::borrow::Cow; use std::collections::HashMap; use std::collections::VecDeque; use std::env; @@ -32,11 +32,11 @@ pub enum RouterToolSelectionStrategy { #[async_trait] pub trait RouterToolSelector: Send + Sync { - async fn select_tools(&self, params: Value) -> Result, ToolError>; - async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ToolError>; - async fn remove_tool(&self, tool_name: &str) -> Result<(), ToolError>; - async fn record_tool_call(&self, tool_name: &str) -> Result<(), ToolError>; - async fn get_recent_tool_calls(&self, limit: usize) -> Result, ToolError>; + async fn select_tools(&self, params: Value) -> Result, ErrorData>; + async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ErrorData>; + async fn remove_tool(&self, tool_name: &str) -> Result<(), ErrorData>; + async fn record_tool_call(&self, tool_name: &str) -> Result<(), ErrorData>; + async fn get_recent_tool_calls(&self, limit: usize) -> Result, ErrorData>; fn selector_type(&self) -> RouterToolSelectionStrategy; } @@ -80,11 +80,15 @@ impl VectorToolSelector { #[async_trait] impl RouterToolSelector for VectorToolSelector { - async fn select_tools(&self, params: Value) -> Result, ToolError> { + async fn select_tools(&self, params: Value) -> Result, ErrorData> { let query = params .get("query") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'query' parameter".to_string()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'query' parameter"), + data: None, + })?; let k = params.get("k").and_then(|v| v.as_u64()).unwrap_or(5) as usize; @@ -93,29 +97,38 @@ impl RouterToolSelector for VectorToolSelector { // Check if provider supports embeddings if !self.embedding_provider.supports_embeddings() { - return Err(ToolError::ExecutionError( - "Embedding provider does not support embeddings".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Embedding provider does not support embeddings"), + data: None, + }); } let embeddings = self .embedding_provider .create_embeddings(vec![query.to_string()]) .await - .map_err(|e| { - ToolError::ExecutionError(format!("Failed to generate query embedding: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to generate query embedding: {}", e)), + data: None, })?; - let query_embedding = embeddings - .into_iter() - .next() - .ok_or_else(|| ToolError::ExecutionError("No embedding returned".to_string()))?; + let query_embedding = embeddings.into_iter().next().ok_or_else(|| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("No embedding returned"), + data: None, + })?; let vector_db = self.vector_db.read().await; let tools = vector_db .search_tools(query_embedding, k, extension_name) .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to search tools: {}", e)))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to search tools: {}", e)), + data: None, + })?; let selected_tools: Vec = tools .into_iter() @@ -131,7 +144,7 @@ impl RouterToolSelector for VectorToolSelector { Ok(selected_tools) } - async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ToolError> { + async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ErrorData> { let texts_to_embed: Vec = tools .iter() .map(|tool| { @@ -150,17 +163,21 @@ impl RouterToolSelector for VectorToolSelector { .collect(); if !self.embedding_provider.supports_embeddings() { - return Err(ToolError::ExecutionError( - "Embedding provider does not support embeddings".to_string(), - )); + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Embedding provider does not support embeddings"), + data: None, + }); } let embeddings = self .embedding_provider .create_embeddings(texts_to_embed) .await - .map_err(|e| { - ToolError::ExecutionError(format!("Failed to generate tool embeddings: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to generate tool embeddings: {}", e)), + data: None, })?; // Create tool records @@ -194,8 +211,10 @@ impl RouterToolSelector for VectorToolSelector { let existing_tools = vector_db .search_tools(record.vector.clone(), 1, Some(&record.extension_name)) .await - .map_err(|e| { - ToolError::ExecutionError(format!("Failed to search for existing tools: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to search for existing tools: {}", e)), + data: None, })?; // Only add if no exact match found @@ -212,21 +231,30 @@ impl RouterToolSelector for VectorToolSelector { vector_db .index_tools(new_tool_records) .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to index tools: {}", e)))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to index tools: {}", e)), + data: None, + })?; } Ok(()) } - async fn remove_tool(&self, tool_name: &str) -> Result<(), ToolError> { + async fn remove_tool(&self, tool_name: &str) -> Result<(), ErrorData> { let vector_db = self.vector_db.read().await; - vector_db.remove_tool(tool_name).await.map_err(|e| { - ToolError::ExecutionError(format!("Failed to remove tool {}: {}", tool_name, e)) - })?; + vector_db + .remove_tool(tool_name) + .await + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to remove tool {}: {}", tool_name, e)), + data: None, + })?; Ok(()) } - async fn record_tool_call(&self, tool_name: &str) -> Result<(), ToolError> { + async fn record_tool_call(&self, tool_name: &str) -> Result<(), ErrorData> { let mut recent_calls = self.recent_tool_calls.write().await; if recent_calls.len() >= 100 { recent_calls.pop_front(); @@ -235,7 +263,7 @@ impl RouterToolSelector for VectorToolSelector { Ok(()) } - async fn get_recent_tool_calls(&self, limit: usize) -> Result, ToolError> { + async fn get_recent_tool_calls(&self, limit: usize) -> Result, ErrorData> { let recent_calls = self.recent_tool_calls.read().await; Ok(recent_calls.iter().rev().take(limit).cloned().collect()) } @@ -263,11 +291,15 @@ impl LLMToolSelector { #[async_trait] impl RouterToolSelector for LLMToolSelector { - async fn select_tools(&self, params: Value) -> Result, ToolError> { + async fn select_tools(&self, params: Value) -> Result, ErrorData> { let query = params .get("query") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::InvalidParameters("Missing 'query' parameter".to_string()))?; + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("Missing 'query' parameter"), + data: None, + })?; let extension_name = params .get("extension_name") @@ -297,8 +329,10 @@ impl RouterToolSelector for LLMToolSelector { }; let user_prompt = - render_global_file("router_tool_selector.md", &context).map_err(|e| { - ToolError::ExecutionError(format!("Failed to render prompt template: {}", e)) + render_global_file("router_tool_selector.md", &context).map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to render prompt template: {}", e)), + data: None, })?; let user_message = Message::user().with_text(&user_prompt); @@ -306,7 +340,11 @@ impl RouterToolSelector for LLMToolSelector { .llm_provider .complete("", &[user_message], &[]) .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to search tools: {}", e)))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to search tools: {}", e)), + data: None, + })?; // Extract just the message content from the response let (message, _usage) = response; @@ -325,7 +363,7 @@ impl RouterToolSelector for LLMToolSelector { } } - async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ToolError> { + async fn index_tools(&self, tools: &[Tool], extension_name: &str) -> Result<(), ErrorData> { let mut tool_strings = self.tool_strings.write().await; for tool in tools { @@ -354,7 +392,7 @@ impl RouterToolSelector for LLMToolSelector { Ok(()) } - async fn remove_tool(&self, tool_name: &str) -> Result<(), ToolError> { + async fn remove_tool(&self, tool_name: &str) -> Result<(), ErrorData> { let mut tool_strings = self.tool_strings.write().await; if let Some(extension_name) = tool_name.split("__").next() { tool_strings.remove(extension_name); @@ -362,7 +400,7 @@ impl RouterToolSelector for LLMToolSelector { Ok(()) } - async fn record_tool_call(&self, tool_name: &str) -> Result<(), ToolError> { + async fn record_tool_call(&self, tool_name: &str) -> Result<(), ErrorData> { let mut recent_calls = self.recent_tool_calls.write().await; if recent_calls.len() >= 100 { recent_calls.pop_front(); @@ -371,7 +409,7 @@ impl RouterToolSelector for LLMToolSelector { Ok(()) } - async fn get_recent_tool_calls(&self, limit: usize) -> Result, ToolError> { + async fn get_recent_tool_calls(&self, limit: usize) -> Result, ErrorData> { let recent_calls = self.recent_tool_calls.read().await; Ok(recent_calls.iter().rev().take(limit).cloned().collect()) } diff --git a/crates/goose/src/agents/schedule_tool.rs b/crates/goose/src/agents/schedule_tool.rs index 30210544185e..7533ede8b03e 100644 --- a/crates/goose/src/agents/schedule_tool.rs +++ b/crates/goose/src/agents/schedule_tool.rs @@ -6,8 +6,8 @@ use std::sync::Arc; use chrono::Utc; -use mcp_core::{ToolError, ToolResult}; -use rmcp::model::Content; +use mcp_core::ToolResult; +use rmcp::model::{Content, ErrorCode, ErrorData}; use crate::recipe::Recipe; use crate::scheduler_trait::SchedulerTrait; @@ -24,8 +24,10 @@ impl Agent { let scheduler = match self.scheduler_service.lock().await.as_ref() { Some(s) => s.clone(), None => { - return Err(ToolError::ExecutionError( + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "Scheduler not available. This tool only works in server mode.".to_string(), + None, )) } }; @@ -33,7 +35,13 @@ impl Agent { let action = arguments .get("action") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'action' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'action' parameter".to_string(), + None, + ) + })?; match action { "list" => self.handle_list_jobs(scheduler).await, @@ -46,10 +54,11 @@ impl Agent { "inspect" => self.handle_inspect_job(scheduler, arguments).await, "sessions" => self.handle_list_sessions(scheduler, arguments).await, "session_content" => self.handle_session_content(arguments).await, - _ => Err(ToolError::ExecutionError(format!( - "Unknown action: {}", - action - ))), + _ => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Unknown action: {}", action), + None, + )), } } @@ -61,17 +70,22 @@ impl Agent { match scheduler.list_scheduled_jobs().await { Ok(jobs) => { let jobs_json = serde_json::to_string_pretty(&jobs).map_err(|e| { - ToolError::ExecutionError(format!("Failed to serialize jobs: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to serialize jobs: {}", e), + None, + ) })?; Ok(vec![Content::text(format!( "Scheduled Jobs:\n{}", jobs_json ))]) } - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to list jobs: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to list jobs: {}", e), + None, + )), } } @@ -85,14 +99,22 @@ impl Agent { .get("recipe_path") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::ExecutionError("Missing 'recipe_path' parameter".to_string()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'recipe_path' parameter".to_string(), + None, + ) })?; let cron_expression = arguments .get("cron_expression") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::ExecutionError("Missing 'cron_expression' parameter".to_string()) + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'cron_expression' parameter".to_string(), + None, + ) })?; // Get the execution_mode parameter, defaulting to "background" if not provided @@ -103,18 +125,23 @@ impl Agent { // Validate execution_mode is either "foreground" or "background" if execution_mode != "foreground" && execution_mode != "background" { - return Err(ToolError::ExecutionError(format!( - "Invalid execution_mode: {}. Must be 'foreground' or 'background'", - execution_mode - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Invalid execution_mode: {}. Must be 'foreground' or 'background'", + execution_mode + ), + None, + )); } // Validate recipe file exists and is readable if !std::path::Path::new(recipe_path).exists() { - return Err(ToolError::ExecutionError(format!( - "Recipe file not found: {}", - recipe_path - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Recipe file not found: {}", recipe_path), + None, + )); } // Validate it's a valid recipe by trying to parse it @@ -122,19 +149,28 @@ impl Agent { Ok(content) => { if recipe_path.ends_with(".json") { serde_json::from_str::(&content).map_err(|e| { - ToolError::ExecutionError(format!("Invalid JSON recipe: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Invalid JSON recipe: {}", e), + None, + ) })?; } else { serde_yaml::from_str::(&content).map_err(|e| { - ToolError::ExecutionError(format!("Invalid YAML recipe: {}", e)) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Invalid YAML recipe: {}", e), + None, + ) })?; } } Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Cannot read recipe file: {}", - e - ))) + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Cannot read recipe file: {}", e), + None, + )) } } @@ -158,10 +194,11 @@ impl Agent { "Successfully created scheduled job '{}' for recipe '{}' with cron expression '{}' in {} mode", job_id, recipe_path, cron_expression, execution_mode ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to create job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to create job: {}", e), + None, + )), } } @@ -174,17 +211,24 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.run_now(job_id).await { Ok(session_id) => Ok(vec![Content::text(format!( "Successfully started job '{}'. Session ID: {}", job_id, session_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to run job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to run job: {}", e), + None, + )), } } @@ -197,17 +241,24 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.pause_schedule(job_id).await { Ok(()) => Ok(vec![Content::text(format!( "Successfully paused job '{}'", job_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to pause job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to pause job: {}", e), + None, + )), } } @@ -220,17 +271,24 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.unpause_schedule(job_id).await { Ok(()) => Ok(vec![Content::text(format!( "Successfully unpaused job '{}'", job_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to unpause job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to unpause job: {}", e), + None, + )), } } @@ -243,17 +301,24 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.remove_scheduled_job(job_id).await { Ok(()) => Ok(vec![Content::text(format!( "Successfully deleted job '{}'", job_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to delete job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to delete job: {}", e), + None, + )), } } @@ -266,17 +331,24 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.kill_running_job(job_id).await { Ok(()) => Ok(vec![Content::text(format!( "Successfully killed running job '{}'", job_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to kill job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to kill job: {}", e), + None, + )), } } @@ -289,7 +361,13 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; match scheduler.get_running_job_info(job_id).await { Ok(Some((session_id, start_time))) => { @@ -303,10 +381,11 @@ impl Agent { "Job '{}' is not currently running", job_id ))]), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to inspect job: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to inspect job: {}", e), + None, + )), } } @@ -319,7 +398,13 @@ impl Agent { let job_id = arguments .get("job_id") .and_then(|v| v.as_str()) - .ok_or_else(|| ToolError::ExecutionError("Missing 'job_id' parameter".to_string()))?; + .ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'job_id' parameter".to_string(), + None, + ) + })?; let limit = arguments .get("limit") @@ -353,10 +438,11 @@ impl Agent { ))]) } } - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to list sessions: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to list sessions: {}", e), + None, + )), } } @@ -369,7 +455,11 @@ impl Agent { .get("session_id") .and_then(|v| v.as_str()) .ok_or_else(|| { - ToolError::ExecutionError("Missing 'session_id' parameter".to_string()) + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Missing 'session_id' parameter".to_string(), + None, + ) })?; // Get the session file path @@ -378,29 +468,32 @@ impl Agent { ) { Ok(path) => path, Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Invalid session ID '{}': {}", - session_id, e - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Invalid session ID '{}': {}", session_id, e), + None, + )); } }; // Check if session file exists if !session_path.exists() { - return Err(ToolError::ExecutionError(format!( - "Session '{}' not found", - session_id - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Session '{}' not found", session_id), + None, + )); } // Read session metadata let metadata = match crate::session::storage::read_metadata(&session_path) { Ok(metadata) => metadata, Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to read session metadata: {}", - e - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read session metadata: {}", e), + None, + )); } }; @@ -408,10 +501,11 @@ impl Agent { let messages = match crate::session::storage::read_messages(&session_path) { Ok(messages) => messages, Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to read session messages: {}", - e - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read session messages: {}", e), + None, + )); } }; @@ -419,20 +513,22 @@ impl Agent { let metadata_json = match serde_json::to_string_pretty(&metadata) { Ok(json) => json, Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to serialize metadata: {}", - e - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to serialize metadata: {}", e), + None, + )); } }; let messages_json = match serde_json::to_string_pretty(&messages) { Ok(json) => json, Err(e) => { - return Err(ToolError::ExecutionError(format!( - "Failed to serialize messages: {}", - e - ))); + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to serialize messages: {}", e), + None, + )); } }; diff --git a/crates/goose/src/agents/sub_recipe_manager.rs b/crates/goose/src/agents/sub_recipe_manager.rs index 314c3c41b616..26cf4eb44183 100644 --- a/crates/goose/src/agents/sub_recipe_manager.rs +++ b/crates/goose/src/agents/sub_recipe_manager.rs @@ -1,7 +1,7 @@ -use mcp_core::ToolError; -use rmcp::model::Content; use rmcp::model::Tool; +use rmcp::model::{Content, ErrorCode, ErrorData}; use serde_json::Value; +use std::borrow::Cow; use std::collections::HashMap; use crate::{ @@ -63,7 +63,11 @@ impl SubRecipeManager { .await; match result { Ok(call_result) => ToolCallResult::from(Ok(call_result)), - Err(e) => ToolCallResult::from(Err(ToolError::ExecutionError(e.to_string()))), + Err(e) => ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + })), } } @@ -72,25 +76,33 @@ impl SubRecipeManager { tool_name: &str, params: Value, tasks_manager: &TasksManager, - ) -> Result, ToolError> { + ) -> Result, ErrorData> { let sub_recipe = self.sub_recipes.get(tool_name).ok_or_else(|| { let sub_recipe_name = tool_name .strip_prefix(SUB_RECIPE_TASK_TOOL_NAME_PREFIX) .and_then(|s| s.strip_prefix("_")) - .ok_or_else(|| { - ToolError::InvalidParameters(format!( + .ok_or_else(|| ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( "Invalid sub-recipe tool name format: {}", tool_name - )) + )), + data: None, }) .unwrap(); - ToolError::InvalidParameters(format!("Sub-recipe '{}' not found", sub_recipe_name)) + ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Sub-recipe '{}' not found", sub_recipe_name)), + data: None, + } })?; let output = create_sub_recipe_task(sub_recipe, params, tasks_manager) .await - .map_err(|e| { - ToolError::ExecutionError(format!("Sub-recipe task createion failed: {}", e)) + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Sub-recipe task creation failed: {}", e)), + data: None, })?; Ok(vec![Content::text(output)]) } diff --git a/crates/goose/src/agents/subagent.rs b/crates/goose/src/agents/subagent.rs index dc8c43eb158a..5e4334338d0c 100644 --- a/crates/goose/src/agents/subagent.rs +++ b/crates/goose/src/agents/subagent.rs @@ -8,8 +8,8 @@ use crate::{ }; use anyhow::anyhow; use chrono::{DateTime, Utc}; -use mcp_core::handler::ToolError; use rmcp::model::Tool; +use rmcp::model::{ErrorCode, ErrorData}; use serde::{Deserialize, Serialize}; // use serde_json::{self}; use crate::conversation::message::{Message, MessageContent, ToolRequest}; @@ -206,7 +206,11 @@ impl SubAgent { .await { Ok(result) => result.result.await, - Err(e) => Err(ToolError::ExecutionError(e.to_string())), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + e.to_string(), + None, + )), }; match tool_result { @@ -220,7 +224,11 @@ impl SubAgent { // Create a user message with the tool error let tool_error_message = Message::user().with_tool_response( request.id.clone(), - Err(ToolError::ExecutionError(e.to_string())), + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + e.to_string(), + None, + )), ); messages.push(tool_error_message); } diff --git a/crates/goose/src/agents/subagent_execution_tool/subagent_execute_task_tool.rs b/crates/goose/src/agents/subagent_execution_tool/subagent_execute_task_tool.rs index 430d39420c54..474912f0c416 100644 --- a/crates/goose/src/agents/subagent_execution_tool/subagent_execute_task_tool.rs +++ b/crates/goose/src/agents/subagent_execution_tool/subagent_execute_task_tool.rs @@ -1,5 +1,6 @@ -use mcp_core::ToolError; -use rmcp::model::{Content, ServerNotification, Tool, ToolAnnotations}; +use std::borrow::Cow; + +use rmcp::model::{Content, ErrorCode, ErrorData, ServerNotification, Tool, ToolAnnotations}; use serde_json::Value; use crate::agents::subagent_task_config::TaskConfig; @@ -90,7 +91,11 @@ pub async fn run_tasks( let output = serde_json::to_string(&result).unwrap(); Ok(vec![Content::text(output)]) } - Err(e) => Err(ToolError::ExecutionError(e.to_string())), + Err(e) => Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string()), + data: None, + }), } }; diff --git a/crates/goose/src/agents/subagent_handler.rs b/crates/goose/src/agents/subagent_handler.rs index 525000b6ea83..80e36f41aa3e 100644 --- a/crates/goose/src/agents/subagent_handler.rs +++ b/crates/goose/src/agents/subagent_handler.rs @@ -1,7 +1,7 @@ use crate::agents::subagent::SubAgent; use crate::agents::subagent_task_config::TaskConfig; use anyhow::Result; -use mcp_core::ToolError; +use rmcp::model::{ErrorCode, ErrorData}; /// Standalone function to run a complete subagent task pub async fn run_complete_subagent_task( @@ -9,9 +9,13 @@ pub async fn run_complete_subagent_task( task_config: TaskConfig, ) -> Result { // Create the subagent with the parent agent's provider - let subagent = SubAgent::new(task_config.clone()) - .await - .map_err(|e| ToolError::ExecutionError(format!("Failed to create subagent: {}", e)))?; + let subagent = SubAgent::new(task_config.clone()).await.map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to create subagent: {}", e), + None, + ) + })?; // Execute the subagent task let messages = subagent diff --git a/crates/goose/src/agents/tool_route_manager.rs b/crates/goose/src/agents/tool_route_manager.rs index 5829d582574c..6c1be91fc92f 100644 --- a/crates/goose/src/agents/tool_route_manager.rs +++ b/crates/goose/src/agents/tool_route_manager.rs @@ -10,8 +10,7 @@ use crate::config::Config; use crate::conversation::message::ToolRequest; use crate::providers::base::Provider; use anyhow::{anyhow, Result}; -use mcp_core::ToolError; -use rmcp::model::Tool; +use rmcp::model::{ErrorCode, ErrorData, Tool}; use serde_json::Value; use std::sync::Arc; use tokio::sync::Mutex; @@ -52,18 +51,21 @@ impl ToolRouteManager { pub async fn dispatch_route_search_tool( &self, arguments: Value, - ) -> Result { + ) -> Result { let selector = self.router_tool_selector.lock().await.clone(); match selector.as_ref() { Some(selector) => match selector.select_tools(arguments).await { Ok(tools) => Ok(ToolCallResult::from(Ok(tools))), - Err(e) => Err(ToolError::ExecutionError(format!( - "Failed to select tools: {}", - e - ))), + Err(e) => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to select tools: {}", e), + None, + )), }, - None => Err(ToolError::ExecutionError( + None => Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, "No tool selector available".to_string(), + None, )), } } diff --git a/crates/goose/src/conversation/message.rs b/crates/goose/src/conversation/message.rs index 800517165808..9bedd5370cf6 100644 --- a/crates/goose/src/conversation/message.rs +++ b/crates/goose/src/conversation/message.rs @@ -603,12 +603,12 @@ impl Message { mod tests { use crate::conversation::message::{Message, MessageContent}; use crate::conversation::*; - use mcp_core::handler::ToolError; use mcp_core::ToolCall; use rmcp::model::{ AnnotateAble, PromptMessage, PromptMessageContent, PromptMessageRole, RawEmbeddedResource, RawImageContent, ResourceContents, }; + use rmcp::model::{ErrorCode, ErrorData}; use serde_json::{json, Value}; #[test] @@ -683,9 +683,11 @@ mod tests { fn test_error_serialization() { let message = Message::assistant().with_tool_request( "tool123", - Err(ToolError::ExecutionError( - "Something went wrong".to_string(), - )), + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: std::borrow::Cow::from("Something went wrong".to_string()), + data: None, + }), ); let json_str = serde_json::to_string_pretty(&message).unwrap(); @@ -697,7 +699,7 @@ mod tests { // Check tool_call serialization with error let tool_call = &value["content"][0]["toolCall"]; assert_eq!(tool_call["status"], "error"); - assert_eq!(tool_call["error"], "Execution failed: Something went wrong"); + assert_eq!(tool_call["error"], "-32603: Something went wrong"); } #[test] diff --git a/crates/goose/src/conversation/tool_result_serde.rs b/crates/goose/src/conversation/tool_result_serde.rs index cf7feb5479c0..a151aeede3bc 100644 --- a/crates/goose/src/conversation/tool_result_serde.rs +++ b/crates/goose/src/conversation/tool_result_serde.rs @@ -1,6 +1,8 @@ -use mcp_core::handler::{ToolError, ToolResult}; +use mcp_core::ToolResult; +use rmcp::model::{ErrorCode, ErrorData}; use serde::ser::SerializeStruct; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use std::borrow::Cow; pub fn serialize(value: &ToolResult, serializer: S) -> Result where @@ -52,7 +54,11 @@ where } ResultFormat::Error { status, error } => { if status == "error" { - Ok(Err(ToolError::ExecutionError(error))) + Ok(Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(error), + data: None, + })) } else { Err(serde::de::Error::custom(format!( "Expected status 'error', got '{}'", diff --git a/crates/goose/src/providers/formats/anthropic.rs b/crates/goose/src/providers/formats/anthropic.rs index 206e75c5e576..639b60391272 100644 --- a/crates/goose/src/providers/formats/anthropic.rs +++ b/crates/goose/src/providers/formats/anthropic.rs @@ -3,8 +3,8 @@ use crate::model::ModelConfig; use crate::providers::base::Usage; use crate::providers::errors::ProviderError; use anyhow::{anyhow, Result}; -use mcp_core::tool::ToolCall; -use rmcp::model::{Role, Tool}; +use mcp_core::ToolCall; +use rmcp::model::{ErrorCode, ErrorData, Role, Tool}; use serde_json::{json, Value}; use std::collections::HashSet; @@ -572,8 +572,10 @@ where Ok(parsed) => parsed, Err(_) => { // If parsing fails, create an error tool request - let error = mcp_core::handler::ToolError::InvalidParameters( - format!("Could not parse tool arguments: {}", args) + let error = ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("Could not parse tool arguments: {}", args), + None, ); let mut message = Message::new( Role::Assistant, @@ -985,7 +987,7 @@ mod tests { #[test] fn test_tool_error_handling_maintains_pairing() { use crate::conversation::message::Message; - use mcp_core::handler::ToolError; + use rmcp::model::{ErrorCode, ErrorData}; let messages = vec![ Message::assistant().with_tool_request( @@ -994,7 +996,11 @@ mod tests { ), Message::user().with_tool_response( "tool_1", - Err(ToolError::ExecutionError("Tool failed".to_string())), + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + "Tool failed".to_string(), + None, + )), ), ]; @@ -1012,7 +1018,7 @@ mod tests { assert_eq!(spec[1]["content"][0]["tool_use_id"], "tool_1"); assert_eq!( spec[1]["content"][0]["content"], - "Error: Execution failed: Tool failed" + "Error: -32603: Tool failed" ); assert_eq!(spec[1]["content"][0]["is_error"], true); } diff --git a/crates/goose/src/providers/formats/bedrock.rs b/crates/goose/src/providers/formats/bedrock.rs index a3f108e0f966..211b78ee5c0e 100644 --- a/crates/goose/src/providers/formats/bedrock.rs +++ b/crates/goose/src/providers/formats/bedrock.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::path::Path; @@ -6,8 +7,8 @@ use aws_sdk_bedrockruntime::types as bedrock; use aws_smithy_types::{Document, Number}; use base64::Engine; use chrono::Utc; -use mcp_core::{ToolCall, ToolError, ToolResult}; -use rmcp::model::{Content, RawContent, ResourceContents, Role, Tool}; +use mcp_core::{ToolCall, ToolResult}; +use rmcp::model::{Content, ErrorCode, ErrorData, RawContent, ResourceContents, Role, Tool}; use serde_json::Value; use super::super::base::Usage; @@ -286,9 +287,11 @@ pub fn from_bedrock_content_block(block: &bedrock::ContentBlock) -> Result MessageContent::tool_response( tool_res.tool_use_id.to_string(), if tool_res.content.is_empty() { - Err(ToolError::ExecutionError( - "Empty content for tool use from Bedrock".to_string(), - )) + Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Empty content for tool use from Bedrock".to_string()), + data: None, + }) } else { tool_res .content @@ -307,9 +310,11 @@ pub fn from_bedrock_tool_result_content_block( Ok(match content { bedrock::ToolResultContentBlock::Text(text) => Content::text(text.to_string()), _ => { - return Err(ToolError::ExecutionError( - "Unsupported tool result from Bedrock".to_string(), - )) + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from("Unsupported tool result from Bedrock".to_string()), + data: None, + }) } }) } diff --git a/crates/goose/src/providers/formats/databricks.rs b/crates/goose/src/providers/formats/databricks.rs index acd2ebaac3bc..06f052b59c2a 100644 --- a/crates/goose/src/providers/formats/databricks.rs +++ b/crates/goose/src/providers/formats/databricks.rs @@ -5,10 +5,13 @@ use crate::providers::utils::{ sanitize_function_name, ImageFormat, }; use anyhow::{anyhow, Error}; -use mcp_core::{ToolCall, ToolError}; -use rmcp::model::{AnnotateAble, Content, RawContent, ResourceContents, Role, Tool}; +use mcp_core::ToolCall; +use rmcp::model::{ + AnnotateAble, Content, ErrorCode, ErrorData, RawContent, ResourceContents, Role, Tool, +}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; +use std::borrow::Cow; #[derive(Serialize)] struct DatabricksMessage { @@ -356,10 +359,14 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { }; if !is_valid_function_name(&function_name) { - let error = ToolError::NotFound(format!( - "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", - function_name - )); + let error = ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!( + "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", + function_name + )), + data: None, + }; content.push(MessageContent::tool_request(id, Err(error))); } else { match safely_parse_json(&arguments_str) { @@ -370,10 +377,14 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { )); } Err(e) => { - let error = ToolError::InvalidParameters(format!( - "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", - id, e, arguments_str - )); + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", + id, e, arguments_str + )), + data: None, + }; content.push(MessageContent::tool_request(id, Err(error))); } } @@ -963,7 +974,11 @@ mod tests { if let MessageContent::ToolRequest(request) = &message.content[0] { match &request.tool_call { - Err(ToolError::NotFound(msg)) => { + Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: msg, + data: None, + }) => { assert!(msg.starts_with("The provided function name")); } _ => panic!("Expected ToolNotFound error"), @@ -985,7 +1000,11 @@ mod tests { if let MessageContent::ToolRequest(request) = &message.content[0] { match &request.tool_call { - Err(ToolError::InvalidParameters(msg)) => { + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: msg, + data: None, + }) => { assert!(msg.starts_with("Could not interpret tool use parameters")); } _ => panic!("Expected InvalidParameters error"), diff --git a/crates/goose/src/providers/formats/google.rs b/crates/goose/src/providers/formats/google.rs index 1a2b40b446c8..e4e01e2a221e 100644 --- a/crates/goose/src/providers/formats/google.rs +++ b/crates/goose/src/providers/formats/google.rs @@ -3,9 +3,10 @@ use crate::providers::base::Usage; use crate::providers::errors::ProviderError; use crate::providers::utils::{is_valid_function_name, sanitize_function_name}; use anyhow::Result; -use mcp_core::tool::ToolCall; +use mcp_core::ToolCall; use rand::{distributions::Alphanumeric, Rng}; -use rmcp::model::{AnnotateAble, RawContent, Role, Tool}; +use rmcp::model::{AnnotateAble, ErrorCode, ErrorData, RawContent, Role, Tool}; +use std::borrow::Cow; use crate::conversation::message::{Message, MessageContent}; use serde_json::{json, Map, Value}; @@ -254,10 +255,14 @@ pub fn response_to_message(response: Value) -> Result { .unwrap_or_default() .to_string(); if !is_valid_function_name(&name) { - let error = mcp_core::ToolError::NotFound(format!( - "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", - name - )); + let error = ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!( + "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", + name + )), + data: None, + }; content.push(MessageContent::tool_request(id, Err(error))); } else { let parameters = function_call.get("args"); @@ -741,7 +746,14 @@ mod tests { assert_eq!(message.role, Role::Assistant); assert_eq!(message.content.len(), 1); if let Err(error) = &message.content[0].as_tool_request().unwrap().tool_call { - assert!(matches!(error, mcp_core::ToolError::NotFound(_))); + assert!(matches!( + error, + ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: _, + data: None, + } + )); } else { panic!("Expected tool request error"); } diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 888abc05a137..e5bdfdb08dfc 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -8,10 +8,13 @@ use crate::providers::utils::{ use anyhow::{anyhow, Error}; use async_stream::try_stream; use futures::Stream; -use mcp_core::{ToolCall, ToolError}; -use rmcp::model::{AnnotateAble, Content, RawContent, ResourceContents, Role, Tool}; +use mcp_core::ToolCall; +use rmcp::model::{ + AnnotateAble, Content, ErrorCode, ErrorData, RawContent, ResourceContents, Role, Tool, +}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; +use std::borrow::Cow; use std::ops::Deref; #[derive(Serialize, Deserialize, Debug)] @@ -299,10 +302,14 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { }; if !is_valid_function_name(&function_name) { - let error = ToolError::NotFound(format!( - "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", - function_name - )); + let error = ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!( + "The provided function name '{}' had invalid characters, it must match this regex [a-zA-Z0-9_-]+", + function_name + )), + data: None, + }; content.push(MessageContent::tool_request(id, Err(error))); } else { match safely_parse_json(&arguments_str) { @@ -313,10 +320,14 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { )); } Err(e) => { - let error = ToolError::InvalidParameters(format!( - "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", - id, e, arguments_str - )); + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", + id, e, arguments_str + )), + data: None, + }; content.push(MessageContent::tool_request(id, Err(error))); } } @@ -508,10 +519,14 @@ where ) }, Err(e) => { - let error = ToolError::InvalidParameters(format!( - "Could not interpret tool use parameters for id {}: {}", - id, e - )); + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Could not interpret tool use parameters for id {}: {}", + id, e + )), + data: None, + }; MessageContent::tool_request(id.clone(), Err(error)) } }; @@ -991,7 +1006,11 @@ mod tests { if let MessageContent::ToolRequest(request) = &message.content[0] { match &request.tool_call { - Err(ToolError::NotFound(msg)) => { + Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: msg, + data: None, + }) => { assert!(msg.starts_with("The provided function name")); } _ => panic!("Expected ToolNotFound error"), @@ -1013,7 +1032,11 @@ mod tests { if let MessageContent::ToolRequest(request) = &message.content[0] { match &request.tool_call { - Err(ToolError::InvalidParameters(msg)) => { + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: msg, + data: None, + }) => { assert!(msg.starts_with("Could not interpret tool use parameters")); } _ => panic!("Expected InvalidParameters error"), diff --git a/crates/goose/tests/private_tests.rs b/crates/goose/tests/private_tests.rs index 6242730af2cf..c38eb3191a22 100644 --- a/crates/goose/tests/private_tests.rs +++ b/crates/goose/tests/private_tests.rs @@ -1,6 +1,6 @@ #![cfg(test)] -use mcp_core::ToolError; +use rmcp::model::ErrorCode; use serde_json::json; use goose::agents::platform_tools::PLATFORM_MANAGE_SCHEDULE_TOOL_NAME; @@ -94,9 +94,10 @@ async fn test_schedule_tool_list_action_error() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Failed to list jobs")); - assert!(msg.contains("Database error")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); + assert!(err.message.contains("Failed to list jobs")); + assert!(err.message.contains("Database error")); } else { panic!("Expected ExecutionError"); } @@ -153,8 +154,9 @@ async fn test_schedule_tool_create_action_missing_params() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Missing 'recipe_path' parameter")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("Missing 'recipe_path' parameter")); } else { panic!("Expected ExecutionError"); } @@ -171,8 +173,9 @@ async fn test_schedule_tool_create_action_missing_params() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Missing 'cron_expression' parameter")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("Missing 'cron_expression' parameter")); } else { panic!("Expected ExecutionError"); } @@ -194,8 +197,9 @@ async fn test_schedule_tool_create_action_nonexistent_recipe() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Recipe file not found")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); + assert!(err.message.contains("Recipe file not found")); } else { panic!("Expected ExecutionError"); } @@ -220,8 +224,9 @@ async fn test_schedule_tool_create_action_invalid_recipe() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Invalid JSON recipe")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); + assert!(err.message.contains("Invalid JSON recipe")); } else { panic!("Expected ExecutionError"); } @@ -253,9 +258,10 @@ async fn test_schedule_tool_create_action_scheduler_error() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Failed to create job")); - assert!(msg.contains("job1")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INTERNAL_ERROR); + assert!(err.message.contains("Failed to create job")); + assert!(err.message.contains("job1")); } else { panic!("Expected ExecutionError"); } @@ -311,8 +317,9 @@ async fn test_schedule_tool_run_now_action_missing_job_id() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Missing 'job_id' parameter")); + if let Err(err) = result { + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("Missing 'job_id' parameter")); } else { panic!("Expected ExecutionError"); } @@ -337,9 +344,9 @@ async fn test_schedule_tool_run_now_action_nonexistent_job() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Failed to run job")); - assert!(msg.contains("nonexistent")); + if let Err(err) = result { + assert!(err.message.contains("Failed to run job")); + assert!(err.message.contains("nonexistent")); } else { panic!("Expected ExecutionError"); } @@ -391,10 +398,11 @@ async fn test_schedule_tool_pause_action_missing_job_id() { let result = agent .handle_schedule_management(arguments, "test_req".to_string()) .await; + assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Missing 'job_id' parameter")); + if let Err(err) = result { + assert!(err.message.contains("Missing 'job_id' parameter")); } else { panic!("Expected ExecutionError"); } @@ -422,9 +430,9 @@ async fn test_schedule_tool_pause_action_running_job() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Failed to pause job")); - assert!(msg.contains("job1")); + if let Err(err) = result { + assert!(err.message.contains("Failed to pause job")); + assert!(err.message.contains("job1")); } else { panic!("Expected ExecutionError"); } @@ -551,8 +559,8 @@ async fn test_schedule_tool_kill_action_not_running() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Failed to kill job")); + if let Err(err) = result { + assert!(err.message.contains("Failed to kill job")); } else { panic!("Expected ExecutionError"); } @@ -764,8 +772,10 @@ async fn test_schedule_tool_session_content_action() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Session 'non_existent_session' not found")); + if let Err(err) = result { + assert!(err + .message + .contains("Session 'non_existent_session' not found")); } else { panic!("Expected ExecutionError"); } @@ -840,8 +850,8 @@ async fn test_schedule_tool_session_content_action_missing_session_id() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Missing 'session_id' parameter")); + if let Err(err) = result { + assert!(err.message.contains("Missing 'session_id' parameter")); } else { panic!("Expected ExecutionError"); } @@ -861,8 +871,8 @@ async fn test_schedule_tool_unknown_action() { .await; assert!(result.is_err()); - if let Err(ToolError::ExecutionError(msg)) = result { - assert!(msg.contains("Unknown action")); + if let Err(err) = result { + assert!(err.message.contains("Unknown action")); } else { panic!("Expected ExecutionError"); } diff --git a/crates/mcp-core/src/handler.rs b/crates/mcp-core/src/handler.rs index 8d97a3fd1f5d..1c104a462cd7 100644 --- a/crates/mcp-core/src/handler.rs +++ b/crates/mcp-core/src/handler.rs @@ -1,22 +1,7 @@ -use serde::{Deserialize, Serialize}; -#[allow(unused_imports)] // this is used in schema below -use serde_json::{json, Value}; +use rmcp::model::{ErrorCode, ErrorData}; use thiserror::Error; -#[non_exhaustive] -#[derive(Error, Debug, Clone, Deserialize, Serialize, PartialEq)] -pub enum ToolError { - #[error("Invalid parameters: {0}")] - InvalidParameters(String), - #[error("Execution failed: {0}")] - ExecutionError(String), - #[error("Schema error: {0}")] - SchemaError(String), - #[error("Tool not found: {0}")] - NotFound(String), -} - -pub type ToolResult = std::result::Result; +pub type ToolResult = std::result::Result; #[derive(Error, Debug)] pub enum ResourceError { @@ -36,31 +21,43 @@ pub enum PromptError { NotFound(String), } -/// Helper function to require a string, returning a ToolError +/// Helper function to require a string, returning an ErrorData pub fn require_str_parameter<'a>( v: &'a serde_json::Value, name: &str, -) -> Result<&'a str, ToolError> { - let v = v - .get(name) - .ok_or_else(|| ToolError::InvalidParameters(format!("The parameter {name} is required")))?; +) -> Result<&'a str, ErrorData> { + let v = v.get(name).ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("The parameter {name} is required"), + None, + ) + })?; match v.as_str() { Some(r) => Ok(r), - None => Err(ToolError::InvalidParameters(format!( - "The parameter {name} must be a string" - ))), + None => Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("The parameter {name} must be a string"), + None, + )), } } -/// Helper function to require a u64, returning a ToolError -pub fn require_u64_parameter(v: &serde_json::Value, name: &str) -> Result { - let v = v - .get(name) - .ok_or_else(|| ToolError::InvalidParameters(format!("The parameter {name} is required")))?; +/// Helper function to require a u64, returning an ErrorData +pub fn require_u64_parameter(v: &serde_json::Value, name: &str) -> Result { + let v = v.get(name).ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("The parameter {name} is required"), + None, + ) + })?; match v.as_u64() { Some(r) => Ok(r), - None => Err(ToolError::InvalidParameters(format!( - "The parameter {name} must be a number" - ))), + None => Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!("The parameter {name} is required"), + None, + )), } } diff --git a/crates/mcp-core/src/lib.rs b/crates/mcp-core/src/lib.rs index 395d2652e261..079b0c7efebd 100644 --- a/crates/mcp-core/src/lib.rs +++ b/crates/mcp-core/src/lib.rs @@ -2,4 +2,4 @@ pub mod handler; pub mod tool; pub use tool::{Tool, ToolCall}; pub mod protocol; -pub use handler::{ToolError, ToolResult}; +pub use handler::ToolResult; diff --git a/crates/mcp-server/src/main.rs b/crates/mcp-server/src/main.rs index 78f712b14ef1..dd2cdfcc8171 100644 --- a/crates/mcp-server/src/main.rs +++ b/crates/mcp-server/src/main.rs @@ -1,13 +1,15 @@ use anyhow::Result; use mcp_core::handler::{PromptError, ResourceError}; -use mcp_core::{handler::ToolError, protocol::ServerCapabilities}; +use mcp_core::protocol::ServerCapabilities; use mcp_server::router::{CapabilitiesBuilder, RouterService}; use mcp_server::{ByteTransport, Router, Server}; use rmcp::model::{ - Content, JsonRpcMessage, Prompt, PromptArgument, RawResource, Resource, Tool, ToolAnnotations, + Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, PromptArgument, RawResource, Resource, + Tool, ToolAnnotations, }; use rmcp::object; use serde_json::Value; +use std::borrow::Cow; use std::{future::Future, pin::Pin, sync::Arc}; use tokio::sync::mpsc; use tokio::{ @@ -30,19 +32,19 @@ impl CounterRouter { } } - async fn increment(&self) -> Result { + async fn increment(&self) -> Result { let mut counter = self.counter.lock().await; *counter += 1; Ok(*counter) } - async fn decrement(&self) -> Result { + async fn decrement(&self) -> Result { let mut counter = self.counter.lock().await; *counter -= 1; Ok(*counter) } - async fn get_value(&self) -> Result { + async fn get_value(&self) -> Result { let counter = self.counter.lock().await; Ok(*counter) } @@ -127,7 +129,7 @@ impl Router for CounterRouter { tool_name: &str, _arguments: Value, _notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>> { + ) -> Pin, ErrorData>> + Send + 'static>> { let this = self.clone(); let tool_name = tool_name.to_string(); @@ -145,7 +147,11 @@ impl Router for CounterRouter { let value = this.get_value().await?; Ok(vec![Content::text(value.to_string())]) } - _ => Err(ToolError::NotFound(format!("Tool {} not found", tool_name))), + _ => Err(ErrorData { + code: ErrorCode::INVALID_REQUEST, + message: Cow::from(format!("Tool {} not found", tool_name)), + data: None, + }), } }) } diff --git a/crates/mcp-server/src/router.rs b/crates/mcp-server/src/router.rs index 0f5fb882454d..2cb754963d23 100644 --- a/crates/mcp-server/src/router.rs +++ b/crates/mcp-server/src/router.rs @@ -6,7 +6,7 @@ use std::{ type PromptFuture = Pin> + Send + 'static>>; use mcp_core::{ - handler::{PromptError, ResourceError, ToolError}, + handler::{PromptError, ResourceError}, protocol::{ CallToolResult, Implementation, InitializeResult, ListPromptsResult, ListResourcesResult, ListToolsResult, PromptsCapability, ReadResourceResult, ResourcesCapability, @@ -14,8 +14,9 @@ use mcp_core::{ }, }; use rmcp::model::{ - Content, GetPromptResult, JsonRpcMessage, JsonRpcRequest, JsonRpcResponse, JsonRpcVersion2_0, - Prompt, PromptMessage, PromptMessageRole, RequestId, Resource, ResourceContents, + Content, ErrorData, GetPromptResult, JsonRpcMessage, JsonRpcRequest, JsonRpcResponse, + JsonRpcVersion2_0, Prompt, PromptMessage, PromptMessageRole, RequestId, Resource, + ResourceContents, }; use serde_json::Value; use tokio::sync::mpsc; @@ -92,7 +93,7 @@ pub trait Router: Send + Sync + 'static { tool_name: &str, arguments: Value, notifier: mpsc::Sender, - ) -> Pin, ToolError>> + Send + 'static>>; + ) -> Pin, ErrorData>> + Send + 'static>>; fn list_resources(&self) -> Vec; fn read_resource( &self, diff --git a/documentation/docs/goose-architecture/extensions-design.md b/documentation/docs/goose-architecture/extensions-design.md index 76ef60d6aaa7..068e1989e096 100644 --- a/documentation/docs/goose-architecture/extensions-design.md +++ b/documentation/docs/goose-architecture/extensions-design.md @@ -48,7 +48,7 @@ async fn echo(&self, params: Value) -> AgentResult ### Error Handling The system uses two main error types: -- `ToolError`: Specific errors related to tool execution +- `ErrorData`: Specific errors related to tool execution - `anyhow::Error`: General purpose errors for extension status and other operations This split allows precise error handling for tool execution while maintaining flexibility for general extension operations. @@ -65,7 +65,7 @@ This split allows precise error handling for tool execution while maintaining fl ### Extension Implementation 1. **State Encapsulation**: Keep extension state private and controlled -2. **Error Propagation**: Use `?` operator with `ToolError` for tool execution +2. **Error Propagation**: Use `?` operator with `ErrorData` for tool execution 3. **Status Clarity**: Provide clear, structured status information 4. **Documentation**: Document all tools and their effects @@ -90,7 +90,11 @@ impl FileSystem { let full_path = self.root_path.join(path); let content = tokio::fs::read_to_string(full_path) .await - .map_err(|e| ToolError::ExecutionError(e.to_string()))?; + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(e.to_string(), + data: None, + }))?; Ok(json!({ "content": content })) }