diff --git a/README.MD b/README.MD index 953c4ec..616975f 100644 --- a/README.MD +++ b/README.MD @@ -32,6 +32,7 @@ The library's current focus is on implementing the core MCP protocol, with the m - Support for Tool, Resource, Prompt, Sampling capabilities - Support for pagination and notifications - Async/await pattern throughout +- Comprehensive logging support - Compatible with .NET 8.0 and later ## Getting Started @@ -55,7 +56,8 @@ var config = new McpServerConfig var factory = new McpClientFactory( [config], - options + options, + NullLoggerFactory.Instance ); ``` @@ -80,6 +82,8 @@ Console.WriteLine(result.Content.FirstOrDefault(c => c.Type == "text").Text); Note that you should pass CancellationToken objects suitable for your use case, to enable proper error handling, timeouts, etc. This example also does not paginate the tools list, which may be necessary for large tool sets. See the IntegrationTests project for an example of pagination, as well as examples of how to handle Prompts and Resources. +It is also highly recommended that you pass a proper LoggerFactory instance to the factory constructor, to enable logging of MCP client operations. + Additional examples and documentation will be added as in the near future. ## License diff --git a/src/mcpdotnet.Tests/IntegrationTestFixture.cs b/src/mcpdotnet.Tests/IntegrationTestFixture.cs new file mode 100644 index 0000000..2805d9f --- /dev/null +++ b/src/mcpdotnet.Tests/IntegrationTestFixture.cs @@ -0,0 +1,49 @@ +using McpDotNet.Client; +using McpDotNet.Configuration; +using Microsoft.Extensions.Logging; + +namespace McpDotNet.Tests; + +public class IntegrationTestFixture : IDisposable +{ + public ILoggerFactory LoggerFactory { get; } + public McpClientFactory Factory { get; } + public McpClientOptions DefaultOptions { get; } + public McpServerConfig DefaultConfig { get; } + + public IntegrationTestFixture() + { + LoggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder => + builder.AddConsole() + .SetMinimumLevel(LogLevel.Debug)); + + DefaultOptions = new() + { + ClientInfo = new() { Name = "IntegrationTestClient", Version = "1.0.0" } + }; + + DefaultConfig = new McpServerConfig + { + Id = "everything", + Name = "Everything", + TransportType = "stdio", + TransportOptions = new Dictionary + { + ["command"] = "npx", + ["arguments"] = "-y @modelcontextprotocol/server-everything", + } + }; + + // Inject the mock transport into the factory + Factory = new McpClientFactory( + [DefaultConfig], + DefaultOptions, + LoggerFactory + ); + } + + public void Dispose() + { + LoggerFactory?.Dispose(); + } +} \ No newline at end of file diff --git a/src/mcpdotnet.Tests/IntegrationTests.cs b/src/mcpdotnet.Tests/IntegrationTests.cs index ef3c24b..a3ec46b 100644 --- a/src/mcpdotnet.Tests/IntegrationTests.cs +++ b/src/mcpdotnet.Tests/IntegrationTests.cs @@ -1,51 +1,26 @@ using McpDotNet.Client; using McpDotNet.Configuration; -using McpDotNet.Protocol.Messages; -using McpDotNet.Protocol.Transport; using McpDotNet.Protocol.Types; -using Moq; -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Net.Mail; -using System.Text; -using System.Threading.Channels; -using System.Threading.Tasks; +using Microsoft.Extensions.Logging; namespace McpDotNet.Tests; -public class IntegrationTests +public class IntegrationTests : IClassFixture { - private readonly McpClientOptions _defaultOptions = new() + private readonly IntegrationTestFixture _fixture; + + public IntegrationTests(IntegrationTestFixture fixture) { - ClientInfo = new() { Name = "IntegrationTestClient", Version = "1.0.0" } - }; + _fixture = fixture; + } [Fact] public async Task ConnectAndPing_Stdio_EverythingServer() { // Arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything", - } - }; - - // Inject the mock transport into the factory - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - + // Act - var client = await factory.GetClientAsync("everything"); + var client = await _fixture.Factory.GetClientAsync("everything"); await client.PingAsync(CancellationToken.None); // Assert @@ -56,25 +31,9 @@ public class IntegrationTests public async Task ListTools_Stdio_EverythingServer() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - + // act - var client = await factory.GetClientAsync("everything"); + var client = await _fixture.Factory.GetClientAsync("everything"); var tools = await client.ListToolsAsync(); // assert @@ -87,25 +46,9 @@ public class IntegrationTests public async Task CallTool_Stdio_EchoServer() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - + // act - var client = await factory.GetClientAsync("everything"); + var client = await _fixture.Factory.GetClientAsync("everything"); var result = await client.CallToolAsync( "echo", new Dictionary @@ -126,24 +69,9 @@ public class IntegrationTests public async Task ListPrompts_Stdio_EverythingServer() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); var prompts = await client.ListPromptsAsync(); // assert @@ -158,24 +86,9 @@ public class IntegrationTests public async Task GetPrompt_Stdio_SimplePrompt() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); var result = await client.GetPromptAsync("simple_prompt", null, CancellationToken.None); // assert @@ -187,24 +100,9 @@ public class IntegrationTests public async Task GetPrompt_Stdio_ComplexPrompt() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); var arguments = new Dictionary { { "temperature", "0.7" }, @@ -221,24 +119,9 @@ public class IntegrationTests public async Task GetPrompt_NonExistent_ThrowsException() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); await Assert.ThrowsAsync(() => client.GetPromptAsync("non_existent_prompt", null, CancellationToken.None)); } @@ -247,24 +130,9 @@ public class IntegrationTests public async Task ListResources_Stdio_EverythingServer() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); List allResources = new(); string? cursor = null; @@ -284,24 +152,9 @@ public class IntegrationTests public async Task ReadResource_Stdio_TextResource() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + // act + var client = await _fixture.Factory.GetClientAsync("everything"); // Odd numbered resources are text in the everything server (despite the docs saying otherwise) // 1 is index 0, which is "even" in the 0-based index var result = await client.ReadResourceAsync("test://static/resource/1", CancellationToken.None); @@ -315,24 +168,9 @@ public class IntegrationTests public async Task ReadResource_Stdio_BinaryResource() { // arrange - var config = new McpServerConfig - { - Id = "everything", - Name = "Everything", - TransportType = "stdio", - TransportOptions = new Dictionary - { - ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" - } - }; - - var factory = new McpClientFactory( - new[] { config }, - _defaultOptions - ); - - var client = await factory.GetClientAsync("everything"); + + // act + var client = await _fixture.Factory.GetClientAsync("everything"); // Even numbered resources are binary in the everything server (despite the docs saying otherwise) // 2 is index 1, which is "odd" in the 0-based index var result = await client.ReadResourceAsync("test://static/resource/2", CancellationToken.None); @@ -354,7 +192,7 @@ public class IntegrationTests TransportOptions = new Dictionary { ["command"] = "npx", - ["arguments"] = "-y @modelcontextprotocol/server-everything" + ["arguments"] = "-y @modelcontextprotocol/server-everything", } }; @@ -368,13 +206,15 @@ public class IntegrationTests }; var factory = new McpClientFactory( - new[] { config }, - options + [config], + options, + _fixture.LoggerFactory ); var client = await factory.GetClientAsync("everything"); // Set up the sampling handler int samplingHandlerCalls = 0; +#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously client.SamplingHandler = async (_) => { samplingHandlerCalls++; @@ -389,6 +229,7 @@ public class IntegrationTests } }; }; +#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously // Call the server's sampleLLM tool which should trigger our sampling handler var result = await client.CallToolAsync( @@ -404,6 +245,6 @@ public class IntegrationTests Assert.NotNull(result); var textContent = Assert.Single(result.Content); Assert.Equal("text", textContent.Type); - Assert.Contains("Test response", textContent.Text); + Assert.False(string.IsNullOrEmpty(textContent.Text)); } } diff --git a/src/mcpdotnet.Tests/McpClientFactoryTests.cs b/src/mcpdotnet.Tests/McpClientFactoryTests.cs index e728751..5d8dd3d 100644 --- a/src/mcpdotnet.Tests/McpClientFactoryTests.cs +++ b/src/mcpdotnet.Tests/McpClientFactoryTests.cs @@ -1,7 +1,9 @@ +using Castle.Core.Logging; using McpDotNet.Client; using McpDotNet.Configuration; using McpDotNet.Protocol.Messages; using McpDotNet.Protocol.Transport; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using System.Threading.Channels; using Xunit; @@ -47,10 +49,11 @@ public class McpClientFactoryTests // Inject the mock transport into the factory var factory = new McpClientFactory( - new[] { config }, + [config], _defaultOptions, + NullLoggerFactory.Instance, transportFactoryMethod: _ => mockTransport.Object, - clientFactoryMethod: (_, _) => mockClient.Object + clientFactoryMethod: (_, _, _) => mockClient.Object ); // Act @@ -86,7 +89,11 @@ public class McpClientFactoryTests .Returns(Task.CompletedTask); mockClient.Setup(c => c.IsInitialized).Returns(true); - var factory = new McpClientFactory(new[] { config }, _defaultOptions, transportFactoryMethod: _ => mockTransport.Object, clientFactoryMethod: (_,_) => mockClient.Object); + var factory = new McpClientFactory([config], + _defaultOptions, + NullLoggerFactory.Instance, + transportFactoryMethod: _ => mockTransport.Object, + clientFactoryMethod: (_,_,_) => mockClient.Object); // Act var client1 = await factory.GetClientAsync("test-server"); @@ -100,7 +107,9 @@ public class McpClientFactoryTests public async Task GetClientAsync_WithInvalidServerId_ThrowsArgumentException() { // Arrange - var factory = new McpClientFactory(Array.Empty(), _defaultOptions); + var factory = new McpClientFactory(Array.Empty(), + _defaultOptions, + NullLoggerFactory.Instance); // Act & Assert await Assert.ThrowsAsync( @@ -120,7 +129,8 @@ public class McpClientFactoryTests Location = "/path/to/server" }; - var factory = new McpClientFactory(new[] { config }, _defaultOptions); + var factory = new McpClientFactory([config], _defaultOptions, + NullLoggerFactory.Instance); // Act & Assert await Assert.ThrowsAsync( @@ -153,7 +163,11 @@ public class McpClientFactoryTests .Returns(Task.CompletedTask); mockClient.Setup(c => c.IsInitialized).Returns(true); - var factory = new McpClientFactory(new[] { config }, _defaultOptions, transportFactoryMethod: _ => mockTransport.Object, clientFactoryMethod: (_, _) => mockClient.Object); + var factory = new McpClientFactory([config], + _defaultOptions, + NullLoggerFactory.Instance, + transportFactoryMethod: _ => mockTransport.Object, + clientFactoryMethod: (_, _, _) => mockClient.Object); // Act var client = await factory.GetClientAsync("test-server"); @@ -166,14 +180,15 @@ public class McpClientFactoryTests public void Constructor_WithDuplicateServerIds_ThrowsArgumentException() { // Arrange - var configs = new[] - { + McpServerConfig[] configs = + [ new McpServerConfig { Id = "duplicate", Name = "duplicate", TransportType = "stdio", Location = "/path1" }, new McpServerConfig { Id = "duplicate", Name = "duplicate", TransportType = "stdio", Location = "/path2" } - }; + ]; // Act & Assert - Assert.Throws(() => new McpClientFactory(configs, _defaultOptions)); + Assert.Throws(() => new McpClientFactory(configs, _defaultOptions, + NullLoggerFactory.Instance)); } [Fact] @@ -188,7 +203,8 @@ public class McpClientFactoryTests Location = "http://localhost:8080" }; - var factory = new McpClientFactory(new[] { config }, _defaultOptions); + var factory = new McpClientFactory([config], _defaultOptions, + NullLoggerFactory.Instance); // Act & Assert await Assert.ThrowsAsync( diff --git a/src/mcpdotnet.Tests/mcpdotnet.Tests.csproj b/src/mcpdotnet.Tests/mcpdotnet.Tests.csproj index 7140ca0..51dc6ef 100644 --- a/src/mcpdotnet.Tests/mcpdotnet.Tests.csproj +++ b/src/mcpdotnet.Tests/mcpdotnet.Tests.csproj @@ -10,6 +10,8 @@ + + diff --git a/src/mcpdotnet/Client/McpClient.cs b/src/mcpdotnet/Client/McpClient.cs index 5f02409..23d514f 100644 --- a/src/mcpdotnet/Client/McpClient.cs +++ b/src/mcpdotnet/Client/McpClient.cs @@ -1,14 +1,15 @@ -namespace McpDotNet.Client; - + using McpDotNet.Protocol.Types; using McpDotNet.Protocol.Transport; using System.Collections.Concurrent; using McpDotNet.Protocol.Messages; -using System.Diagnostics; using System.Text.Json; using McpDotNet.Utils.Json; -using System.Text; -using System.Threading; +using Microsoft.Extensions.Logging; +using McpDotNet.Logging; +using McpDotNet.Configuration; + +namespace McpDotNet.Client; /// internal class McpClient : IMcpClient @@ -19,13 +20,16 @@ internal class McpClient : IMcpClient private readonly ConcurrentDictionary>> _notificationHandlers; private readonly Dictionary>> _requestHandlers = new(); private int _nextRequestId; + private readonly McpServerConfig _serverConfig; private bool _isInitialized; private Task? _messageProcessingTask; private CancellationTokenSource? _cts; - private readonly JsonSerializerOptions _jsonOptions = new JsonSerializerOptions().ConfigureForMcp(); + private readonly JsonSerializerOptions _jsonOptions; + private readonly ILogger _logger; + private volatile bool _isInitializing; /// - public bool IsInitialized => _isInitialized; + public bool IsInitialized => _isInitialized; /// public ServerCapabilities? ServerCapabilities { get; private set; } @@ -38,20 +42,29 @@ internal class McpClient : IMcpClient /// /// An MCP transport implementation. /// Options for the client, defining protocol version and capabilities. - public McpClient(IMcpTransport transport, McpClientOptions options) + /// The server configuration. + /// The logger factory. + public McpClient(IMcpTransport transport, McpClientOptions options, McpServerConfig serverConfig, ILoggerFactory loggerFactory) { _transport = transport; _options = options; _pendingRequests = new(); _notificationHandlers = new(); _nextRequestId = 1; + _serverConfig = serverConfig; + _jsonOptions = new JsonSerializerOptions().ConfigureForMcp(loggerFactory); + _logger = loggerFactory.CreateLogger(); if (options.Capabilities?.Sampling != null) { SetRequestHandler("sampling/createMessage", async (request) => { if (SamplingHandler == null) + { + // Setting the capability, but not a handler means we have nothing to return to the server + _logger.SamplingHandlerNotConfigured(_serverConfig.Id, _serverConfig.Name); throw new McpClientException("Sampling handler not configured"); + } return await SamplingHandler(request); }); @@ -61,9 +74,17 @@ internal class McpClient : IMcpClient /// public async Task ConnectAsync(CancellationToken cancellationToken = default) { + if (_isInitializing) + { + _logger.ClientAlreadyInitializing(_serverConfig.Id, _serverConfig.Name); + throw new InvalidOperationException("Client is already initializing"); + } + _isInitializing = true; + if (_isInitialized) { - throw new InvalidOperationException("Client is already initialized"); + _logger.ClientAlreadyInitialized(_serverConfig.Id, _serverConfig.Name); + return; } try @@ -83,10 +104,14 @@ internal class McpClient : IMcpClient } catch (Exception e) { - Debug.WriteLine($"Client connection error: {e}"); + _logger.ClientInitializationError(_serverConfig.Id, _serverConfig.Name, e); await CleanupAsync().ConfigureAwait(false); throw; } + finally + { + _isInitializing = false; + } } private async Task InitializeAsync(CancellationToken cancellationToken) @@ -112,12 +137,14 @@ internal class McpClient : IMcpClient ).ConfigureAwait(false); // Store server information + _logger.ServerCapabilitiesReceived(_serverConfig.Id, _serverConfig.Name, JsonSerializer.Serialize(initializeResponse.Capabilities), JsonSerializer.Serialize(initializeResponse.ServerInfo)); ServerCapabilities = initializeResponse.Capabilities; ServerInfo = initializeResponse.ServerInfo; // Validate protocol version if (initializeResponse.ProtocolVersion != _options.ProtocolVersion) { + _logger.ServerProtocolVersionMismatch(_serverConfig.Id, _serverConfig.Name, _options.ProtocolVersion, initializeResponse.ProtocolVersion); throw new McpClientException($"Server protocol version mismatch. Expected {_options.ProtocolVersion}, got {initializeResponse.ProtocolVersion}"); } @@ -132,6 +159,7 @@ internal class McpClient : IMcpClient } catch (OperationCanceledException) when (initializationCts.IsCancellationRequested) { + _logger.ClientInitializationTimeout(_serverConfig.Id, _serverConfig.Name); throw new McpClientException("Initialization timed out"); } } @@ -139,6 +167,7 @@ internal class McpClient : IMcpClient /// public async Task PingAsync(CancellationToken cancellationToken) { + _logger.PingingServer(_serverConfig.Id, _serverConfig.Name); await SendRequestAsync( new JsonRpcRequest { @@ -151,6 +180,7 @@ internal class McpClient : IMcpClient /// public async Task ListToolsAsync(string? cursor = null, CancellationToken cancellationToken = default) { + _logger.ListingTools(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)"); return await SendRequestAsync( new JsonRpcRequest { @@ -164,6 +194,7 @@ internal class McpClient : IMcpClient /// public async Task ListPromptsAsync(string? cursor = null, CancellationToken cancellationToken = default) { + _logger.ListingPrompts(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)"); return await SendRequestAsync( new JsonRpcRequest { @@ -177,6 +208,7 @@ internal class McpClient : IMcpClient /// public async Task GetPromptAsync(string name, Dictionary? arguments = null, CancellationToken cancellationToken = default) { + _logger.GettingPrompt(_serverConfig.Id, _serverConfig.Name, name, arguments == null ? "{}" : JsonSerializer.Serialize(arguments)); return await SendRequestAsync( new JsonRpcRequest { @@ -190,6 +222,7 @@ internal class McpClient : IMcpClient /// public async Task ListResourcesAsync(string? cursor = null, CancellationToken cancellationToken = default) { + _logger.ListingResources(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)"); return await SendRequestAsync( new JsonRpcRequest { @@ -203,6 +236,7 @@ internal class McpClient : IMcpClient /// public async Task ReadResourceAsync(string uri, CancellationToken cancellationToken = default) { + _logger.ReadingResource(_serverConfig.Id, _serverConfig.Name, uri); return await SendRequestAsync( new JsonRpcRequest { @@ -220,6 +254,7 @@ internal class McpClient : IMcpClient /// public async Task SubscribeToResourceAsync(string uri, CancellationToken cancellationToken = default) { + _logger.SubscribingToResource(_serverConfig.Id, _serverConfig.Name, uri); await SendRequestAsync( new JsonRpcRequest { @@ -237,6 +272,7 @@ internal class McpClient : IMcpClient /// public async Task UnsubscribeFromResourceAsync(string uri, CancellationToken cancellationToken = default) { + _logger.UnsubscribingFromResource(_serverConfig.Id, _serverConfig.Name, uri); await SendRequestAsync( new JsonRpcRequest { @@ -254,6 +290,7 @@ internal class McpClient : IMcpClient public async Task CallToolAsync(string toolName, Dictionary? arguments = null, CancellationToken cancellationToken = default) { + _logger.CallingTool(_serverConfig.Id, _serverConfig.Name, toolName, arguments == null ? "{}" : JsonSerializer.Serialize(arguments)); return await SendRequestAsync( new JsonRpcRequest { @@ -291,6 +328,7 @@ internal class McpClient : IMcpClient catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { // Normal shutdown + _logger.ClientMessageProcessingCancelled(_serverConfig.Id, _serverConfig.Name); } catch (NullReferenceException) { @@ -307,7 +345,9 @@ internal class McpClient : IMcpClient { try { + _logger.RequestHandlerCalled(_serverConfig.Id, _serverConfig.Name, request.Method); var result = await handler(request); + _logger.RequestHandlerCompleted(_serverConfig.Id, _serverConfig.Name, request.Method); await _transport.SendMessageAsync(new JsonRpcResponse { Id = request.Id, @@ -317,6 +357,7 @@ internal class McpClient : IMcpClient } catch (Exception ex) { + _logger.RequestHandlerError(_serverConfig.Id, _serverConfig.Name, request.Method, ex); // Send error response await _transport.SendMessageAsync(new JsonRpcError { @@ -336,6 +377,10 @@ internal class McpClient : IMcpClient { tcs.TrySetResult(message); } + else + { + _logger.NoRequestFoundForMessageWithId(_serverConfig.Id, _serverConfig.Name, messageWithId.Id.ToString()); + } break; case JsonRpcNotification notification: @@ -350,7 +395,7 @@ internal class McpClient : IMcpClient catch (Exception ex) { // Log handler error but continue processing - Debug.WriteLine($"Notification handler error: {ex}"); + _logger.NotificationHandlerError(_serverConfig.Id, _serverConfig.Name, notification.Method, ex); } } } @@ -363,6 +408,7 @@ internal class McpClient : IMcpClient { if (!_transport.IsConnected) { + _logger.ClientNotConnected(_serverConfig.Id, _serverConfig.Name); throw new McpClientException("Transport is not connected"); } @@ -374,13 +420,21 @@ internal class McpClient : IMcpClient try { - Debug.WriteLine($"Sending initialize request: {JsonSerializer.Serialize(request)}"); + // Expensive logging, use the logging framework to check if the logger is enabled + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.SendingRequestPayload(_serverConfig.Id, _serverConfig.Name, JsonSerializer.Serialize(request)); + } + + // Less expensive information logging + _logger.SendingRequest(_serverConfig.Id, _serverConfig.Name, request.Method); await _transport.SendMessageAsync(request, cancellationToken).ConfigureAwait(false); var response = await tcs.Task.WaitAsync(cancellationToken).ConfigureAwait(false); if (response is JsonRpcError error) { + _logger.RequestFailed(_serverConfig.Id, _serverConfig.Name, request.Method, error.Error.Message, error.Error.Code); throw new McpClientException($"Request failed: {error.Error.Message}", error.Error.Code); } @@ -389,15 +443,23 @@ internal class McpClient : IMcpClient // Convert the Result object to JSON and back to get our strongly-typed result var resultJson = JsonSerializer.Serialize(success.Result, _jsonOptions); var resultObject = JsonSerializer.Deserialize(resultJson, _jsonOptions); + + // Not expensive logging because we're already converting to JSON in order to get the result object + _logger.RequestResponseReceivedPayload(_serverConfig.Id, _serverConfig.Name, resultJson); + _logger.RequestResponseReceived(_serverConfig.Id, _serverConfig.Name, request.Method); + if (resultObject != null) { return resultObject; } - Debug.WriteLine($"Received response: {JsonSerializer.Serialize(success.Result)}"); - Debug.WriteLine($"Expected type: {typeof(T)}"); + + // Result object was null, this is unexpected + _logger.RequestResponseTypeConversionError(_serverConfig.Id, _serverConfig.Name, request.Method, typeof(T)); throw new McpClientException($"Unexpected response type {JsonSerializer.Serialize(success.Result)}, expected {typeof(T)}"); } + // Unexpected response type + _logger.RequestInvalidResponseType(_serverConfig.Id, _serverConfig.Name, request.Method); throw new McpClientException("Invalid response type"); } finally @@ -410,6 +472,7 @@ internal class McpClient : IMcpClient { if (!_transport.IsConnected) { + _logger.ClientNotConnected(_serverConfig.Id, _serverConfig.Name); throw new McpClientException("Transport is not connected"); } @@ -434,12 +497,21 @@ internal class McpClient : IMcpClient // Convert the params JsonElement to our type using the same options var jsonString = JsonSerializer.Serialize(request.Params); var typedRequest = JsonSerializer.Deserialize(jsonString, _jsonOptions); + + if (typedRequest == null) + { + _logger.RequestParamsTypeConversionError(_serverConfig.Id, _serverConfig.Name, method, typeof(TRequest)); + throw new McpClientException($"Invalid request parameters type {jsonString}, expected {typeof(TRequest)}"); + } + return await handler(typedRequest); }; } private async Task CleanupAsync() { + _logger.CleaningUpClient(_serverConfig.Id, _serverConfig.Name); + _cts?.Cancel(); if (_messageProcessingTask != null) @@ -465,6 +537,8 @@ internal class McpClient : IMcpClient _cts?.Dispose(); _isInitialized = false; + + _logger.ClientCleanedUp(_serverConfig.Id, _serverConfig.Name); } /// diff --git a/src/mcpdotnet/Client/McpClientFactory.cs b/src/mcpdotnet/Client/McpClientFactory.cs index 7b9200f..2bb2cd6 100644 --- a/src/mcpdotnet/Client/McpClientFactory.cs +++ b/src/mcpdotnet/Client/McpClientFactory.cs @@ -1,8 +1,9 @@ -namespace McpDotNet.Client; - -using McpDotNet.Configuration; +using McpDotNet.Configuration; +using McpDotNet.Logging; using McpDotNet.Protocol.Transport; +using Microsoft.Extensions.Logging; +namespace McpDotNet.Client; /// /// Factory for creating MCP clients based on configuration. It caches clients for reuse, so it is safe to call GetClientAsync multiple times. /// Call GetClientAsync to get a client for a specific server (by ID), which will create a new client and connect if it doesn't already exist. @@ -15,7 +16,9 @@ public class McpClientFactory private readonly McpClientOptions _clientOptions; private readonly Dictionary _clients = new(); private readonly Func _transportFactoryMethod; - private readonly Func _clientFactoryMethod; + private readonly Func _clientFactoryMethod; + private readonly ILoggerFactory _loggerFactory; + private readonly ILogger _logger; /// /// Initializes a new instance of the class. @@ -24,18 +27,33 @@ public class McpClientFactory /// /// Configuration objects for each server the factory should support. /// A configuration object which specifies client capabilities and protocol version. + /// A logger factory for creating loggers for clients. /// An optional factory method which returns transport implementations based on a server configuration. /// An optional factory method which creates a client based on client options and transport implementation. public McpClientFactory( IEnumerable serverConfigs, McpClientOptions clientOptions, + ILoggerFactory loggerFactory, Func? transportFactoryMethod = null, - Func? clientFactoryMethod = null) + Func? clientFactoryMethod = null) { _serverConfigs = serverConfigs.ToDictionary(c => c.Id); _clientOptions = clientOptions; + _loggerFactory = loggerFactory; + _logger = loggerFactory.CreateLogger(); _transportFactoryMethod = transportFactoryMethod ?? CreateTransport; - _clientFactoryMethod = clientFactoryMethod ?? ((transport, options) => new McpClient(transport, options)); + _clientFactoryMethod = clientFactoryMethod ?? ((transport, serverConfig, options) => new McpClient(transport, options, serverConfig, loggerFactory)); + + // Initialize commands for stdio transport, this is to run commands in a shell even if specified directly, as otherwise + // the stdio protocol will not work correctly. + _logger.InitializingStdioCommands(); + foreach (var config in _serverConfigs.Values) + { + if (config.TransportType.ToLowerInvariant() == "stdio") + { + InitializeCommand(config); + } + } } /// @@ -49,24 +67,32 @@ public class McpClientFactory { if (!_serverConfigs.TryGetValue(serverId, out var config)) { + _logger.ServerNotFound(serverId); throw new ArgumentException($"Server with ID '{serverId}' not found.", nameof(serverId)); } if (_clients.TryGetValue(serverId, out var existingClient)) { + _logger.ClientExists(serverId, config.Name); return existingClient; } + _logger.CreatingClient(serverId, config.Name); + var transport = _transportFactoryMethod(config); - var client = _clientFactoryMethod(transport, _clientOptions); + var client = _clientFactoryMethod(transport, config, _clientOptions); await client.ConnectAsync(cancellationToken); + _logger.ClientCreated(serverId, config.Name); + _clients[serverId] = client; return client; } private IMcpTransport CreateTransport(McpServerConfig config) { + var options = string.Join(", ", config.TransportOptions?.Select(kv => $"{kv.Key}={kv.Value}") ?? Enumerable.Empty()); + _logger.CreatingTransport(config.Id, config.Name, config.TransportType, options); return config.TransportType.ToLowerInvariant() switch { "stdio" => new StdioTransport(new StdioTransportOptions @@ -77,34 +103,47 @@ public class McpClientFactory EnvironmentVariables = config.TransportOptions? .Where(kv => kv.Key.StartsWith("env:")) .ToDictionary(kv => kv.Key.Substring(4), kv => kv.Value) - }), - // Add other transport types here + }, config, _loggerFactory), _ => throw new ArgumentException($"Unsupported transport type '{config.TransportType}'.", nameof(config)) }; } private string GetCommand(McpServerConfig config) { - if (config.TransportOptions == null) + if (config.TransportOptions == null || + string.IsNullOrEmpty(config.TransportOptions.GetValueOrDefault("command"))) { return config.Location!; } - var command = config.TransportOptions.GetValueOrDefault("command"); - if (string.IsNullOrEmpty(command)) - { - return config.Location!; - } - - if (config.TransportOptions.ContainsKey("arguments")) - { - var arguments = config.TransportOptions.GetValueOrDefault("arguments"); - config.TransportOptions["arguments"] = $"/c {command} {arguments}"; - } - else - { - config.TransportOptions["arguments"] = $"/c {command}"; - } return $"cmd.exe"; } + + /// + /// Initializes a non-shell command by injecting a /c {command} argument, as the command will be run in a shell. + /// + private void InitializeCommand(McpServerConfig config) + { + // If the command is empty or already contains cmd.exe, we don't need to do anything + var command = config.TransportOptions?.GetValueOrDefault("command"); + if (string.IsNullOrEmpty(command)) + { + return; + } + + if (command.Contains("cmd.exe")) + { + _logger.SkippingShellWrapper(config.Id, config.Name); + return; + } + + // If the command is not empty and does not contain cmd.exe, we need to inject /c {command} + if (config.TransportOptions != null && !string.IsNullOrEmpty(command)) + { + _logger.PromotingCommandToShellArgumentForStdio(config.Id, config.Name, command, config.TransportOptions.GetValueOrDefault("arguments") ?? ""); + config.TransportOptions["arguments"] = config.TransportOptions.ContainsKey("arguments") + ? $"/c {command} {config.TransportOptions["arguments"]}" + : $"/c {command}"; + } + } } \ No newline at end of file diff --git a/src/mcpdotnet/Logging/Log.cs b/src/mcpdotnet/Logging/Log.cs new file mode 100644 index 0000000..dca0a7e --- /dev/null +++ b/src/mcpdotnet/Logging/Log.cs @@ -0,0 +1,271 @@ +using McpDotNet.Configuration; +using Microsoft.Extensions.Logging; + +namespace McpDotNet.Logging; + +/// +/// Logging methods for the McpDotNet library. +/// +internal static partial class Log +{ + [LoggerMessage(Level = LogLevel.Debug, Message = "Client {clientId} initializing connection to server {serverId}")] + internal static partial void ClientConnecting(this ILogger logger, string clientId, string serverId); + + [LoggerMessage(Level = LogLevel.Information, Message = "Server capabilities received: {capabilities}")] + internal static partial void ServerCapabilitiesReceived(this ILogger logger, string capabilities); + + [LoggerMessage(Level = LogLevel.Warning, Message = "Request {requestId} timed out")] + internal static partial void RequestTimeout(this ILogger logger, int requestId); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Command for {ServerId} ({Name}) already contains shell wrapper, skipping argument injection")] + internal static partial void SkippingShellWrapper(this ILogger logger, string serverId, string name); + + [LoggerMessage(Level = LogLevel.Error, Message = "Server config for Id={serverId} not found")] + internal static partial void ServerNotFound(this ILogger logger, string serverId); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Client for {serverId} ({name}) already created, returning cached client")] + internal static partial void ClientExists(this ILogger logger, string serverId, string name); + + [LoggerMessage(Level = LogLevel.Information, Message = "Creating client for {serverId} ({name})")] + internal static partial void CreatingClient(this ILogger logger, string serverId, string name); + + [LoggerMessage(Level = LogLevel.Information, Message = "Client for {serverId} ({name}) created and connected")] + internal static partial void ClientCreated(this ILogger logger, string serverId, string name); + + [LoggerMessage(Level = LogLevel.Information, Message = "Creating transport for {serverId} ({name}) with type {transportType} and options {options}")] + internal static partial void CreatingTransport(this ILogger logger, string serverId, string name, string transportType, string options); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Promoting command for {serverId} ({name}) to shell argument for stdio transport with command {command} and arguments {arguments}")] + internal static partial void PromotingCommandToShellArgumentForStdio(this ILogger logger, string serverId, string name, string command, string arguments); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Initializing stdio commands")] + internal static partial void InitializingStdioCommands(this ILogger logger); + + [LoggerMessage(Level = LogLevel.Error, Message = "Sampling handler not configured for server {serverId} ({serverName}), always set a handler when using this capability")] + internal static partial void SamplingHandlerNotConfigured(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Client server {serverId} ({serverName}) already initializing")] + internal static partial void ClientAlreadyInitializing(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Client server {serverId} ({serverName}) already initialized")] + internal static partial void ClientAlreadyInitialized(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Client server {serverId} ({serverName}) initialization error")] + internal static partial void ClientInitializationError(this ILogger logger, string serverId, string serverName, Exception exception); + + [LoggerMessage(Level = LogLevel.Information, Message = "Server {serverId} ({serverName}) capabilities received: {capabilities}, server info: {serverInfo}")] + internal static partial void ServerCapabilitiesReceived(this ILogger logger, string serverId, string serverName, string capabilities, string serverInfo); + + [LoggerMessage(Level = LogLevel.Error, Message = "Server {serverId} ({serverName}) protocol version mismatch, expected {expected}, received {received}")] + internal static partial void ServerProtocolVersionMismatch(this ILogger logger, string serverId, string serverName, string expected, string received); + + [LoggerMessage(Level = LogLevel.Error, Message = "Client server {serverId} ({serverName}) initialization timeout")] + internal static partial void ClientInitializationTimeout(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Pinging server {serverId} ({serverName})")] + internal static partial void PingingServer(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Listing tools for server {serverId} ({serverName}) with cursor {cursor}")] + internal static partial void ListingTools(this ILogger logger, string serverId, string serverName, string cursor); + + [LoggerMessage(Level = LogLevel.Information, Message = "Listing prompts for server {serverId} ({serverName}) with cursor {cursor}")] + internal static partial void ListingPrompts(this ILogger logger, string serverId, string serverName, string cursor); + + [LoggerMessage(Level = LogLevel.Information, Message = "Getting prompt {name} for server {serverId} ({serverName}) with arguments {arguments}")] + internal static partial void GettingPrompt(this ILogger logger, string serverId, string serverName, string name, string arguments); + + [LoggerMessage(Level = LogLevel.Information, Message = "Listing resources for server {serverId} ({serverName}) with cursor {cursor}")] + internal static partial void ListingResources(this ILogger logger, string serverId, string serverName, string cursor); + + [LoggerMessage(Level = LogLevel.Information, Message = "Reading resource {uri} for server {serverId} ({serverName})")] + internal static partial void ReadingResource(this ILogger logger, string serverId, string serverName, string uri); + + [LoggerMessage(Level = LogLevel.Information, Message = "Subscribing to resource {uri} for server {serverId} ({serverName})")] + internal static partial void SubscribingToResource(this ILogger logger, string serverId, string serverName, string uri); + + [LoggerMessage(Level = LogLevel.Information, Message = "Unsubscribing from resource {uri} for server {serverId} ({serverName})")] + internal static partial void UnsubscribingFromResource(this ILogger logger, string serverId, string serverName, string uri); + + [LoggerMessage(Level = LogLevel.Information, Message = "Calling tool {toolName} for server {serverId} ({serverName}) with arguments {arguments}")] + internal static partial void CallingTool(this ILogger logger, string serverId, string serverName, string toolName, string arguments); + + [LoggerMessage(Level = LogLevel.Information, Message = "Client message processing cancelled for server {serverId} ({serverName})")] + internal static partial void ClientMessageProcessingCancelled(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Request handler called for server {serverId} ({serverName}) with method {method}")] + internal static partial void RequestHandlerCalled(this ILogger logger, string serverId, string serverName, string method); + + [LoggerMessage(Level = LogLevel.Information, Message = "Request handler completed for server {serverId} ({serverName}) with method {method}")] + internal static partial void RequestHandlerCompleted(this ILogger logger, string serverId, string serverName, string method); + + [LoggerMessage(Level = LogLevel.Error, Message = "Request handler error for server {serverId} ({serverName}) with method {method}")] + internal static partial void RequestHandlerError(this ILogger logger, string serverId, string serverName, string method, Exception exception); + + [LoggerMessage(Level = LogLevel.Warning, Message = "No request found for message with ID {messageWithId} for server {serverId} ({serverName})")] + internal static partial void NoRequestFoundForMessageWithId(this ILogger logger, string serverId, string serverName, string messageWithId); + + [LoggerMessage(Level = LogLevel.Error, Message = "Notification handler error for server {serverId} ({serverName}) with method {method}")] + internal static partial void NotificationHandlerError(this ILogger logger, string serverId, string serverName, string method, Exception exception); + + [LoggerMessage(Level = LogLevel.Error, Message = "Client not connected for server {serverId} ({serverName})")] + internal static partial void ClientNotConnected(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Sending request payload for server {serverId} ({serverName}): {payload}")] + internal static partial void SendingRequestPayload(this ILogger logger, string serverId, string serverName, string payload); + + [LoggerMessage(Level = LogLevel.Information, Message = "Sending request for server {serverId} ({serverName}) with method {method}")] + internal static partial void SendingRequest(this ILogger logger, string serverId, string serverName, string method); + + [LoggerMessage(Level = LogLevel.Error, Message = "Request failed for server {serverId} ({serverName}) with method {method}: {message} ({code})")] + internal static partial void RequestFailed(this ILogger logger, string serverId, string serverName, string method, string message, int code); + + [LoggerMessage(Level = LogLevel.Information, Message = "Request response received payload for server {serverId} ({serverName}): {payload}")] + internal static partial void RequestResponseReceivedPayload(this ILogger logger, string serverId, string serverName, string payload); + + [LoggerMessage(Level = LogLevel.Information, Message = "Request response received for server {serverId} ({serverName}) with method {method}")] + internal static partial void RequestResponseReceived(this ILogger logger, string serverId, string serverName, string method); + + [LoggerMessage(Level = LogLevel.Error, Message = "Request response type conversion error for server {serverId} ({serverName}) with method {method}: expected {expectedType}")] + internal static partial void RequestResponseTypeConversionError(this ILogger logger, string serverId, string serverName, string method, Type expectedType); + + [LoggerMessage(Level = LogLevel.Error, Message = "Request invalid response type for server {serverId} ({serverName}) with method {method}")] + internal static partial void RequestInvalidResponseType(this ILogger logger, string serverId, string serverName, string method); + + [LoggerMessage(Level = LogLevel.Error, Message = "Request params type conversion error for server {serverId} ({serverName}) with method {method}: expected {expectedType}")] + internal static partial void RequestParamsTypeConversionError(this ILogger logger, string serverId, string serverName, string method, Type expectedType); + + [LoggerMessage(Level = LogLevel.Information, Message = "Cleaning up client for server {serverId} ({serverName})")] + internal static partial void CleaningUpClient(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Client cleaned up for server {serverId} ({serverName})")] + internal static partial void ClientCleanedUp(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport for server {serverId} ({serverName}) already connected")] + internal static partial void TransportAlreadyConnected(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport for server {serverId} ({serverName}) connecting")] + internal static partial void TransportConnecting(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Creating process for transport for server {serverId} ({serverName}) with command {command}, arguments {arguments}, environment {environment}, working directory {workingDirectory}, shutdown timeout {shutdownTimeout}")] + internal static partial void CreateProcessForTransport(this ILogger logger, string serverId, string serverName, string command, string arguments, string environment, string workingDirectory, string shutdownTimeout); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport for server {serverId} ({serverName}) error: {data}")] + internal static partial void TransportError(this ILogger logger, string serverId, string serverName, string data); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport process start failed for server {serverId} ({serverName})")] + internal static partial void TransportProcessStartFailed(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport process started for server {serverId} ({serverName}) with PID {processId}")] + internal static partial void TransportProcessStarted(this ILogger logger, string serverId, string serverName, int processId); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport reading messages for server {serverId} ({serverName})")] + internal static partial void TransportReadingMessages(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport connect failed for server {serverId} ({serverName})")] + internal static partial void TransportConnectFailed(this ILogger logger, string serverId, string serverName, Exception exception); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport not connected for server {serverId} ({serverName})")] + internal static partial void TransportNotConnected(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport sending message for server {serverId} ({serverName}) with ID {messageId}, JSON {json}")] + internal static partial void TransportSendingMessage(this ILogger logger, string serverId, string serverName, string messageId, string json); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport message sent for server {serverId} ({serverName}) with ID {messageId}")] + internal static partial void TransportSentMessage(this ILogger logger, string serverId, string serverName, string messageId); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport send failed for server {serverId} ({serverName}) with ID {messageId}")] + internal static partial void TransportSendFailed(this ILogger logger, string serverId, string serverName, string messageId, Exception exception); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport entering read messages loop for server {serverId} ({serverName})")] + internal static partial void TransportEnteringReadMessagesLoop(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport waiting for message for server {serverId} ({serverName})")] + internal static partial void TransportWaitingForMessage(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport end of stream for server {serverId} ({serverName})")] + internal static partial void TransportEndOfStream(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport received message for server {serverId} ({serverName}): {line}")] + internal static partial void TransportReceivedMessage(this ILogger logger, string serverId, string serverName, string line); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport received message parsed for server {serverId} ({serverName}): {messageId}")] + internal static partial void TransportReceivedMessageParsed(this ILogger logger, string serverId, string serverName, string messageId); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport message written for server {serverId} ({serverName}) with ID {messageId}")] + internal static partial void TransportMessageWritten(this ILogger logger, string serverId, string serverName, string messageId); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport message parse failed due to unexpected message schema for server {serverId} ({serverName}): {line}")] + internal static partial void TransportMessageParseUnexpectedType(this ILogger logger, string serverId, string serverName, string line); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport message parse failed for server {serverId} ({serverName}): {line}")] + internal static partial void TransportMessageParseFailed(this ILogger logger, string serverId, string serverName, string line, Exception exception); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport exiting read messages loop for server {serverId} ({serverName})")] + internal static partial void TransportExitingReadMessagesLoop(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport read messages cancelled for server {serverId} ({serverName})")] + internal static partial void TransportReadMessagesCancelled(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport read messages failed for server {serverId} ({serverName})")] + internal static partial void TransportReadMessagesFailed(this ILogger logger, string serverId, string serverName, Exception exception); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport cleaning up for server {serverId} ({serverName})")] + internal static partial void TransportCleaningUp(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Transport closing stdin for server {serverId} ({serverName})")] + internal static partial void TransportClosingStdin(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Transport waiting for shutdown for server {serverId} ({serverName})")] + internal static partial void TransportWaitingForShutdown(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Warning, Message = "Transport killing process for server {serverId} ({serverName})")] + internal static partial void TransportKillingProcess(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport shutdown failed for server {serverId} ({serverName})")] + internal static partial void TransportShutdownFailed(this ILogger logger, string serverId, string serverName, Exception exception); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Transport waiting for read task for server {serverId} ({serverName})")] + internal static partial void TransportWaitingForReadTask(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Warning, Message = "Transport cleanup read task timeout for server {serverId} ({serverName})")] + internal static partial void TransportCleanupReadTaskTimeout(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport cleanup read task cancelled for server {serverId} ({serverName})")] + internal static partial void TransportCleanupReadTaskCancelled(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "Transport cleanup read task failed for server {serverId} ({serverName})")] + internal static partial void TransportCleanupReadTaskFailed(this ILogger logger, string serverId, string serverName, Exception exception); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport read task cleaned up for server {serverId} ({serverName})")] + internal static partial void TransportReadTaskCleanedUp(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Information, Message = "Transport cleaned up for server {serverId} ({serverName})")] + internal static partial void TransportCleanedUp(this ILogger logger, string serverId, string serverName); + + [LoggerMessage(Level = LogLevel.Error, Message = "JSON-RPC message start object token expected")] + internal static partial void JsonRpcMessageConverterExpectedStartObjectToken(this ILogger logger); + + [LoggerMessage(Level = LogLevel.Error, Message = "JSON-RPC message invalid version")] + internal static partial void JsonRpcMessageConverterInvalidJsonRpcVersion(this ILogger logger); + + [LoggerMessage(Level = LogLevel.Trace, Message = "JSON-RPC message deserializing error response: {rawText}")] + internal static partial void JsonRpcMessageConverterDeserializingErrorResponse(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Trace, Message = "JSON-RPC message deserializing response: {rawText}")] + internal static partial void JsonRpcMessageConverterDeserializingResponse(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Error, Message = "JSON-RPC message response must have result or error: {rawText}")] + internal static partial void JsonRpcMessageConverterResponseMustHaveResultOrError(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Trace, Message = "JSON-RPC message deserializing notification: {rawText}")] + internal static partial void JsonRpcMessageConverterDeserializingNotification(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Trace, Message = "JSON-RPC message deserializing request: {rawText}")] + internal static partial void JsonRpcMessageConverterDeserializingRequest(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Error, Message = "JSON-RPC message invalid format: {rawText}")] + internal static partial void JsonRpcMessageConverterInvalidMessageFormat(this ILogger logger, string rawText); + + [LoggerMessage(Level = LogLevel.Error, Message = "JSON-RPC message write unknown message type: {type}")] + internal static partial void JsonRpcMessageConverterWriteUnknownMessageType(this ILogger logger, string type); +} \ No newline at end of file diff --git a/src/mcpdotnet/Protocol/Transport/StdioTransport.cs b/src/mcpdotnet/Protocol/Transport/StdioTransport.cs index 963fbfa..8966428 100644 --- a/src/mcpdotnet/Protocol/Transport/StdioTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/StdioTransport.cs @@ -1,42 +1,12 @@ -// Protocol/Transport/StdioTransport.cs -namespace McpDotNet.Protocol.Transport; - -using System.Diagnostics; +using System.Diagnostics; using System.Text.Json; -using System.Threading; +using McpDotNet.Configuration; using McpDotNet.Protocol.Messages; using McpDotNet.Utils.Json; +using Microsoft.Extensions.Logging; +using McpDotNet.Logging; -/// -/// Represents configuration options for the stdio transport. -/// -public record StdioTransportOptions -{ - /// - /// The command to execute to start the server process. - /// - public required string Command { get; set; } - - /// - /// Arguments to pass to the server process. - /// - public string[]? Arguments { get; set; } = Array.Empty(); - - /// - /// The working directory for the server process. - /// - public string? WorkingDirectory { get; set; } - - /// - /// Environment variables to set for the server process. - /// - public Dictionary? EnvironmentVariables { get; set; } - - /// - /// The timeout to wait for the server to shut down gracefully. - /// - public TimeSpan ShutdownTimeout { get; init; } = TimeSpan.FromSeconds(5); -} +namespace McpDotNet.Protocol.Transport; /// /// Implements the MCP transport protocol over standard input/output streams. @@ -44,6 +14,8 @@ public record StdioTransportOptions public sealed class StdioTransport : TransportBase { private readonly StdioTransportOptions _options; + private readonly McpServerConfig _serverConfig; + private readonly ILogger _logger; private readonly JsonSerializerOptions _jsonOptions; private Process? _process; private Task? _readTask; @@ -54,10 +26,14 @@ public sealed class StdioTransport : TransportBase /// Initializes a new instance of the StdioTransport class. /// /// Configuration options for the transport. - public StdioTransport(StdioTransportOptions options) + /// The server configuration for the transport. + /// A logger factory for creating loggers. + public StdioTransport(StdioTransportOptions options, McpServerConfig serverConfig, ILoggerFactory loggerFactory) { _options = options; - _jsonOptions = new JsonSerializerOptions().ConfigureForMcp(); + _serverConfig = serverConfig; + _logger = loggerFactory.CreateLogger(); + _jsonOptions = new JsonSerializerOptions().ConfigureForMcp(loggerFactory); } /// @@ -65,11 +41,14 @@ public sealed class StdioTransport : TransportBase { if (IsConnected) { + _logger.TransportAlreadyConnected(_serverConfig.Id, _serverConfig.Name); throw new McpTransportException("Transport is already connected"); } try { + _logger.TransportConnecting(_serverConfig.Id, _serverConfig.Name); + _shutdownCts = new CancellationTokenSource(); var startInfo = new ProcessStartInfo @@ -99,33 +78,37 @@ public sealed class StdioTransport : TransportBase } } + _logger.CreateProcessForTransport(_serverConfig.Id, _serverConfig.Name, _options.Command, + string.Join(", ", startInfo.ArgumentList), string.Join(", ", startInfo.Environment.Select(kvp => kvp.Key + "=" + kvp.Value)), + startInfo.WorkingDirectory, _options.ShutdownTimeout.ToString()); + _process = new Process { StartInfo = startInfo }; // Set up error logging _process.ErrorDataReceived += (sender, args) => { - if (!string.IsNullOrEmpty(args.Data)) - { - Debug.WriteLine($"MCP Server Error: {args.Data}"); - } + _logger.TransportError(_serverConfig.Id, _serverConfig.Name, args.Data ?? "(no data)"); }; if (!_process.Start()) { - Debug.WriteLine("Failed to start MCP server process"); + _logger.TransportProcessStartFailed(_serverConfig.Id, _serverConfig.Name); throw new McpTransportException("Failed to start MCP server process"); } + _logger.TransportProcessStarted(_serverConfig.Id, _serverConfig.Name, _process.Id); _processStarted = true; _process.BeginErrorReadLine(); // Start reading messages in the background _readTask = Task.Run(async () => await ReadMessagesAsync(_shutdownCts.Token)); + _logger.TransportReadingMessages(_serverConfig.Id, _serverConfig.Name); SetConnected(true); } catch (Exception ex) { + _logger.TransportConnectFailed(_serverConfig.Id, _serverConfig.Name, ex); await CleanupAsync(cancellationToken); throw new McpTransportException("Failed to connect transport", ex); } @@ -136,20 +119,30 @@ public sealed class StdioTransport : TransportBase { if (!IsConnected || _process?.HasExited == true) { + _logger.TransportNotConnected(_serverConfig.Id, _serverConfig.Name); throw new McpTransportException("Transport is not connected"); } + string id = "(no id)"; + if (message is IJsonRpcMessageWithId messageWithId) + { + id = messageWithId.Id.ToString(); + } + try { var json = JsonSerializer.Serialize(message, _jsonOptions); - Debug.WriteLine($"Sending message: {json}"); + _logger.TransportSendingMessage(_serverConfig.Id, _serverConfig.Name, id, json); // Write the message followed by a newline await _process!.StandardInput.WriteLineAsync(json.AsMemory(), cancellationToken); await _process.StandardInput.FlushAsync(); + + _logger.TransportSentMessage(_serverConfig.Id, _serverConfig.Name, id); } catch (Exception ex) { + _logger.TransportSendFailed(_serverConfig.Id, _serverConfig.Name, id, ex); throw new McpTransportException("Failed to send message", ex); } } @@ -165,14 +158,17 @@ public sealed class StdioTransport : TransportBase { try { + _logger.TransportEnteringReadMessagesLoop(_serverConfig.Id, _serverConfig.Name); + using var reader = _process!.StandardOutput; while (!cancellationToken.IsCancellationRequested && !_process.HasExited) { - Debug.WriteLine("Reading message..."); + _logger.TransportWaitingForMessage(_serverConfig.Id, _serverConfig.Name); var line = await reader.ReadLineAsync(cancellationToken); if (line == null) { + _logger.TransportEndOfStream(_serverConfig.Id, _serverConfig.Name); break; } @@ -181,61 +177,73 @@ public sealed class StdioTransport : TransportBase continue; } - Debug.WriteLine($"Received stdio message: {line}"); + _logger.TransportReceivedMessage(_serverConfig.Id, _serverConfig.Name, line); try { var message = JsonSerializer.Deserialize(line, _jsonOptions); if (message != null) { - + string messageId = "(no id)"; + if (message is IJsonRpcMessageWithId messageWithId) + { + messageId = messageWithId.Id.ToString(); + } + _logger.TransportReceivedMessageParsed(_serverConfig.Id, _serverConfig.Name, messageId); await WriteMessageAsync(message, cancellationToken); + _logger.TransportMessageWritten(_serverConfig.Id, _serverConfig.Name, messageId); + } + else + { + _logger.TransportMessageParseUnexpectedType(_serverConfig.Id, _serverConfig.Name, line); } } catch (JsonException ex) { - Debug.WriteLine($"Failed to parse message: {ex.Message}"); + _logger.TransportMessageParseFailed(_serverConfig.Id, _serverConfig.Name, line, ex); // Continue reading even if we fail to parse a message } } - Debug.WriteLine("Exiting read loop"); + _logger.TransportExitingReadMessagesLoop(_serverConfig.Id, _serverConfig.Name); } catch (OperationCanceledException) { + _logger.TransportReadMessagesCancelled(_serverConfig.Id, _serverConfig.Name); // Normal shutdown } catch (Exception ex) { - Debug.WriteLine($"Error reading messages: {ex}"); + _logger.TransportReadMessagesFailed(_serverConfig.Id, _serverConfig.Name, ex); } finally { - Debug.WriteLine("Entering ReadMessages cleanup"); await CleanupAsync(cancellationToken); - Debug.WriteLine("Completed ReadMessages cleanup"); } } private async Task CleanupAsync(CancellationToken cancellationToken) { + _logger.TransportCleaningUp(_serverConfig.Id, _serverConfig.Name); if (_process != null && _processStarted && !_process.HasExited) - { - + { try { // Try to close stdin to signal the process to exit + _logger.TransportClosingStdin(_serverConfig.Id, _serverConfig.Name); _process.StandardInput.Close(); // Wait for the process to exit + _logger.TransportWaitingForShutdown(_serverConfig.Id, _serverConfig.Name); if (!_process.WaitForExit((int)_options.ShutdownTimeout.TotalMilliseconds)) { // If it doesn't exit gracefully, terminate it + _logger.TransportKillingProcess(_serverConfig.Id, _serverConfig.Name); _process.Kill(true); } } catch (Exception ex) { - Debug.WriteLine($"Error during cleanup: {ex}"); + _logger.TransportShutdownFailed(_serverConfig.Id, _serverConfig.Name, ex); } _process.Dispose(); @@ -250,25 +258,29 @@ public sealed class StdioTransport : TransportBase { try { + _logger.TransportWaitingForReadTask(_serverConfig.Id, _serverConfig.Name); await _readTask.WaitAsync(TimeSpan.FromSeconds(5), cancellationToken); } catch (TimeoutException) { - Debug.WriteLine("Cleanup timeout waiting for read task"); + _logger.TransportCleanupReadTaskTimeout(_serverConfig.Id, _serverConfig.Name); // Continue with cleanup } catch (OperationCanceledException) { + _logger.TransportCleanupReadTaskCancelled(_serverConfig.Id, _serverConfig.Name); // Ignore cancellation } catch (Exception ex) { - Debug.WriteLine($"Error waiting for read task: {ex}"); + _logger.TransportCleanupReadTaskFailed(_serverConfig.Id, _serverConfig.Name, ex); } _readTask = null; + _logger.TransportReadTaskCleanedUp(_serverConfig.Id, _serverConfig.Name); } SetConnected(false); + _logger.TransportCleanedUp(_serverConfig.Id, _serverConfig.Name); } } \ No newline at end of file diff --git a/src/mcpdotnet/Protocol/Transport/StdioTransportOptions.cs b/src/mcpdotnet/Protocol/Transport/StdioTransportOptions.cs new file mode 100644 index 0000000..220131f --- /dev/null +++ b/src/mcpdotnet/Protocol/Transport/StdioTransportOptions.cs @@ -0,0 +1,33 @@ +// Protocol/Transport/StdioTransport.cs +namespace McpDotNet.Protocol.Transport; + +/// +/// Represents configuration options for the stdio transport. +/// +public record StdioTransportOptions +{ + /// + /// The command to execute to start the server process. + /// + public required string Command { get; set; } + + /// + /// Arguments to pass to the server process. + /// + public string[]? Arguments { get; set; } = Array.Empty(); + + /// + /// The working directory for the server process. + /// + public string? WorkingDirectory { get; set; } + + /// + /// Environment variables to set for the server process. + /// + public Dictionary? EnvironmentVariables { get; set; } + + /// + /// The timeout to wait for the server to shut down gracefully. + /// + public TimeSpan ShutdownTimeout { get; init; } = TimeSpan.FromSeconds(5); +} diff --git a/src/mcpdotnet/Protocol/Types/ContextInclusion.cs b/src/mcpdotnet/Protocol/Types/ContextInclusion.cs index 0aa1996..013f4f4 100644 --- a/src/mcpdotnet/Protocol/Types/ContextInclusion.cs +++ b/src/mcpdotnet/Protocol/Types/ContextInclusion.cs @@ -5,7 +5,18 @@ /// public enum ContextInclusion { + /// + /// No context should be included. + /// None, + + /// + /// Include context from the server that sent the request. + /// ThisServer, + + /// + /// Include context from all servers that the client is connected to. + /// AllServers } diff --git a/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs b/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs index dab374a..930d8ae 100644 --- a/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs +++ b/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs @@ -1,6 +1,8 @@ using System.Text.Json; using System.Text.Json.Serialization; using McpDotNet.Protocol.Messages; +using Microsoft.Extensions.Logging; +using McpDotNet.Logging; namespace McpDotNet.Utils.Json; @@ -9,11 +11,19 @@ namespace McpDotNet.Utils.Json; /// internal class JsonRpcMessageConverter : JsonConverter { + private readonly ILogger _logger; + + public JsonRpcMessageConverter(ILogger logger) + { + _logger = logger; + } + /// public override IJsonRpcMessage? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TokenType != JsonTokenType.StartObject) { + _logger.JsonRpcMessageConverterExpectedStartObjectToken(); throw new JsonException("Expected StartObject token"); } @@ -24,6 +34,7 @@ internal class JsonRpcMessageConverter : JsonConverter if (!root.TryGetProperty("jsonrpc", out var versionProperty) || versionProperty.GetString() != "2.0") { + _logger.JsonRpcMessageConverterInvalidJsonRpcVersion(); throw new JsonException("Invalid or missing jsonrpc version"); } @@ -40,26 +51,32 @@ internal class JsonRpcMessageConverter : JsonConverter // Messages with an error property are error responses if (hasError) { + _logger.JsonRpcMessageConverterDeserializingErrorResponse(rawText); return JsonSerializer.Deserialize(rawText, options); } // Messages with a result property are success responses else if (root.TryGetProperty("result", out _)) { + _logger.JsonRpcMessageConverterDeserializingResponse(rawText); return JsonSerializer.Deserialize(rawText, options); } + _logger.JsonRpcMessageConverterResponseMustHaveResultOrError(rawText); throw new JsonException("Response must have either result or error"); } // Messages with a method but no id are notifications else if (hasMethod && !hasId) { + _logger.JsonRpcMessageConverterDeserializingNotification(rawText); return JsonSerializer.Deserialize(rawText, options); } // Messages with both method and id are requests else if (hasMethod && hasId) { + _logger.JsonRpcMessageConverterDeserializingRequest(rawText); return JsonSerializer.Deserialize(rawText, options); } + _logger.JsonRpcMessageConverterInvalidMessageFormat(rawText); throw new JsonException("Invalid JSON-RPC message format"); } @@ -81,6 +98,7 @@ internal class JsonRpcMessageConverter : JsonConverter JsonSerializer.Serialize(writer, error, options); break; default: + _logger.JsonRpcMessageConverterWriteUnknownMessageType(value.GetType().ToString()); throw new JsonException($"Unknown JSON-RPC message type: {value.GetType()}"); } } diff --git a/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs b/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs index 22bb679..1b24c6b 100644 --- a/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs +++ b/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs @@ -1,4 +1,5 @@ -using System.Text.Json; +using Microsoft.Extensions.Logging; +using System.Text.Json; using System.Text.Json.Serialization; namespace McpDotNet.Utils.Json; @@ -11,11 +12,12 @@ internal static class JsonSerializerOptionsExtensions /// Configures JsonSerializerOptions with MCP-specific settings and converters. /// /// The options to configure. + /// The logger factory to use for logging. /// The configured options. - public static JsonSerializerOptions ConfigureForMcp(this JsonSerializerOptions options) + public static JsonSerializerOptions ConfigureForMcp(this JsonSerializerOptions options, ILoggerFactory loggerFactory) { // Add custom converters - options.Converters.Add(new JsonRpcMessageConverter()); + options.Converters.Add(new JsonRpcMessageConverter(loggerFactory.CreateLogger())); options.Converters.Add(new JsonStringEnumConverter()); // Configure general options diff --git a/src/mcpdotnet/mcpdotnet.csproj b/src/mcpdotnet/mcpdotnet.csproj index 6d6f756..b1e726e 100644 --- a/src/mcpdotnet/mcpdotnet.csproj +++ b/src/mcpdotnet/mcpdotnet.csproj @@ -9,7 +9,7 @@ mcpdotnet - 0.3.0.2 + 0.4.0.1 PederHP .NET client library for the Model Context Protocol (MCP) https://github.com/PederHP/mcpdotnet @@ -29,5 +29,9 @@ + + + +