Skip to content

Commit a355d98

Browse files
committed
Propagate tool call exceptions through filters
1 parent f286391 commit a355d98

File tree

5 files changed

+197
-43
lines changed

5 files changed

+197
-43
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
namespace ModelContextProtocol;
2+
3+
/// <summary>
4+
/// Represents an exception that is thrown by an MCP tool to communicate detailed error information.
5+
/// </summary>
6+
/// <remarks>
7+
/// This exception is used by MCP tools to provide detailed error messages when a tool execution fails.
8+
/// Unlike <see cref="McpException"/>, this exception is intended for application-level errors within tool calls.
9+
/// and does not include JSON-RPC error codes. The <see cref="Exception.Message"/> from this exception
10+
/// will be propagated to the remote endpoint to inform the caller about the tool execution failure.
11+
/// </remarks>
12+
public class McpServerToolException : Exception
13+
{
14+
/// <summary>
15+
/// Initializes a new instance of the <see cref="McpServerToolException"/> class.
16+
/// </summary>
17+
public McpServerToolException()
18+
{
19+
}
20+
21+
/// <summary>
22+
/// Initializes a new instance of the <see cref="McpServerToolException"/> class with a specified error message.
23+
/// </summary>
24+
/// <param name="message">The message that describes the error.</param>
25+
public McpServerToolException(string message) : base(message)
26+
{
27+
}
28+
29+
/// <summary>
30+
/// Initializes a new instance of the <see cref="McpServerToolException"/> class with a specified error message and a reference to the inner exception that is the cause of this exception.
31+
/// </summary>
32+
/// <param name="message">The message that describes the error.</param>
33+
/// <param name="innerException">The exception that is the cause of the current exception, or a null reference if no inner exception is specified.</param>
34+
public McpServerToolException(string message, Exception? innerException) : base(message, innerException)
35+
{
36+
}
37+
}

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
580580

581581
listToolsHandler = BuildFilterPipeline(listToolsHandler, options.Filters.ListToolsFilters);
582582
callToolHandler = BuildFilterPipeline(callToolHandler, options.Filters.CallToolFilters, handler =>
583-
(request, cancellationToken) =>
583+
async (request, cancellationToken) =>
584584
{
585585
// Initial handler that sets MatchedPrimitive
586586
if (request.Params?.Name is { } toolName && tools is not null &&
@@ -589,37 +589,23 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
589589
request.MatchedPrimitive = tool;
590590
}
591591

592-
return handler(request, cancellationToken);
593-
}, handler =>
594-
async (request, cancellationToken) =>
595-
{
596-
// Final handler that provides exception handling only for tool execution
597-
// Only wrap tool execution in try-catch, not tool resolution
598-
if (request.MatchedPrimitive is McpServerTool)
592+
try
599593
{
600-
try
601-
{
602-
return await handler(request, cancellationToken).ConfigureAwait(false);
603-
}
604-
catch (Exception e) when (e is not OperationCanceledException)
605-
{
606-
ToolCallError(request.Params?.Name ?? string.Empty, e);
607-
608-
string errorMessage = e is McpException ?
609-
$"An error occurred invoking '{request.Params?.Name}': {e.Message}" :
610-
$"An error occurred invoking '{request.Params?.Name}'.";
611-
612-
return new()
613-
{
614-
IsError = true,
615-
Content = [new TextContentBlock { Text = errorMessage }],
616-
};
617-
}
594+
return await handler(request, cancellationToken);
618595
}
619-
else
596+
catch (Exception e) when (e is not OperationCanceledException and not McpException)
620597
{
621-
// For unmatched tools, let exceptions bubble up as protocol errors
622-
return await handler(request, cancellationToken).ConfigureAwait(false);
598+
ToolCallError(request.Params?.Name ?? string.Empty, e);
599+
600+
string errorMessage = e is McpServerToolException ?
601+
$"An error occurred invoking '{request.Params?.Name}': {e.Message}" :
602+
$"An error occurred invoking '{request.Params?.Name}'.";
603+
604+
return new()
605+
{
606+
IsError = true,
607+
Content = [new TextContentBlock { Text = errorMessage }],
608+
};
623609
}
624610
});
625611

@@ -735,16 +721,10 @@ private void SetHandler<TParams, TResult>(
735721
private static McpRequestHandler<TParams, TResult> BuildFilterPipeline<TParams, TResult>(
736722
McpRequestHandler<TParams, TResult> baseHandler,
737723
List<McpRequestFilter<TParams, TResult>> filters,
738-
McpRequestFilter<TParams, TResult>? initialHandler = null,
739-
McpRequestFilter<TParams, TResult>? finalHandler = null)
724+
McpRequestFilter<TParams, TResult>? initialHandler = null)
740725
{
741726
var current = baseHandler;
742727

743-
if (finalHandler is not null)
744-
{
745-
current = finalHandler(current);
746-
}
747-
748728
for (int i = filters.Count - 1; i >= 0; i--)
749729
{
750730
current = filters[i](current);

tests/ModelContextProtocol.AspNetCore.Tests/AuthorizeAttributeTests.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -289,21 +289,23 @@ log.Exception is InvalidOperationException &&
289289
}
290290

291291
[Fact]
292-
public async Task CallTool_WithoutAuthFilters_ThrowsInvalidOperationException()
292+
public async Task CallTool_WithoutAuthFilters_ReturnsError()
293293
{
294294
_mockLoggerProvider.LogMessages.Clear();
295295
await using var app = await StartServerWithoutAuthFilters(builder => builder.WithTools<AuthorizationTestTools>());
296296
var client = await ConnectAsync();
297297

298-
var exception = await Assert.ThrowsAsync<McpException>(async () =>
299-
await client.CallToolAsync(
298+
var toolResult = await client.CallToolAsync(
300299
"authorized_tool",
301300
new Dictionary<string, object?> { ["message"] = "test" },
302-
cancellationToken: TestContext.Current.CancellationToken));
301+
cancellationToken: TestContext.Current.CancellationToken);
303302

304-
Assert.Equal("Request failed (remote): An error occurred.", exception.Message);
303+
Assert.True(toolResult.IsError);
304+
305+
var errorContent = Assert.IsType<TextContentBlock>(Assert.Single(toolResult.Content));
306+
Assert.Equal("An error occurred invoking 'authorized_tool'.", errorContent.Text);
305307
Assert.Contains(_mockLoggerProvider.LogMessages, log =>
306-
log.LogLevel == LogLevel.Warning &&
308+
log.LogLevel == LogLevel.Error &&
307309
log.Exception is InvalidOperationException &&
308310
log.Exception.Message.Contains("Authorization filter was not invoked for tools/call operation") &&
309311
log.Exception.Message.Contains("Ensure that AddAuthorizationFilters() is called"));

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsFilterTests.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,18 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
5858
var logger = GetLogger(request.Services, "CallToolFilter");
5959
var primitiveId = request.MatchedPrimitive?.Id ?? "unknown";
6060
logger.LogInformation($"CallToolFilter executed for tool: {primitiveId}");
61-
return await next(request, cancellationToken);
61+
try
62+
{
63+
return await next(request, cancellationToken);
64+
}
65+
catch (Exception ex)
66+
{
67+
return new CallToolResult
68+
{
69+
Content = [new TextContentBlock { Type = "text", Text = $"Error from filter: {ex.Message}" }],
70+
IsError = true
71+
};
72+
}
6273
})
6374
.AddListPromptsFilter((next) => async (request, cancellationToken) =>
6475
{
@@ -162,6 +173,20 @@ public async Task AddCallToolFilter_Logs_When_CallTool_Called()
162173
Assert.Equal("CallToolFilter", logMessage.Category);
163174
}
164175

176+
[Fact]
177+
public async Task AddCallToolFilter_Catches_Exception_From_Tool()
178+
{
179+
await using McpClient client = await CreateMcpClientForServer();
180+
181+
var result = await client.CallToolAsync("throwing_tool_method", cancellationToken: TestContext.Current.CancellationToken);
182+
183+
Assert.True(result.IsError);
184+
Assert.NotNull(result.Content);
185+
var textContent = Assert.Single(result.Content);
186+
var textBlock = Assert.IsType<TextContentBlock>(textContent);
187+
Assert.Equal("Error from filter: This tool always throws an exception", textBlock.Text);
188+
}
189+
165190
[Fact]
166191
public async Task AddListPromptsFilter_Logs_When_ListPrompts_Called()
167192
{
@@ -286,6 +311,12 @@ public static string TestToolMethod()
286311
{
287312
return "test result";
288313
}
314+
315+
[McpServerTool]
316+
public static string ThrowingToolMethod()
317+
{
318+
throw new InvalidOperationException("This tool always throws an exception");
319+
}
289320
}
290321

291322
[McpServerPromptType]

tests/ModelContextProtocol.Tests/Server/McpServerTests.cs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,110 @@ public async Task Can_Handle_Call_Tool_Requests_Throws_Exception_If_No_Handler_A
532532
await Succeeds_Even_If_No_Handler_Assigned(new ServerCapabilities { Tools = new() }, RequestMethods.ToolsCall, "CallTool handler not configured");
533533
}
534534

535+
[Fact]
536+
public async Task Can_Handle_Call_Tool_Requests_With_McpServerToolException()
537+
{
538+
const string errorMessage = "Tool execution failed with detailed error";
539+
await Can_Handle_Requests(
540+
new ServerCapabilities
541+
{
542+
Tools = new()
543+
},
544+
method: RequestMethods.ToolsCall,
545+
configureOptions: options =>
546+
{
547+
options.Handlers.CallToolHandler = async (request, ct) =>
548+
{
549+
throw new McpServerToolException(errorMessage);
550+
};
551+
options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException();
552+
},
553+
assertResult: (_, response) =>
554+
{
555+
var result = JsonSerializer.Deserialize<CallToolResult>(response, McpJsonUtilities.DefaultOptions);
556+
Assert.NotNull(result);
557+
Assert.True(result.IsError);
558+
Assert.NotEmpty(result.Content);
559+
var textContent = Assert.IsType<TextContentBlock>(result.Content[0]);
560+
Assert.Contains(errorMessage, textContent.Text);
561+
});
562+
}
563+
564+
[Fact]
565+
public async Task Can_Handle_Call_Tool_Requests_With_Plain_Exception()
566+
{
567+
await Can_Handle_Requests(
568+
new ServerCapabilities
569+
{
570+
Tools = new()
571+
},
572+
method: RequestMethods.ToolsCall,
573+
configureOptions: options =>
574+
{
575+
options.Handlers.CallToolHandler = async (request, ct) =>
576+
{
577+
throw new InvalidOperationException("This sensitive message should not be exposed");
578+
};
579+
options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException();
580+
},
581+
assertResult: (_, response) =>
582+
{
583+
var result = JsonSerializer.Deserialize<CallToolResult>(response, McpJsonUtilities.DefaultOptions);
584+
Assert.NotNull(result);
585+
Assert.True(result.IsError);
586+
Assert.NotEmpty(result.Content);
587+
var textContent = Assert.IsType<TextContentBlock>(result.Content[0]);
588+
// Should be a generic error message, not the actual exception message
589+
Assert.DoesNotContain("sensitive", textContent.Text, StringComparison.OrdinalIgnoreCase);
590+
Assert.Contains("An error occurred", textContent.Text);
591+
});
592+
}
593+
594+
[Fact]
595+
public async Task Can_Handle_Call_Tool_Requests_With_McpException()
596+
{
597+
const string errorMessage = "Invalid tool parameters";
598+
const McpErrorCode errorCode = McpErrorCode.InvalidParams;
599+
600+
await using var transport = new TestServerTransport();
601+
var options = CreateOptions(new ServerCapabilities { Tools = new() });
602+
options.Handlers.CallToolHandler = async (request, ct) =>
603+
{
604+
throw new McpException(errorMessage, errorCode);
605+
};
606+
options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException();
607+
608+
await using var server = McpServer.Create(transport, options, LoggerFactory);
609+
610+
var runTask = server.RunAsync(TestContext.Current.CancellationToken);
611+
612+
var receivedMessage = new TaskCompletionSource<JsonRpcError>();
613+
614+
transport.OnMessageSent = (message) =>
615+
{
616+
if (message is JsonRpcError error && error.Id.ToString() == "55")
617+
receivedMessage.SetResult(error);
618+
};
619+
620+
await transport.SendMessageAsync(
621+
new JsonRpcRequest
622+
{
623+
Method = RequestMethods.ToolsCall,
624+
Id = new RequestId(55)
625+
},
626+
TestContext.Current.CancellationToken
627+
);
628+
629+
var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
630+
Assert.NotNull(error);
631+
Assert.NotNull(error.Error);
632+
Assert.Equal((int)errorCode, error.Error.Code);
633+
Assert.Equal(errorMessage, error.Error.Message);
634+
635+
await transport.DisposeAsync();
636+
await runTask;
637+
}
638+
535639
private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action<McpServerOptions>? configureOptions, Action<McpServer, JsonNode?> assertResult)
536640
{
537641
await using var transport = new TestServerTransport();

0 commit comments

Comments
 (0)