Skip to content

Commit 10a2e6d

Browse files
Copilothalter73
Copilot
and
halter73
committed
Log ToolCallError, GetPromptError, ReadResourceError for all exception types
Remove the exception type filter (when clause) from the error logging catch blocks for prompts and resources, and add a second catch block for tools, so that OperationCanceledException and McpProtocolException are also logged with tool/prompt/resource names. Behavior is unchanged: tools still produce IsError results for non-cancel/non-protocol exceptions, and prompts/resources still rethrow all exceptions. Add tests verifying error logging and LogRequestHandlerException for OperationCanceledException and McpProtocolException in all three handler types. Co-authored-by: halter73 <[email protected]>
1 parent d250e0d commit 10a2e6d

File tree

4 files changed

+160
-15
lines changed

4 files changed

+160
-15
lines changed

‎src/ModelContextProtocol.Core/Server/McpServerImpl.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ await originalListResourceTemplatesHandler(request, cancellationToken).Configure
398398
ReadResourceCompleted(request.Params?.Uri ?? string.Empty);
399399
return result;
400400
}
401-
catch (Exception e) when (e is not OperationCanceledException and not McpProtocolException)
401+
catch (Exception e)
402402
{
403403
ReadResourceError(request.Params?.Uri ?? string.Empty, e);
404404
throw;
@@ -512,7 +512,7 @@ await originalListPromptsHandler(request, cancellationToken).ConfigureAwait(fals
512512
GetPromptCompleted(request.Params?.Name ?? string.Empty);
513513
return result;
514514
}
515-
catch (Exception e) when (e is not OperationCanceledException and not McpProtocolException)
515+
catch (Exception e)
516516
{
517517
GetPromptError(request.Params?.Name ?? string.Empty, e);
518518
throw;
@@ -655,6 +655,11 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
655655
Content = [new TextContentBlock { Text = errorMessage }],
656656
};
657657
}
658+
catch (Exception e)
659+
{
660+
ToolCallError(request.Params?.Name ?? string.Empty, e);
661+
throw;
662+
}
658663
});
659664

660665
ServerCapabilities.Tools.ListChanged = listChanged;

‎tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsPromptsTests.cs‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public async Task Can_List_And_Call_Registered_Prompts()
102102
await using McpClient client = await CreateMcpClientForServer();
103103

104104
var prompts = await client.ListPromptsAsync(cancellationToken: TestContext.Current.CancellationToken);
105-
Assert.Equal(6, prompts.Count);
105+
Assert.Equal(8, prompts.Count);
106106

107107
var prompt = prompts.First(t => t.Name == "returns_chat_messages");
108108
Assert.Equal("Returns chat messages", prompt.Description);
@@ -131,7 +131,7 @@ public async Task Can_Be_Notified_Of_Prompt_Changes()
131131
await using McpClient client = await CreateMcpClientForServer();
132132

133133
var prompts = await client.ListPromptsAsync(cancellationToken: TestContext.Current.CancellationToken);
134-
Assert.Equal(6, prompts.Count);
134+
Assert.Equal(8, prompts.Count);
135135

136136
Channel<JsonRpcNotification> listChanged = Channel.CreateUnbounded<JsonRpcNotification>();
137137
var notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -152,7 +152,7 @@ public async Task Can_Be_Notified_Of_Prompt_Changes()
152152
await notificationRead;
153153

154154
prompts = await client.ListPromptsAsync(cancellationToken: TestContext.Current.CancellationToken);
155-
Assert.Equal(7, prompts.Count);
155+
Assert.Equal(9, prompts.Count);
156156
Assert.Contains(prompts, t => t.Name == "NewPrompt");
157157

158158
notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -162,7 +162,7 @@ public async Task Can_Be_Notified_Of_Prompt_Changes()
162162
}
163163

164164
prompts = await client.ListPromptsAsync(cancellationToken: TestContext.Current.CancellationToken);
165-
Assert.Equal(6, prompts.Count);
165+
Assert.Equal(8, prompts.Count);
166166
Assert.DoesNotContain(prompts, t => t.Name == "NewPrompt");
167167
}
168168

@@ -227,6 +227,44 @@ await Assert.ThrowsAsync<McpProtocolException>(async () => await client.GetPromp
227227
Assert.IsType<FormatException>(errorLog.Exception);
228228
}
229229

230+
[Fact]
231+
public async Task Logs_Prompt_Error_When_Prompt_Throws_OperationCanceledException()
232+
{
233+
await using McpClient client = await CreateMcpClientForServer();
234+
235+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.GetPromptAsync(
236+
"throws_operation_canceled_exception",
237+
cancellationToken: TestContext.Current.CancellationToken));
238+
239+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
240+
m.LogLevel == LogLevel.Error &&
241+
m.Message == "GetPrompt \"throws_operation_canceled_exception\" threw an unhandled exception." &&
242+
m.Exception is OperationCanceledException);
243+
244+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
245+
m.LogLevel == LogLevel.Warning &&
246+
m.Message.Contains("request handler failed"));
247+
}
248+
249+
[Fact]
250+
public async Task Logs_Prompt_Error_When_Prompt_Throws_McpProtocolException()
251+
{
252+
await using McpClient client = await CreateMcpClientForServer();
253+
254+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.GetPromptAsync(
255+
"throws_mcp_protocol_exception",
256+
cancellationToken: TestContext.Current.CancellationToken));
257+
258+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
259+
m.LogLevel == LogLevel.Error &&
260+
m.Message == "GetPrompt \"throws_mcp_protocol_exception\" threw an unhandled exception." &&
261+
m.Exception is McpProtocolException);
262+
263+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
264+
m.LogLevel == LogLevel.Warning &&
265+
m.Message.Contains("request handler failed"));
266+
}
267+
230268
[Fact]
231269
public async Task Throws_Exception_On_Unknown_Prompt()
232270
{
@@ -367,6 +405,14 @@ public static ChatMessage[] ReturnsChatMessages([Description("The first paramete
367405
public static ChatMessage[] ThrowsException([Description("The first parameter")] string message) =>
368406
throw new FormatException("uh oh");
369407

408+
[McpServerPrompt, Description("Throws OperationCanceledException")]
409+
public static ChatMessage[] ThrowsOperationCanceledException() =>
410+
throw new OperationCanceledException("Prompt was canceled");
411+
412+
[McpServerPrompt, Description("Throws McpProtocolException")]
413+
public static ChatMessage[] ThrowsMcpProtocolException() =>
414+
throw new McpProtocolException("Prompt protocol error", McpErrorCode.InvalidParams);
415+
370416
[McpServerPrompt(Title = "This is a title", IconSource = "https://example.com/prompt-icon.svg"), Description("Returns chat messages")]
371417
public string ReturnsString([Description("The first parameter")] string message) =>
372418
$"The prompt is: {message}. The id is {id}.";

‎tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsResourcesTests.cs‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public async Task Can_List_And_Call_Registered_Resources()
131131
Assert.NotNull(client.ServerCapabilities.Resources);
132132

133133
var resources = await client.ListResourcesAsync(cancellationToken: TestContext.Current.CancellationToken);
134-
Assert.Equal(5, resources.Count);
134+
Assert.Equal(7, resources.Count);
135135

136136
var resource = resources.First(t => t.Name == "some_neat_direct_resource");
137137
Assert.Equal("Some neat direct resource", resource.Description);
@@ -165,7 +165,7 @@ public async Task Can_Be_Notified_Of_Resource_Changes()
165165
await using McpClient client = await CreateMcpClientForServer();
166166

167167
var resources = await client.ListResourcesAsync(cancellationToken: TestContext.Current.CancellationToken);
168-
Assert.Equal(5, resources.Count);
168+
Assert.Equal(7, resources.Count);
169169

170170
Channel<JsonRpcNotification> listChanged = Channel.CreateUnbounded<JsonRpcNotification>();
171171
var notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -186,7 +186,7 @@ public async Task Can_Be_Notified_Of_Resource_Changes()
186186
await notificationRead;
187187

188188
resources = await client.ListResourcesAsync(cancellationToken: TestContext.Current.CancellationToken);
189-
Assert.Equal(6, resources.Count);
189+
Assert.Equal(8, resources.Count);
190190
Assert.Contains(resources, t => t.Name == "NewResource");
191191

192192
notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -196,7 +196,7 @@ public async Task Can_Be_Notified_Of_Resource_Changes()
196196
}
197197

198198
resources = await client.ListResourcesAsync(cancellationToken: TestContext.Current.CancellationToken);
199-
Assert.Equal(5, resources.Count);
199+
Assert.Equal(7, resources.Count);
200200
Assert.DoesNotContain(resources, t => t.Name == "NewResource");
201201
}
202202

@@ -269,6 +269,44 @@ await Assert.ThrowsAsync<McpProtocolException>(async () => await client.ReadReso
269269
Assert.IsType<InvalidOperationException>(errorLog.Exception);
270270
}
271271

272+
[Fact]
273+
public async Task Logs_Resource_Error_When_Resource_Throws_OperationCanceledException()
274+
{
275+
await using McpClient client = await CreateMcpClientForServer();
276+
277+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.ReadResourceAsync(
278+
"resource://mcp/throws_operation_canceled_exception",
279+
cancellationToken: TestContext.Current.CancellationToken));
280+
281+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
282+
m.LogLevel == LogLevel.Error &&
283+
m.Message == "ReadResource \"resource://mcp/throws_operation_canceled_exception\" threw an unhandled exception." &&
284+
m.Exception is OperationCanceledException);
285+
286+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
287+
m.LogLevel == LogLevel.Warning &&
288+
m.Message.Contains("request handler failed"));
289+
}
290+
291+
[Fact]
292+
public async Task Logs_Resource_Error_When_Resource_Throws_McpProtocolException()
293+
{
294+
await using McpClient client = await CreateMcpClientForServer();
295+
296+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.ReadResourceAsync(
297+
"resource://mcp/throws_mcp_protocol_exception",
298+
cancellationToken: TestContext.Current.CancellationToken));
299+
300+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
301+
m.LogLevel == LogLevel.Error &&
302+
m.Message == "ReadResource \"resource://mcp/throws_mcp_protocol_exception\" threw an unhandled exception." &&
303+
m.Exception is McpProtocolException);
304+
305+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
306+
m.LogLevel == LogLevel.Warning &&
307+
m.Message.Contains("request handler failed"));
308+
}
309+
272310
[Fact]
273311
public async Task Throws_Exception_On_Unknown_Resource()
274312
{
@@ -391,6 +429,12 @@ public sealed class SimpleResources
391429

392430
[McpServerResource]
393431
public static string ThrowsException() => throw new InvalidOperationException("uh oh");
432+
433+
[McpServerResource]
434+
public static string ThrowsOperationCanceledException() => throw new OperationCanceledException("Resource was canceled");
435+
436+
[McpServerResource]
437+
public static string ThrowsMcpProtocolException() => throw new McpProtocolException("Resource protocol error", McpErrorCode.InvalidParams);
394438
}
395439

396440
[McpServerResourceType]

‎tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public async Task Can_List_Registered_Tools()
127127
await using McpClient client = await CreateMcpClientForServer();
128128

129129
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
130-
Assert.Equal(17, tools.Count);
130+
Assert.Equal(19, tools.Count);
131131

132132
McpClientTool echoTool = tools.First(t => t.Name == "echo");
133133
Assert.Equal("Echoes the input back to the client.", echoTool.Description);
@@ -165,7 +165,7 @@ public async Task Can_Create_Multiple_Servers_From_Options_And_List_Registered_T
165165
cancellationToken: TestContext.Current.CancellationToken))
166166
{
167167
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
168-
Assert.Equal(17, tools.Count);
168+
Assert.Equal(19, tools.Count);
169169

170170
McpClientTool echoTool = tools.First(t => t.Name == "echo");
171171
Assert.Equal("Echoes the input back to the client.", echoTool.Description);
@@ -191,7 +191,7 @@ public async Task Can_Be_Notified_Of_Tool_Changes()
191191
await using McpClient client = await CreateMcpClientForServer();
192192

193193
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
194-
Assert.Equal(17, tools.Count);
194+
Assert.Equal(19, tools.Count);
195195

196196
Channel<JsonRpcNotification> listChanged = Channel.CreateUnbounded<JsonRpcNotification>();
197197
var notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -212,7 +212,7 @@ public async Task Can_Be_Notified_Of_Tool_Changes()
212212
await notificationRead;
213213

214214
tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
215-
Assert.Equal(18, tools.Count);
215+
Assert.Equal(20, tools.Count);
216216
Assert.Contains(tools, t => t.Name == "NewTool");
217217

218218
notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
@@ -222,7 +222,7 @@ public async Task Can_Be_Notified_Of_Tool_Changes()
222222
}
223223

224224
tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
225-
Assert.Equal(17, tools.Count);
225+
Assert.Equal(19, tools.Count);
226226
Assert.DoesNotContain(tools, t => t.Name == "NewTool");
227227
}
228228

@@ -413,6 +413,44 @@ public async Task Logs_Tool_Name_With_IsError_When_Tool_Returns_Error()
413413
Assert.Equal(LogLevel.Information, infoLog.LogLevel);
414414
}
415415

416+
[Fact]
417+
public async Task Logs_Tool_Error_When_Tool_Throws_OperationCanceledException()
418+
{
419+
await using McpClient client = await CreateMcpClientForServer();
420+
421+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.CallToolAsync(
422+
"throw_operation_canceled_exception",
423+
cancellationToken: TestContext.Current.CancellationToken));
424+
425+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
426+
m.LogLevel == LogLevel.Error &&
427+
m.Message == "\"throw_operation_canceled_exception\" threw an unhandled exception." &&
428+
m.Exception is OperationCanceledException);
429+
430+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
431+
m.LogLevel == LogLevel.Warning &&
432+
m.Message.Contains("request handler failed"));
433+
}
434+
435+
[Fact]
436+
public async Task Logs_Tool_Error_When_Tool_Throws_McpProtocolException()
437+
{
438+
await using McpClient client = await CreateMcpClientForServer();
439+
440+
await Assert.ThrowsAsync<McpProtocolException>(async () => await client.CallToolAsync(
441+
"throw_mcp_protocol_exception",
442+
cancellationToken: TestContext.Current.CancellationToken));
443+
444+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
445+
m.LogLevel == LogLevel.Error &&
446+
m.Message == "\"throw_mcp_protocol_exception\" threw an unhandled exception." &&
447+
m.Exception is McpProtocolException);
448+
449+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
450+
m.LogLevel == LogLevel.Warning &&
451+
m.Message.Contains("request handler failed"));
452+
}
453+
416454
[Fact]
417455
public async Task Throws_Exception_On_Unknown_Tool()
418456
{
@@ -819,6 +857,18 @@ public static string ThrowException()
819857
throw new InvalidOperationException("Test error");
820858
}
821859

860+
[McpServerTool]
861+
public static string ThrowOperationCanceledException()
862+
{
863+
throw new OperationCanceledException("Tool was canceled");
864+
}
865+
866+
[McpServerTool]
867+
public static string ThrowMcpProtocolException()
868+
{
869+
throw new McpProtocolException("Tool protocol error", McpErrorCode.InvalidParams);
870+
}
871+
822872
[McpServerTool]
823873
public static CallToolResult ReturnIsError()
824874
{

0 commit comments

Comments
 (0)