IntoCallToolResult implementations over constrain error conversion

[Paul-E]

Is your feature request related to a problem? Please describe.

The trait implementations on IntoCallToolResult over constrain error results, making use of the tool macro more difficult. The implementations on IntoCallToolResult only permit the error to be converted to either unstructured contents via IntoContents, or protocol level errors via manual conversion into ErrorData.

When using the tool macro, it is not possible to have a return signature of Result<T, E> where:

• E could become either a protocol error or an application error, depending on its variant
• E becomes an application error that bears both structured content and unstructured content. Eg display a user friendly message via an unstructured message with user audience, while giving the LLM a detailed error in the structured content.

Describe the solution you'd like

The trait implementation below would work

impl<T: IntoCallToolResult, E: IntoCallToolResult> IntoCallToolResult for Result<T, E> { 
 fn into_call_tool_result(self) -> Result<CallToolResult, crate::ErrorData> { 
 match self { 
 Ok(value) => value.into_call_tool_result(), 
 Err(error) => error.into_call_tool_result(), 
 } 
 } 
}

This would require removing some of the other implementations. It would allow errors to produce arbitrary outputs.

One drawback is that this would be over-permissive. Error variants would be able to produce CallToolResult objects that are not errors. Avoiding that would probably require new types and traits, along the lines of:

struct SuccessResult {
 /// All fields in `CallToolResult`, except `is_error`
}

impl From<SuccessResult> for CallToolResult {
 fn from(value: SuccessResult) -> Self {
 todo!()
 }
}

struct ErrorResult {
 /// All fields in `CallToolResult`, except `is_error`
}

enum McpError {
 ProtocolError(ErrorData),
 ApplicationError(ErrorResult)
}

impl IntoCallToolResult for McpError {
 fn into_call_tool_result(self) -> Result<CallToolResult, crate::ErrorData> {
 match self {
 ProtocolError(protocol_error) => Err(protocol_error),
 ApplicationError(application_error) => todo!(),
 }
 }
}

impl IntoCallToolResult<T: Into<SuccessResult>, E: Into<McpError>> for Result<T, E> {
 fn into_call_tool_result(self) -> Result<CallToolResult, crate::ErrorData> {
 match self {
 Ok(value) => {
 // Convert into SuccessResult
 let res: SuccessResult = value.into();
 // Convert into CallToolResult
 Ok(res.into())
 },
 Err(error) => {
 // Convert into McpError
 let err: McpError = error.into();
 // this is constrained to produce error results, as rmcp defines the into_call_tool_result
 // implementation on McpError
 error.into_call_tool_result()
 }
 }
 }
}

This would ensure the Ok variant only ever produces happy path results, and the Err variant only ever produces error results.

Describe alternatives you've considered

Right now I'm using an approach that defines a Result newtype

pub struct UnstructuredContent<T>(T);
pub struct ToolResult<T, E>(pub Result<T, E>);

impl<T, E> From<Result<T, E>> for ToolResult<T, E> {
 fn from(result: Result<T, E>) -> Self {
 Self(result)
 }
}

impl<T, E> IntoCallToolResult for ToolResult<UnstructuredContent<T>, E>
where
 T: IntoContents,
 E: IntoCallToolResult,
{
 fn into_call_tool_result(self) -> Result<CallToolResult, McpError> {
 match self.0 {
 Ok(content) => Ok(CallToolResult::success(content.0.into_contents())),
 Err(err) => err.into_call_tool_result(),
 }
 }
}

impl<T, E> IntoCallToolResult for ToolResult<Json<T>, E>
where
 T: Serialize + 'static,
 E: IntoCallToolResult,
{
 fn into_call_tool_result(self) -> Result<CallToolResult, McpError> {
 match self.0 {
 Ok(structured_content) => serde_json::to_value(structured_content.0)
 .map_err(|serialize_err| {
 McpError::internal_error(
 format!("failed to serialize response: {serialize_err}"),
 None,
 )
 })
 .map(CallToolResult::structured),
 Err(err) => err.into_call_tool_result(),
 }
 }
}

[Paul-E]

Either of these approaches would complicate schema generation though, as it would require merging the schemas from the variants

[alexhancock]

@jamadeo do you have thoughts on this one?