From adb2098e4d847ae6075f98a06b8fbca8c057f6cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=BA=C9=B5=C9=B5=D1=95=D0=BD=C4=AB?=
 <1934337+kooshi@users.noreply.github.com>
Date: Fri, 23 May 2025 16:48:47 -0500
Subject: [PATCH] Log tool invocation errors (#447)

* Log tool errors
* add test and implement source generated logging
---
 .../Server/AIFunctionMcpServerTool.cs         | 16 ++++++--
 .../Server/McpServerToolTests.cs              | 41 +++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs b/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs
index 366eb23..7f91186 100644
--- a/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs
+++ b/src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs
@@ -1,5 +1,7 @@
 using Microsoft.Extensions.AI;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 using ModelContextProtocol.Protocol;
 using System.ComponentModel;
 using System.Diagnostics.CodeAnalysis;
@@ -9,8 +11,10 @@ using System.Text.Json;
 namespace ModelContextProtocol.Server;
 
 /// <summary>Provides an <see cref="McpServerTool"/> that's implemented via an <see cref="AIFunction"/>.</summary>
-internal sealed class AIFunctionMcpServerTool : McpServerTool
+internal sealed partial class AIFunctionMcpServerTool : McpServerTool
 {
+    private readonly ILogger _logger;
+
     /// <summary>
     /// Creates an <see cref="McpServerTool"/> instance for a method, specified via a <see cref="Delegate"/> instance.
     /// </summary>
@@ -194,7 +198,7 @@ internal sealed class AIFunctionMcpServerTool : McpServerTool
             }
         }
 
-        return new AIFunctionMcpServerTool(function, tool);
+        return new AIFunctionMcpServerTool(function, tool, options?.Services);
     }
 
     private static McpServerToolCreateOptions DeriveOptions(MethodInfo method, McpServerToolCreateOptions? options)
@@ -239,10 +243,11 @@ internal sealed class AIFunctionMcpServerTool : McpServerTool
     internal AIFunction AIFunction { get; }
 
     /// <summary>Initializes a new instance of the <see cref="McpServerTool"/> class.</summary>
-    private AIFunctionMcpServerTool(AIFunction function, Tool tool)
+    private AIFunctionMcpServerTool(AIFunction function, Tool tool, IServiceProvider? serviceProvider)
     {
         AIFunction = function;
         ProtocolTool = tool;
+        _logger = serviceProvider?.GetService<ILoggerFactory>()?.CreateLogger<McpServerTool>() ?? (ILogger)NullLogger.Instance;
     }
 
     /// <inheritdoc />
@@ -277,6 +282,8 @@ internal sealed class AIFunctionMcpServerTool : McpServerTool
         }
         catch (Exception e) when (e is not OperationCanceledException)
         {
+            ToolCallError(request.Params?.Name ?? string.Empty, e);
+
             string errorMessage = e is McpException ?
                 $"An error occurred invoking '{request.Params?.Name}': {e.Message}" :
                 $"An error occurred invoking '{request.Params?.Name}'.";
@@ -359,4 +366,7 @@ internal sealed class AIFunctionMcpServerTool : McpServerTool
             IsError = allErrorContent && hasAny
         };
     }
+
+    [LoggerMessage(Level = LogLevel.Error, Message = "\"{ToolName}\" threw an unhandled exception.")]
+    private partial void ToolCallError(string toolName, Exception exception);
 }
\ No newline at end of file
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs
index cb98d9b..db6f1cd 100644
--- a/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs
+++ b/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs
@@ -1,7 +1,9 @@
 using Microsoft.Extensions.AI;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using ModelContextProtocol.Protocol;
 using ModelContextProtocol.Server;
+using ModelContextProtocol.Tests.Utils;
 using Moq;
 using System.Reflection;
 using System.Text.Json;
@@ -381,6 +383,45 @@ public partial class McpServerToolTests
         );
     }
 
+    [Fact]
+    public async Task ToolCallError_LogsErrorMessage()
+    {
+        // Arrange
+        var mockLoggerProvider = new MockLoggerProvider();
+        var loggerFactory = new LoggerFactory(new[] { mockLoggerProvider });
+        var services = new ServiceCollection();
+        services.AddSingleton<ILoggerFactory>(loggerFactory);
+        var serviceProvider = services.BuildServiceProvider();
+
+        var toolName = "tool-that-throws";
+        var exceptionMessage = "Test exception message";
+
+        McpServerTool tool = McpServerTool.Create(() =>
+        {
+            throw new InvalidOperationException(exceptionMessage);
+        }, new() { Name = toolName, Services = serviceProvider });
+
+        var mockServer = new Mock<IMcpServer>();
+        var request = new RequestContext<CallToolRequestParams>(mockServer.Object)
+        {
+            Params = new CallToolRequestParams() { Name = toolName },
+            Services = serviceProvider
+        };
+
+        // Act
+        var result = await tool.InvokeAsync(request, TestContext.Current.CancellationToken);
+
+        // Assert
+        Assert.True(result.IsError);
+        Assert.Single(result.Content);
+        Assert.Equal($"An error occurred invoking '{toolName}'.", result.Content[0].Text);
+
+        var errorLog = Assert.Single(mockLoggerProvider.LogMessages, m => m.LogLevel == LogLevel.Error);
+        Assert.Equal($"\"{toolName}\" threw an unhandled exception.", errorLog.Message);
+        Assert.IsType<InvalidOperationException>(errorLog.Exception);
+        Assert.Equal(exceptionMessage, errorLog.Exception.Message);
+    }
+
     private sealed class MyService;
 
     private class DisposableToolType : IDisposable