* Logging
Fixed bug in stdio transport argument promotion

* Logging
Fixed bug in stdio transport argument promotion

* Version bump
This commit is contained in:
Peder Holdgaard Pedersen 2025-01-09 19:39:11 +01:00 • committed by GitHub
commit 4b8df6638e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 674 additions and 298 deletions

View file

@ -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;
/// <inheritdoc/>
internal class McpClient : IMcpClient
@ -19,13 +20,16 @@ internal class McpClient : IMcpClient
private readonly ConcurrentDictionary<string, List<Func<JsonRpcNotification,Task>>> _notificationHandlers;
private readonly Dictionary<string, Func<JsonRpcRequest, Task<object>>> _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<McpClient> _logger;
private volatile bool _isInitializing;
/// <inheritdoc/>
public bool IsInitialized => _isInitialized;
public bool IsInitialized => _isInitialized;
/// <inheritdoc/>
public ServerCapabilities? ServerCapabilities { get; private set; }
@ -38,20 +42,29 @@ internal class McpClient : IMcpClient
/// </summary>
/// <param name="transport">An MCP transport implementation.</param>
/// <param name="options">Options for the client, defining protocol version and capabilities.</param>
public McpClient(IMcpTransport transport, McpClientOptions options)
/// <param name="serverConfig">The server configuration.</param>
/// <param name="loggerFactory">The logger factory.</param>
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<McpClient>();
if (options.Capabilities?.Sampling != null)
{
SetRequestHandler<CreateMessageRequestParams, CreateMessageResult>("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
/// <inheritdoc/>
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
/// <inheritdoc/>
public async Task PingAsync(CancellationToken cancellationToken)
{
_logger.PingingServer(_serverConfig.Id, _serverConfig.Name);
await SendRequestAsync<dynamic>(
new JsonRpcRequest
{
@ -151,6 +180,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task<ListToolsResult> ListToolsAsync(string? cursor = null, CancellationToken cancellationToken = default)
{
_logger.ListingTools(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)");
return await SendRequestAsync<ListToolsResult>(
new JsonRpcRequest
{
@ -164,6 +194,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task<ListPromptsResult> ListPromptsAsync(string? cursor = null, CancellationToken cancellationToken = default)
{
_logger.ListingPrompts(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)");
return await SendRequestAsync<ListPromptsResult>(
new JsonRpcRequest
{
@ -177,6 +208,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task<GetPromptResult> GetPromptAsync(string name, Dictionary<string, object>? arguments = null, CancellationToken cancellationToken = default)
{
_logger.GettingPrompt(_serverConfig.Id, _serverConfig.Name, name, arguments == null ? "{}" : JsonSerializer.Serialize(arguments));
return await SendRequestAsync<GetPromptResult>(
new JsonRpcRequest
{
@ -190,6 +222,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task<ListResourcesResult> ListResourcesAsync(string? cursor = null, CancellationToken cancellationToken = default)
{
_logger.ListingResources(_serverConfig.Id, _serverConfig.Name, cursor ?? "(null)");
return await SendRequestAsync<ListResourcesResult>(
new JsonRpcRequest
{
@ -203,6 +236,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task<ReadResourceResult> ReadResourceAsync(string uri, CancellationToken cancellationToken = default)
{
_logger.ReadingResource(_serverConfig.Id, _serverConfig.Name, uri);
return await SendRequestAsync<ReadResourceResult>(
new JsonRpcRequest
{
@ -220,6 +254,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task SubscribeToResourceAsync(string uri, CancellationToken cancellationToken = default)
{
_logger.SubscribingToResource(_serverConfig.Id, _serverConfig.Name, uri);
await SendRequestAsync<dynamic>(
new JsonRpcRequest
{
@ -237,6 +272,7 @@ internal class McpClient : IMcpClient
/// <inheritdoc/>
public async Task UnsubscribeFromResourceAsync(string uri, CancellationToken cancellationToken = default)
{
_logger.UnsubscribingFromResource(_serverConfig.Id, _serverConfig.Name, uri);
await SendRequestAsync<dynamic>(
new JsonRpcRequest
{
@ -254,6 +290,7 @@ internal class McpClient : IMcpClient
public async Task<CallToolResponse> CallToolAsync(string toolName, Dictionary<string, object>? arguments = null,
CancellationToken cancellationToken = default)
{
_logger.CallingTool(_serverConfig.Id, _serverConfig.Name, toolName, arguments == null ? "{}" : JsonSerializer.Serialize(arguments));
return await SendRequestAsync<CallToolResponse>(
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<T>(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<TRequest>(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);
}
/// <inheritdoc/>

View file

@ -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;
/// <summary>
/// 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<string, IMcpClient> _clients = new();
private readonly Func<McpServerConfig, IMcpTransport> _transportFactoryMethod;
private readonly Func<IMcpTransport, McpClientOptions, IMcpClient> _clientFactoryMethod;
private readonly Func<IMcpTransport, McpServerConfig, McpClientOptions, IMcpClient> _clientFactoryMethod;
private readonly ILoggerFactory _loggerFactory;
private readonly ILogger<McpClientFactory> _logger;
/// <summary>
/// Initializes a new instance of the <see cref="McpClientFactory"/> class.
@ -24,18 +27,33 @@ public class McpClientFactory
/// </summary>
/// <param name="serverConfigs">Configuration objects for each server the factory should support.</param>
/// <param name="clientOptions">A configuration object which specifies client capabilities and protocol version.</param>
/// <param name="loggerFactory">A logger factory for creating loggers for clients.</param>
/// <param name="transportFactoryMethod">An optional factory method which returns transport implementations based on a server configuration.</param>
/// <param name="clientFactoryMethod">An optional factory method which creates a client based on client options and transport implementation. </param>
public McpClientFactory(
IEnumerable<McpServerConfig> serverConfigs,
McpClientOptions clientOptions,
ILoggerFactory loggerFactory,
Func<McpServerConfig, IMcpTransport>? transportFactoryMethod = null,
Func<IMcpTransport, McpClientOptions, IMcpClient>? clientFactoryMethod = null)
Func<IMcpTransport, McpServerConfig, McpClientOptions, IMcpClient>? clientFactoryMethod = null)
{
_serverConfigs = serverConfigs.ToDictionary(c => c.Id);
_clientOptions = clientOptions;
_loggerFactory = loggerFactory;
_logger = loggerFactory.CreateLogger<McpClientFactory>();
_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);
}
}
}
/// <summary>
@ -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<string>());
_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";
}
/// <summary>
/// Initializes a non-shell command by injecting a /c {command} argument, as the command will be run in a shell.
/// </summary>
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}";
}
}
}