From 63ec9f43ec62875d91cb6701ba789d599b201a3e Mon Sep 17 00:00:00 2001 From: github-ops Date: Sat, 20 Jun 2026 20:44:13 +0000 Subject: [PATCH] Add structured MCP tool errors --- Cargo.lock | 1 + crates/crank-community-mcp/Cargo.toml | 1 + crates/crank-community-mcp/src/app.rs | 161 +++------------ crates/crank-community-mcp/src/lib.rs | 1 + crates/crank-community-mcp/src/tool_error.rs | 193 ++++++++++++++++++ crates/crank-community-mcp/tests/unit.rs | 2 + .../tests/unit/tool_error.rs | 45 ++++ 7 files changed, 275 insertions(+), 129 deletions(-) create mode 100644 crates/crank-community-mcp/src/tool_error.rs create mode 100644 crates/crank-community-mcp/tests/unit/tool_error.rs diff --git a/Cargo.lock b/Cargo.lock index 41fb8c0..6b28c36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -422,6 +422,7 @@ dependencies = [ "async-trait", "axum", "base64", + "crank-adapter-rest", "crank-core", "crank-mapping", "crank-registry", diff --git a/crates/crank-community-mcp/Cargo.toml b/crates/crank-community-mcp/Cargo.toml index e38a2b8..6205838 100644 --- a/crates/crank-community-mcp/Cargo.toml +++ b/crates/crank-community-mcp/Cargo.toml @@ -9,6 +9,7 @@ version.workspace = true async-trait = "0.1" axum.workspace = true base64.workspace = true +crank-adapter-rest = { path = "../crank-adapter-rest" } crank-core = { path = "../crank-core" } crank-registry = { path = "../crank-registry" } crank-runtime = { path = "../crank-runtime" } diff --git a/crates/crank-community-mcp/src/app.rs b/crates/crank-community-mcp/src/app.rs index cfb3ab7..9e7e413 100644 --- a/crates/crank-community-mcp/src/app.rs +++ b/crates/crank-community-mcp/src/app.rs @@ -45,6 +45,10 @@ use crate::{ }, manifest::tool_definitions, session::{SessionState, SharedSessionStore}, + tool_error::{ + ToolErrorContract, generic_tool_error_contract, runtime_error_code, + tool_error_contract_from_runtime, tool_error_text, tool_error_value, + }, }; const HEADER_MCP_SESSION_ID: &str = "MCP-Session-Id"; @@ -460,18 +464,17 @@ async fn handle_tool_call( message, response_mode, &session.protocol_version, - "machine_access_insufficient", - format!( - "machine access mode {} does not satisfy {} operation security", - serialize_machine_access_mode(credential.machine_access_mode), - serialize_security_level(resolved.tool.operation.security_level), + generic_tool_error_contract( + "machine_access_insufficient", + format!( + "machine access mode {} does not satisfy {} operation security", + serialize_machine_access_mode(credential.machine_access_mode), + serialize_security_level(resolved.tool.operation.security_level), + ), + transport_request_id, + false, + Some("Используйте ключ агента с достаточным уровнем доступа."), ), - Some(json!({ - "machine_access_mode": credential.machine_access_mode, - "credential_security_level": credential.max_security_level, - "required_security_level": resolved.tool.operation.security_level, - "upgrade_required": true, - })), ); } @@ -660,9 +663,7 @@ async fn handle_base_tool_call( message, response_mode, &session.protocol_version, - runtime_error_code(&error), - error.to_string(), - runtime_error_context(&error), + tool_error_contract_from_runtime(&error, transport_request_id), ) } } @@ -1242,17 +1243,10 @@ fn tool_error_response( message: &Value, response_mode: ResponseMode, protocol_version: &str, - code: &str, - error_message: String, - error_context: Option, + error: ToolErrorContract, ) -> Response { - let mut error = json!({ - "code": code, - "message": error_message - }); - if let Some(context) = error_context { - error["context"] = context; - } + let error_message = tool_error_text(&error); + let error_value = tool_error_value(&error); transport_response( StatusCode::OK, @@ -1266,7 +1260,7 @@ fn tool_error_response( } ], "structuredContent": { - "error": error + "error": error_value }, "isError": true }), @@ -1283,64 +1277,6 @@ fn add_millis(timestamp: OffsetDateTime, millis: u64) -> OffsetDateTime { timestamp + delta } -fn runtime_error_code(error: &RuntimeError) -> &'static str { - match error { - RuntimeError::Schema(_) => "schema_validation_error", - RuntimeError::Mapping(_) => "mapping_error", - RuntimeError::RestAdapter(_) => "adapter_execution_error", - RuntimeError::ProtocolAdapter(_) => "adapter_execution_error", - RuntimeError::UnsupportedProtocol { .. } => "unsupported_protocol", - RuntimeError::ConcurrencyLimitExceeded { .. } => "runtime_overloaded", - RuntimeError::UnsupportedExecutionMode { .. } => "streaming_mode_error", - RuntimeError::InvalidPreparedRequest { .. } => "runtime_error", - RuntimeError::MissingAuthProfile { .. } => "auth_profile_not_found", - RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { - "secret_not_found" - } - RuntimeError::InvalidAuthSecretValue { .. } => "secret_value_error", - RuntimeError::SecretCrypto { .. } => "secret_crypto_error", - } -} - -fn runtime_error_context(error: &RuntimeError) -> Option { - match error { - RuntimeError::InvalidPreparedRequest { field, reason } => Some(json!({ - "field": field, - "reason": reason, - })), - RuntimeError::InvalidAuthSecretValue { secret_id, reason } => Some(json!({ - "secret_id": secret_id, - "reason": reason, - })), - RuntimeError::SecretCrypto { operation, details } => Some(json!({ - "operation": operation, - "details": details, - })), - RuntimeError::MissingAuthProfile { auth_profile_id } => Some(json!({ - "auth_profile_id": auth_profile_id, - })), - RuntimeError::MissingSecret { secret_id } => Some(json!({ - "secret_id": secret_id, - })), - RuntimeError::MissingSecretVersion { secret_id, version } => Some(json!({ - "secret_id": secret_id, - "version": version, - })), - RuntimeError::UnsupportedExecutionMode { operation_id, mode } => Some(json!({ - "operation_id": operation_id, - "mode": mode, - })), - RuntimeError::UnsupportedProtocol { protocol } => Some(json!({ - "protocol": protocol, - })), - RuntimeError::ConcurrencyLimitExceeded { kind, limit } => Some(json!({ - "kind": kind, - "limit": limit, - })), - _ => None, - } -} - fn hash_access_secret(secret: &str) -> String { let digest = Sha256::digest(secret.as_bytes()); URL_SAFE_NO_PAD.encode(digest) @@ -1511,43 +1447,9 @@ mod tests { use axum::body::to_bytes; use serde_json::{Value, json}; - use super::{ResponseMode, runtime_error_context, tool_error_response}; + use super::{ResponseMode, tool_error_response}; use crate::jsonrpc::CURRENT_PROTOCOL_VERSION; - use crank_runtime::RuntimeError; - - #[test] - fn runtime_error_context_includes_secret_crypto_operation() { - let context = runtime_error_context(&RuntimeError::SecretCrypto { - operation: "decode secret envelope", - details: "bad base64".to_owned(), - }) - .unwrap(); - - assert_eq!( - context, - json!({ - "operation": "decode secret envelope", - "details": "bad base64" - }) - ); - } - - #[test] - fn runtime_error_context_includes_runtime_overload_details() { - let context = runtime_error_context(&RuntimeError::ConcurrencyLimitExceeded { - kind: "async_job", - limit: 8, - }) - .unwrap(); - - assert_eq!( - context, - json!({ - "kind": "async_job", - "limit": 8 - }) - ); - } + use crate::tool_error::generic_tool_error_contract; #[tokio::test] async fn tool_error_response_includes_structured_context() { @@ -1555,12 +1457,13 @@ mod tests { &json!({"jsonrpc": "2.0", "id": "req-1"}), ResponseMode::Json, CURRENT_PROTOCOL_VERSION, - "streaming_payload_error", - "request root must be an object".to_owned(), - Some(json!({ - "field": "request", - "reason": "must be an object" - })), + generic_tool_error_contract( + "streaming_payload_error", + "request root must be an object", + "req-1", + false, + Some("Проверьте параметры вызова инструмента."), + ), ); let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); @@ -1570,11 +1473,11 @@ mod tests { payload["result"]["structuredContent"]["error"], json!({ "code": "streaming_payload_error", + "error_code": "streaming_payload_error", "message": "request root must be an object", - "context": { - "field": "request", - "reason": "must be an object" - } + "recoverable": false, + "request_id": "req-1", + "suggested_action": "Проверьте параметры вызова инструмента." }) ); } diff --git a/crates/crank-community-mcp/src/lib.rs b/crates/crank-community-mcp/src/lib.rs index faedbef..4104352 100644 --- a/crates/crank-community-mcp/src/lib.rs +++ b/crates/crank-community-mcp/src/lib.rs @@ -4,5 +4,6 @@ pub mod catalog; pub mod jsonrpc; pub mod manifest; pub mod session; +pub mod tool_error; pub use app::build_app; diff --git a/crates/crank-community-mcp/src/tool_error.rs b/crates/crank-community-mcp/src/tool_error.rs new file mode 100644 index 0000000..099f482 --- /dev/null +++ b/crates/crank-community-mcp/src/tool_error.rs @@ -0,0 +1,193 @@ +use crank_adapter_rest::RestAdapterError; +use crank_runtime::RuntimeError; +use serde::Serialize; +use serde_json::{Value, json}; + +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct ToolErrorContract { + pub code: &'static str, + pub error_code: &'static str, + pub message: String, + pub recoverable: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub suggested_action: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + pub upstream_status: Option, + pub request_id: String, +} + +pub fn tool_error_contract_from_runtime( + error: &RuntimeError, + request_id: &str, +) -> ToolErrorContract { + let error_code = runtime_error_code(error); + ToolErrorContract { + code: error_code, + error_code, + message: safe_runtime_error_message(error), + recoverable: is_recoverable_runtime_error(error), + suggested_action: suggested_action(error), + upstream_status: upstream_status(error), + request_id: request_id.to_owned(), + } +} + +pub fn generic_tool_error_contract( + error_code: &'static str, + message: impl Into, + request_id: &str, + recoverable: bool, + suggested_action: Option<&'static str>, +) -> ToolErrorContract { + ToolErrorContract { + code: error_code, + error_code, + message: message.into(), + recoverable, + suggested_action, + upstream_status: None, + request_id: request_id.to_owned(), + } +} + +pub fn tool_error_text(error: &ToolErrorContract) -> String { + match error.suggested_action { + Some(action) => format!("{} {}", error.message, action), + None => error.message.clone(), + } +} + +pub fn tool_error_value(error: &ToolErrorContract) -> Value { + serde_json::to_value(error).unwrap_or_else(|_| { + json!({ + "code": "runtime_error", + "error_code": "runtime_error", + "message": "Не удалось выполнить инструмент.", + "recoverable": false, + "request_id": error.request_id + }) + }) +} + +pub fn runtime_error_code(error: &RuntimeError) -> &'static str { + match error { + RuntimeError::Schema(_) => "schema_validation_error", + RuntimeError::Mapping(_) => "mapping_error", + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { status, .. }) => { + upstream_status_code(*status) + } + RuntimeError::RestAdapter(RestAdapterError::Transport(_)) => "upstream_transport_error", + RuntimeError::RestAdapter(_) => "adapter_execution_error", + RuntimeError::ProtocolAdapter(_) => "adapter_execution_error", + RuntimeError::UnsupportedProtocol { .. } => "unsupported_protocol", + RuntimeError::ConcurrencyLimitExceeded { .. } => "runtime_overloaded", + RuntimeError::UnsupportedExecutionMode { .. } => "streaming_mode_error", + RuntimeError::InvalidPreparedRequest { .. } => "runtime_error", + RuntimeError::MissingAuthProfile { .. } => "auth_profile_not_found", + RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { + "secret_not_found" + } + RuntimeError::InvalidAuthSecretValue { .. } => "secret_value_error", + RuntimeError::SecretCrypto { .. } => "secret_crypto_error", + } +} + +fn upstream_status_code(status: u16) -> &'static str { + match status { + 401 | 403 => "upstream_auth_error", + 404 => "upstream_not_found", + 408 | 429 => "upstream_rate_limited", + 500..=599 => "upstream_server_error", + _ => "upstream_status_error", + } +} + +fn safe_runtime_error_message(error: &RuntimeError) -> String { + match error { + RuntimeError::Schema(_) => "Входные параметры не прошли проверку схемы.".to_owned(), + RuntimeError::Mapping(_) => { + "Не удалось сопоставить параметры инструмента с API-запросом.".to_owned() + } + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { status, .. }) => { + format!("Внешний API вернул HTTP {}.", status) + } + RuntimeError::RestAdapter(RestAdapterError::Transport(_)) => { + "Не удалось подключиться к внешнему API.".to_owned() + } + RuntimeError::RestAdapter(_) => "Не удалось выполнить запрос к внешнему API.".to_owned(), + RuntimeError::ProtocolAdapter(_) => "Не удалось выполнить протокольный адаптер.".to_owned(), + RuntimeError::UnsupportedProtocol { .. } => { + "Протокол операции не поддерживается.".to_owned() + } + RuntimeError::ConcurrencyLimitExceeded { .. } => { + "Сервис временно перегружен и не может выполнить инструмент.".to_owned() + } + RuntimeError::UnsupportedExecutionMode { .. } => { + "Режим выполнения операции не поддерживается.".to_owned() + } + RuntimeError::InvalidPreparedRequest { .. } => { + "Не удалось подготовить корректный API-запрос.".to_owned() + } + RuntimeError::MissingAuthProfile { .. } => "Профиль авторизации не найден.".to_owned(), + RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { + "Секрет авторизации не найден.".to_owned() + } + RuntimeError::InvalidAuthSecretValue { .. } => { + "Секрет авторизации имеет неподходящий формат.".to_owned() + } + RuntimeError::SecretCrypto { .. } => { + "Не удалось расшифровать секрет авторизации.".to_owned() + } + } +} + +fn is_recoverable_runtime_error(error: &RuntimeError) -> bool { + matches!( + error, + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { + status: 408 | 429 | 500..=599, + .. + }) | RuntimeError::RestAdapter(RestAdapterError::Transport(_)) + | RuntimeError::ConcurrencyLimitExceeded { .. } + | RuntimeError::SecretCrypto { .. } + ) +} + +fn suggested_action(error: &RuntimeError) -> Option<&'static str> { + match error { + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { + status: 401 | 403, .. + }) => Some("Проверьте настройки авторизации внешнего API."), + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { status: 404, .. }) => { + Some("Проверьте путь endpoint-а и параметры запроса.") + } + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { + status: 408 | 429 | 500..=599, + .. + }) + | RuntimeError::RestAdapter(RestAdapterError::Transport(_)) + | RuntimeError::ConcurrencyLimitExceeded { .. } => Some("Повторите запрос позже."), + RuntimeError::Schema(_) + | RuntimeError::Mapping(_) + | RuntimeError::InvalidPreparedRequest { .. } => { + Some("Проверьте параметры вызова инструмента.") + } + RuntimeError::MissingAuthProfile { .. } + | RuntimeError::MissingSecret { .. } + | RuntimeError::MissingSecretVersion { .. } + | RuntimeError::InvalidAuthSecretValue { .. } + | RuntimeError::SecretCrypto { .. } => { + Some("Проверьте настройки авторизации и секретов в Crank.") + } + _ => None, + } +} + +fn upstream_status(error: &RuntimeError) -> Option { + match error { + RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { status, .. }) => { + Some(*status) + } + _ => None, + } +} diff --git a/crates/crank-community-mcp/tests/unit.rs b/crates/crank-community-mcp/tests/unit.rs index cb9ba95..8a0c99f 100644 --- a/crates/crank-community-mcp/tests/unit.rs +++ b/crates/crank-community-mcp/tests/unit.rs @@ -1,2 +1,4 @@ #[path = "unit/manifest.rs"] mod manifest; +#[path = "unit/tool_error.rs"] +mod tool_error; diff --git a/crates/crank-community-mcp/tests/unit/tool_error.rs b/crates/crank-community-mcp/tests/unit/tool_error.rs new file mode 100644 index 0000000..c692063 --- /dev/null +++ b/crates/crank-community-mcp/tests/unit/tool_error.rs @@ -0,0 +1,45 @@ +use crank_adapter_rest::RestAdapterError; +use crank_community_mcp::tool_error::tool_error_contract_from_runtime; +use crank_runtime::RuntimeError; +use serde_json::json; + +#[test] +fn maps_upstream_429_to_recoverable_structured_tool_error() { + let contract = tool_error_contract_from_runtime( + &RuntimeError::RestAdapter(RestAdapterError::UnexpectedStatus { + status: 429, + body: json!({ + "error": "rate limit exceeded", + "internal_trace": "do not leak" + }), + }), + "req-429", + ); + + assert_eq!(contract.error_code, "upstream_rate_limited"); + assert!(contract.recoverable); + assert_eq!(contract.upstream_status, Some(429)); + assert_eq!(contract.request_id, "req-429"); + assert_eq!(contract.suggested_action, Some("Повторите запрос позже.")); + assert!(!contract.message.contains("internal_trace")); +} + +#[test] +fn maps_mapping_error_to_non_recoverable_structured_tool_error() { + let contract = tool_error_contract_from_runtime( + &RuntimeError::InvalidPreparedRequest { + field: "query.base".to_owned(), + reason: "expected string".to_owned(), + }, + "req-map", + ); + + assert_eq!(contract.error_code, "runtime_error"); + assert!(!contract.recoverable); + assert_eq!(contract.upstream_status, None); + assert_eq!(contract.request_id, "req-map"); + assert_eq!( + contract.suggested_action, + Some("Проверьте параметры вызова инструмента.") + ); +}