Improved Mechanism for Propagating Tool Failure

[eiriktsarpalis]

Originally posted by @PederHP in PederHP/mcpdotnet#43.

Tool failure (using IsError = true) is currently not possible to return from the Tool mapping layer. This could be done via:

1. Enforcing all Tools return a ToolResult type which has an IsError field. This is not desirable as it removes the ability to expose any method as tool.

2. A new Exception (ie McpToolException) which is caught in the handler and then returns IsError true and an error message provided in the exception. This would allow developers a way to hook into the MCP protocol without enforcing specific return types on their method.

[stephentoub]

This is possible now by either:

1. Returning a CallToolResponse from a tool method:
   

   csharp-sdk/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs

   Line 314 in ac0bc3a

   CallToolResponse callToolResponse => callToolResponse,

2. Throwing an exception from a tool; the exception will be caught and translated into an IsError=true:

   csharp-sdk/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs

   Lines 268 to 275 in ac0bc3a

   	catch (Exception e) when (e is not OperationCanceledException)
   	{
   	return new CallToolResponse()
   	{
   	IsError = true,
   	Content = [new() { Text = e.Message, Type = "text" }],
   	};
   	}

[PederHP]

I worry that putting the exception message by default on any exception thrown from the tool could be a security issue. I would prefer only a specific exception type resulting the exception message being emitted. Something like

 catch (Exception e) when (e is not OperationCanceledException) 
 { 
     return new CallToolResponse() 
     { 
         IsError = true, 
         Content = [new() { Text = (e is McpToolException) ? e.Message : "An error has occurred", Type = "text" }], 
     }; 
 } 

(might need some unwrapping if McpToolException is aggregate based).

Alternatively or additionally having an attribute explicitly allow exception details to propagate.

[McpServerTool, Description("Echoes the input back to the client."), AllowExceptionDetails(true)]

Otherwise, developers will need to consider the security aspects of decorating their existing code to be exposed MCP tools, which means eventually someone will forget that they shouldn't leak internal exception details and forget to add an exception handler in the tool method to filter out the exception message.

Maybe there is a smoother way to do this or maybe I am seeing this as a more serious concern than it is, but I don't feel good about implicitly and by default transmitting exception details to clients.

If you agree or would like to see a proposal, I am happy to make a PR.