diff --git a/Cargo.lock b/Cargo.lock index 5a95598..c6b7dc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,6 +63,7 @@ dependencies = [ "tokio-stream", "tonic", "tonic-tls", + "tonic-types", "tracing", ] @@ -4532,6 +4533,17 @@ dependencies = [ "tracing", ] +[[package]] +name = "tonic-types" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a875a902255423d34c1f20838ab374126db8eb41625b7947a1d54113b0b7399" +dependencies = [ + "prost", + "prost-types", + "tonic", +] + [[package]] name = "tower" version = "0.5.3" diff --git a/Cargo.toml b/Cargo.toml index c9f258f..7cdfa28 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ tonic = "0.14" tonic-build = "0.14" tonic-prost = "0.14" tonic-prost-build = "0.14" +tonic-types = "0.14" tonic-tls = { version = "0.7", default-features = false } prost = "0.14" prost-types = "0.14" diff --git a/a2a-grpc/Cargo.toml b/a2a-grpc/Cargo.toml index 15bc657..4328372 100644 --- a/a2a-grpc/Cargo.toml +++ b/a2a-grpc/Cargo.toml @@ -17,6 +17,7 @@ a2a-client = { workspace = true, default-features = false } a2a-server = { workspace = true, default-features = false } a2a-pb = { workspace = true } tonic = { workspace = true } +tonic-types = { workspace = true } prost = { workspace = true } tokio = { workspace = true } tokio-stream = { workspace = true, features = ["net"] } diff --git a/a2a-grpc/src/errors.rs b/a2a-grpc/src/errors.rs index d50e14f..3b87f9f 100644 --- a/a2a-grpc/src/errors.rs +++ b/a2a-grpc/src/errors.rs @@ -1,6 +1,9 @@ // Copyright AGNTCY Contributors (https://github.com/agntcy) // SPDX-License-Identifier: Apache-2.0 -use a2a::A2AError; +use std::collections::HashMap; + +use a2a::{A2AError, error_reason, errordetails, reason_to_error_code}; +use tonic_types::{ErrorDetails, FieldViolation as GrpcFieldViolation, StatusExt}; /// Convert an A2A error to a tonic gRPC status code. pub fn a2a_error_to_status(err: &A2AError) -> tonic::Status { @@ -22,27 +25,111 @@ pub fn a2a_error_to_status(err: &A2AError) -> tonic::Status { error_code::INTERNAL_ERROR => tonic::Code::Internal, _ => tonic::Code::Unknown, }; - tonic::Status::new(code, &err.message) + let metadata: HashMap = err + .details + .as_deref() + .and_then(|details| { + details + .iter() + .find(|detail| detail.type_url == errordetails::ERROR_INFO_TYPE) + }) + .and_then(|detail| detail.value.get("metadata")) + .and_then(|metadata| metadata.as_object()) + .map(|metadata| { + metadata + .iter() + .filter_map(|(key, value)| { + value.as_str().map(|value| (key.clone(), value.to_owned())) + }) + .collect() + }) + .unwrap_or_default(); + let mut details = ErrorDetails::with_error_info( + error_reason(err.code), + errordetails::PROTOCOL_DOMAIN, + metadata, + ); + + let mut field_violations = Vec::new(); + if let Some(error_details) = &err.details { + for detail in error_details + .iter() + .filter(|detail| detail.type_url == errordetails::BAD_REQUEST_TYPE) + { + let Some(violations) = detail + .value + .get("fieldViolations") + .and_then(|value| value.as_array()) + else { + continue; + }; + field_violations.extend(violations.iter().filter_map(|violation| { + Some(GrpcFieldViolation::new( + violation.get("field")?.as_str()?, + violation.get("description")?.as_str()?, + )) + })); + } + } + if !field_violations.is_empty() { + details.set_bad_request(field_violations); + } + + tonic::Status::with_error_details(code, &err.message, details) } /// Convert a tonic gRPC status to an A2A error. pub fn status_to_a2a_error(status: &tonic::Status) -> A2AError { use a2a::error_code; - let code = match status.code() { - tonic::Code::NotFound => error_code::TASK_NOT_FOUND, - tonic::Code::FailedPrecondition => error_code::TASK_NOT_CANCELABLE, - tonic::Code::Unimplemented => error_code::METHOD_NOT_FOUND, - tonic::Code::InvalidArgument => error_code::INVALID_PARAMS, - tonic::Code::Internal => error_code::INTERNAL_ERROR, - _ => error_code::INTERNAL_ERROR, - }; + let code = status + .get_details_error_info() + .filter(|detail| detail.domain == errordetails::PROTOCOL_DOMAIN) + .and_then(|detail| reason_to_error_code(&detail.reason)) + .unwrap_or_else(|| match status.code() { + tonic::Code::NotFound => error_code::TASK_NOT_FOUND, + tonic::Code::FailedPrecondition => error_code::TASK_NOT_CANCELABLE, + tonic::Code::Unimplemented => error_code::METHOD_NOT_FOUND, + tonic::Code::InvalidArgument => error_code::INVALID_PARAMS, + tonic::Code::Internal => error_code::INTERNAL_ERROR, + _ => error_code::INTERNAL_ERROR, + }); A2AError::new(code, status.message()) } #[cfg(test)] mod tests { use super::*; - use a2a::error_code; + use std::collections::HashMap; + + use a2a::{error_code, errordetails}; + use tonic_types::{ErrorDetails, StatusExt}; + + const FAILED_PRECONDITION_ERRORS: [(i32, &str); 6] = [ + (error_code::TASK_NOT_CANCELABLE, "TASK_NOT_CANCELABLE"), + ( + error_code::PUSH_NOTIFICATION_NOT_SUPPORTED, + "PUSH_NOTIFICATION_NOT_SUPPORTED", + ), + (error_code::UNSUPPORTED_OPERATION, "UNSUPPORTED_OPERATION"), + ( + error_code::EXTENDED_CARD_NOT_CONFIGURED, + "EXTENDED_AGENT_CARD_NOT_CONFIGURED", + ), + ( + error_code::EXTENSION_SUPPORT_REQUIRED, + "EXTENSION_SUPPORT_REQUIRED", + ), + (error_code::VERSION_NOT_SUPPORTED, "VERSION_NOT_SUPPORTED"), + ]; + const INVALID_ARGUMENT_ERRORS: [(i32, &str); 4] = [ + ( + error_code::CONTENT_TYPE_NOT_SUPPORTED, + "CONTENT_TYPE_NOT_SUPPORTED", + ), + (error_code::PARSE_ERROR, "PARSE_ERROR"), + (error_code::INVALID_REQUEST, "INVALID_REQUEST"), + (error_code::INVALID_PARAMS, "INVALID_PARAMS"), + ]; #[test] fn test_a2a_error_to_status_mapping() { @@ -106,6 +193,140 @@ mod tests { } } + #[test] + fn test_failed_precondition_errors_round_trip_with_error_info() { + for (code, expected_reason) in FAILED_PRECONDITION_ERRORS { + let status = a2a_error_to_status(&A2AError::new(code, "test")); + + assert_eq!(status.code(), tonic::Code::FailedPrecondition); + let error_info = status + .get_details_error_info() + .expect("A2A gRPC statuses should include ErrorInfo"); + assert_eq!(error_info.reason, expected_reason); + assert_eq!(error_info.domain, errordetails::PROTOCOL_DOMAIN); + + let decoded = status_to_a2a_error(&status); + assert_eq!(decoded.code, code, "failed to recover {expected_reason}"); + } + } + + #[test] + fn test_invalid_argument_errors_round_trip_with_error_info() { + for (code, expected_reason) in INVALID_ARGUMENT_ERRORS { + let status = a2a_error_to_status(&A2AError::new(code, "test")); + + assert_eq!(status.code(), tonic::Code::InvalidArgument); + let error_info = status + .get_details_error_info() + .expect("A2A gRPC statuses should include ErrorInfo"); + assert_eq!(error_info.reason, expected_reason); + assert_eq!(error_info.domain, errordetails::PROTOCOL_DOMAIN); + + let decoded = status_to_a2a_error(&status); + assert_eq!(decoded.code, code, "failed to recover {expected_reason}"); + } + } + + #[test] + fn test_a2a_error_details_are_preserved_in_grpc_status() { + let metadata = HashMap::from([ + ("requestId".to_string(), "request-123".to_string()), + ("tenant".to_string(), "example".to_string()), + ]); + let err = A2AError::invalid_params("invalid message").with_details(vec![ + errordetails::TypedDetail::error_info( + "INVALID_PARAMS", + errordetails::PROTOCOL_DOMAIN, + Some(metadata.clone()), + ), + errordetails::TypedDetail::bad_request(vec![ + errordetails::FieldViolation { + field: "message.parts".to_string(), + description: "at least one part is required".to_string(), + }, + errordetails::FieldViolation { + field: "message.role".to_string(), + description: "must be user or agent".to_string(), + }, + ]), + ]); + + let status = a2a_error_to_status(&err); + + let error_info = status + .get_details_error_info() + .expect("A2A gRPC statuses should include ErrorInfo"); + assert_eq!(error_info.metadata, metadata); + + let bad_request = status + .get_details_bad_request() + .expect("BadRequest details should be preserved"); + assert_eq!(bad_request.field_violations.len(), 2); + assert_eq!(bad_request.field_violations[0].field, "message.parts"); + assert_eq!( + bad_request.field_violations[0].description, + "at least one part is required" + ); + assert_eq!(bad_request.field_violations[1].field, "message.role"); + assert_eq!( + bad_request.field_violations[1].description, + "must be user or agent" + ); + } + + #[test] + fn test_protocol_error_info_recovers_precise_a2a_code() { + for (expected_code, reason) in FAILED_PRECONDITION_ERRORS { + let status = tonic::Status::with_error_details( + tonic::Code::FailedPrecondition, + "test", + ErrorDetails::with_error_info( + reason, + errordetails::PROTOCOL_DOMAIN, + HashMap::new(), + ), + ); + + let decoded = status_to_a2a_error(&status); + assert_eq!(decoded.code, expected_code, "failed to decode {reason}"); + } + } + + #[test] + fn test_untrusted_error_info_falls_back_to_grpc_code() { + let cases = [ + ErrorDetails::with_error_info( + "PUSH_NOTIFICATION_NOT_SUPPORTED", + "example.com", + HashMap::new(), + ), + ErrorDetails::with_error_info( + "UNKNOWN_REASON", + errordetails::PROTOCOL_DOMAIN, + HashMap::new(), + ), + ]; + + for details in cases { + let status = + tonic::Status::with_error_details(tonic::Code::FailedPrecondition, "test", details); + let decoded = status_to_a2a_error(&status); + assert_eq!(decoded.code, error_code::TASK_NOT_CANCELABLE); + } + } + + #[test] + fn test_malformed_error_details_fall_back_to_grpc_code() { + let status = tonic::Status::with_details( + tonic::Code::FailedPrecondition, + "test", + prost::bytes::Bytes::from_static(b"not a google.rpc.Status"), + ); + + let decoded = status_to_a2a_error(&status); + assert_eq!(decoded.code, error_code::TASK_NOT_CANCELABLE); + } + #[test] fn test_unknown_code_maps_to_unknown() { let err = A2AError::new(99999, "test");