modelcontextprotocol/csharp-sdk
Watch
1
0
Fork
You've already forked csharp-sdk
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Fix some code smells (#39)

Browse source 
* Fix some code smells

* Fix build

* Fix come issues

* Revert some changes
This commit is contained in:
Toni Wenzel 2025-03-10 18:03:47 +01:00 • committed by GitHub
commit 709b2e0a74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 270 additions and 244 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/McpDotNet.Extensions.AI/McpSessionScope.cs
View file

@ -35,7 +35,7 @@ public sealed class McpSessionScope : IAsyncDisposable
/// <param name="options">An options object with the client name and capabilities. Passed to the server.</param>
/// <param name="loggerFactory">A logger factory for mcpdotnet.</param>
/// <returns>A session scope which will keep the connection alive until disposed.</returns>
public static async Task<McpSessionScope> CreateAsync(McpServerConfig serverConfig,
public static async Task<McpSessionScope> CreateAsync(McpServerConfig serverConfig,
McpClientOptions? options = null,
ILoggerFactory? loggerFactory = null)
{
@ -62,7 +62,7 @@ public sealed class McpSessionScope : IAsyncDisposable
/// <param name="options">An options object with the client name and capabilities. Passed to the servers.</param>
/// <param name="loggerFactory">A logger factory for mcpdotnet.</param>
/// <returns>A session scope which keep the connections alive until disposed.</returns>
public static async Task<McpSessionScope> CreateAsync(IEnumerable<McpServerConfig> serverConfigs,
public static async Task<McpSessionScope> CreateAsync(IEnumerable<McpServerConfig> serverConfigs,
McpClientOptions? options = null,
ILoggerFactory? loggerFactory = null)
{
@ -106,5 +106,7 @@ public sealed class McpSessionScope : IAsyncDisposable
}
_clients.Clear();
Tools = [];
GC.SuppressFinalize(this);
}
}

16
src/mcpdotnet/Client/McpClientFactory.cs
View file

@ -1,8 +1,8 @@
using McpDotNet.Configuration;
using System.Runtime.InteropServices;
using McpDotNet.Configuration;
using McpDotNet.Logging;
using McpDotNet.Protocol.Transport;
using Microsoft.Extensions.Logging;
using System.Runtime.InteropServices;
namespace McpDotNet.Client;
/// <summary>
@ -13,6 +13,8 @@ namespace McpDotNet.Client;
/// </summary>
public class McpClientFactory
{
private const string ARGUMENTS_OPTIONS_KEY = "arguments";
private const string COMMAND_OPTIONS_KEY = "command";
private readonly Dictionary<string, McpServerConfig> _serverConfigs;
private readonly McpClientOptions _clientOptions;
private readonly Dictionary<string, IMcpClient> _clients = [];
@ -103,7 +105,7 @@ public class McpClientFactory
return new StdioClientTransport(new StdioClientTransportOptions
{
Command = GetCommand(config),
Arguments = config.TransportOptions?.GetValueOrDefault("arguments")?.Split(' '),
Arguments = config.TransportOptions?.GetValueOrDefault(ARGUMENTS_OPTIONS_KEY)?.Split(' '),
WorkingDirectory = config.TransportOptions?.GetValueOrDefault("workingDirectory"),
EnvironmentVariables = config.TransportOptions?
.Where(kv => kv.Key.StartsWith("env:", StringComparison.Ordinal))
@ -142,7 +144,7 @@ public class McpClientFactory
private static string GetCommand(McpServerConfig config)
{
var command = config.TransportOptions?.GetValueOrDefault("command");
var command = config.TransportOptions?.GetValueOrDefault(COMMAND_OPTIONS_KEY);
if (string.IsNullOrEmpty(command))
return config.Location!;
@ -158,7 +160,7 @@ public class McpClientFactory
string endpointName = $"Client ({config.Id}: {config.Name})";
// If the command is empty or already contains cmd.exe, we don't need to do anything
var command = config.TransportOptions?.GetValueOrDefault("command");
var command = config.TransportOptions?.GetValueOrDefault(COMMAND_OPTIONS_KEY);
if (string.IsNullOrEmpty(command) || !RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
@ -174,8 +176,8 @@ public class McpClientFactory
// If the command is not empty and does not contain cmd.exe, we need to inject /c {command} (usually npx or uvicorn)
// This is because the stdio transport will not work correctly if the command is not run in a shell
_logger.PromotingCommandToShellArgumentForStdio(endpointName, command, config.TransportOptions!.GetValueOrDefault("arguments") ?? "");
config.TransportOptions!["arguments"] = config.TransportOptions.TryGetValue("arguments", out var args)
_logger.PromotingCommandToShellArgumentForStdio(endpointName, command, config.TransportOptions!.GetValueOrDefault(ARGUMENTS_OPTIONS_KEY) ?? "");
config.TransportOptions![ARGUMENTS_OPTIONS_KEY] = config.TransportOptions.TryGetValue(ARGUMENTS_OPTIONS_KEY, out var args)
? $"/c {command} {args}"
: $"/c {command}";
}

7
src/mcpdotnet/Logging/Log.cs
View file

@ -11,8 +11,8 @@ 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.Information, Message = "Server {endpointName} capabilities received: {capabilities}, server info: {serverInfo}")]
internal static partial void ServerCapabilitiesReceived(this ILogger logger, string endpointName, string capabilities, string serverInfo);
[LoggerMessage(Level = LogLevel.Warning, Message = "Request {requestId} timed out")]
internal static partial void RequestTimeout(this ILogger logger, int requestId);
@ -74,9 +74,6 @@ internal static partial class Log
[LoggerMessage(Level = LogLevel.Error, Message = "Client server {endpointName} initialization error")]
internal static partial void ClientInitializationError(this ILogger logger, string endpointName, Exception exception);
[LoggerMessage(Level = LogLevel.Information, Message = "Server {endpointName} capabilities received: {capabilities}, server info: {serverInfo}")]
internal static partial void ServerCapabilitiesReceived(this ILogger logger, string endpointName, string capabilities, string serverInfo);
[LoggerMessage(Level = LogLevel.Error, Message = "Server {endpointName} protocol version mismatch, expected {expected}, received {received}")]
internal static partial void ServerProtocolVersionMismatch(this ILogger logger, string endpointName, string expected, string received);

2
src/mcpdotnet/Protocol/Transport/SseClientTransport.cs
View file

@ -12,7 +12,7 @@ using Microsoft.Extensions.Logging.Abstractions;
namespace McpDotNet.Protocol.Transport;
/// <summary>
///
/// The ServerSideEvents client transport implementation
/// </summary>
public sealed class SseClientTransport : TransportBase, IClientTransport
{

63
src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs
View file

@ -1,12 +1,12 @@
using McpDotNet.Configuration;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text;
using System.Text.Json;
using McpDotNet.Configuration;
using McpDotNet.Logging;
using McpDotNet.Protocol.Messages;
using McpDotNet.Utils.Json;
using Microsoft.Extensions.Logging;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text;
using System.Text.Json;
namespace McpDotNet.Protocol.Transport;
@ -186,30 +186,7 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
_logger.TransportReceivedMessage(EndpointName, line);
try
{
var message = JsonSerializer.Deserialize<IJsonRpcMessage>(line, _jsonOptions);
if (message != null)
{
string messageId = "(no id)";
if (message is IJsonRpcMessageWithId messageWithId)
{
messageId = messageWithId.Id.ToString();
}
_logger.TransportReceivedMessageParsed(EndpointName, messageId);
await WriteMessageAsync(message, cancellationToken).ConfigureAwait(false);
_logger.TransportMessageWritten(EndpointName, messageId);
}
else
{
_logger.TransportMessageParseUnexpectedType(EndpointName, line);
}
}
catch (JsonException ex)
{
_logger.TransportMessageParseFailed(EndpointName, line, ex);
// Continue reading even if we fail to parse a message
}
await ProcessMessageAsync(line, cancellationToken).ConfigureAwait(false);
}
_logger.TransportExitingReadMessagesLoop(EndpointName);
}
@ -228,6 +205,34 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
}
}
private async Task ProcessMessageAsync(string line, CancellationToken cancellationToken)
{
try
{
var message = JsonSerializer.Deserialize<IJsonRpcMessage>(line, _jsonOptions);
if (message != null)
{
string messageId = "(no id)";
if (message is IJsonRpcMessageWithId messageWithId)
{
messageId = messageWithId.Id.ToString();
}
_logger.TransportReceivedMessageParsed(EndpointName, messageId);
await WriteMessageAsync(message, cancellationToken).ConfigureAwait(false);
_logger.TransportMessageWritten(EndpointName, messageId);
}
else
{
_logger.TransportMessageParseUnexpectedType(EndpointName, line);
}
}
catch (JsonException ex)
{
_logger.TransportMessageParseFailed(EndpointName, line, ex);
// Continue reading even if we fail to parse a message
}
}
private async Task CleanupAsync(CancellationToken cancellationToken)
{
_logger.TransportCleaningUp(EndpointName);

155
src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs
View file

@ -102,77 +102,90 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
switch (message)
{
case JsonRpcRequest request:
if (_requestHandlers.TryGetValue(request.Method, out var handler))
{
try
{
_logger.RequestHandlerCalled(EndpointName, request.Method);
var result = await handler(request).ConfigureAwait(false);
_logger.RequestHandlerCompleted(EndpointName, request.Method);
await _transport.SendMessageAsync(new JsonRpcResponse
{
Id = request.Id,
JsonRpc = "2.0",
Result = result
}, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.RequestHandlerError(EndpointName, request.Method, ex);
// Send error response
await _transport.SendMessageAsync(new JsonRpcError
{
Id = request.Id,
JsonRpc = "2.0",
Error = new JsonRpcErrorDetail
{
Code = -32000, // Implementation defined error
Message = ex.Message
}
}, cancellationToken).ConfigureAwait(false);
}
}
else
{
_logger.NoHandlerFoundForRequest(EndpointName, request.Method);
}
await HandleRequest(request, cancellationToken).ConfigureAwait(false);
break;
case IJsonRpcMessageWithId messageWithId:
if (_pendingRequests.TryRemove(messageWithId.Id, out var tcs))
{
_logger.ResponseMatchedPendingRequest(EndpointName, messageWithId.Id.ToString());
tcs.TrySetResult(message);
}
else
{
_logger.NoRequestFoundForMessageWithId(EndpointName, messageWithId.Id.ToString());
}
HandleMessageWithId(message, messageWithId);
break;
case JsonRpcNotification notification:
if (_notificationHandlers.TryGetValue(notification.Method, out var handlers))
{
foreach (var notificationHandler in handlers)
{
try
{
await notificationHandler(notification).ConfigureAwait(false);
}
catch (Exception ex)
{
// Log handler error but continue processing
_logger.NotificationHandlerError(EndpointName, notification.Method, ex);
}
}
}
await HandleNotification(notification).ConfigureAwait(false);
break;
default:
_logger.EndpointHandlerUnexpectedMessageType(EndpointName, message.GetType().Name);
break;
}
}
private async Task HandleNotification(JsonRpcNotification notification)
{
if (_notificationHandlers.TryGetValue(notification.Method, out var handlers))
{
foreach (var notificationHandler in handlers)
{
try
{
await notificationHandler(notification).ConfigureAwait(false);
}
catch (Exception ex)
{
// Log handler error but continue processing
_logger.NotificationHandlerError(EndpointName, notification.Method, ex);
}
}
}
}
private void HandleMessageWithId(IJsonRpcMessage message, IJsonRpcMessageWithId messageWithId)
{
if (_pendingRequests.TryRemove(messageWithId.Id, out var tcs))
{
_logger.ResponseMatchedPendingRequest(EndpointName, messageWithId.Id.ToString());
tcs.TrySetResult(message);
}
else
{
_logger.NoRequestFoundForMessageWithId(EndpointName, messageWithId.Id.ToString());
}
}
private async Task HandleRequest(JsonRpcRequest request, CancellationToken cancellationToken)
{
if (_requestHandlers.TryGetValue(request.Method, out var handler))
{
try
{
_logger.RequestHandlerCalled(EndpointName, request.Method);
var result = await handler(request).ConfigureAwait(false);
_logger.RequestHandlerCompleted(EndpointName, request.Method);
await _transport.SendMessageAsync(new JsonRpcResponse
{
Id = request.Id,
JsonRpc = "2.0",
Result = result
}, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.RequestHandlerError(EndpointName, request.Method, ex);
// Send error response
await _transport.SendMessageAsync(new JsonRpcError
{
Id = request.Id,
JsonRpc = "2.0",
Error = new JsonRpcErrorDetail
{
Code = -32000, // Implementation defined error
Message = ex.Message
}
}, cancellationToken).ConfigureAwait(false);
}
}
else
{
_logger.NoHandlerFoundForRequest(EndpointName, request.Method);
}
}
/// <summary>
/// Sends a generic JSON-RPC request to the server.
/// It is strongly recommended use the capability-specific methods instead of this one.
@ -257,7 +270,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
{
if (!_transport.IsConnected)
{
// TODO: _logger.ClientNotConnected(_serverConfig.Id, _serverConfig.Name);
_logger.ClientNotConnected(EndpointName);
throw new McpClientException("Transport is not connected");
}
@ -265,13 +278,10 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
// Log if enabled
if (_logger.IsEnabled(LogLevel.Debug))
{
// TODO: _logger.SendingNotificationPayload(_serverConfig.Id, _serverConfig.Name, JsonSerializer.Serialize(notification));
}
_logger.SendingNotificationPayload(EndpointName, JsonSerializer.Serialize(notification));
// Log basic info
// TODO: _logger.SendingNotification(_serverConfig.Id, _serverConfig.Name, method);
_logger.SendingNotification(EndpointName, method);
await _transport.SendMessageAsync(notification, cancellationToken).ConfigureAwait(false);
}
@ -285,21 +295,22 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
{
if (!_transport.IsConnected)
{
// TODO: _logger.ClientNotConnected(_serverConfig.Id, _serverConfig.Name);
_logger.ClientNotConnected(EndpointName);
throw new McpClientException("Transport is not connected");
}
var notification = new JsonRpcNotification
{
Method = method,
Params = parameters
};
// Log if enabled
if (_logger.IsEnabled(LogLevel.Debug))
{
// TODO: _logger.SendingNotificationPayload(_serverConfig.Id, _serverConfig.Name, JsonSerializer.Serialize(notification));
}
_logger.SendingNotificationPayload(EndpointName, JsonSerializer.Serialize(notification));
// Log basic info
// TODO: _logger.SendingNotification(_serverConfig.Id, _serverConfig.Name, method);
_logger.SendingNotification(EndpointName, method);
await _transport.SendMessageAsync(notification, cancellationToken).ConfigureAwait(false);
}
@ -312,7 +323,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
{
if (!_transport.IsConnected)
{
// TODO: _logger.ClientNotConnected(_serverConfig.Id, _serverConfig.Name);
_logger.ClientNotConnected(EndpointName);
throw new McpClientException("Transport is not connected");
}

11
src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs
View file

@ -1,8 +1,6 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using McpDotNet.Protocol.Messages;
using Microsoft.Extensions.Logging;
using McpDotNet.Logging;
namespace McpDotNet.Utils.Json;
@ -44,21 +42,24 @@ internal sealed class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
{
return JsonSerializer.Deserialize<JsonRpcError>(rawText, options);
}
// Messages with a result property are success responses
else if (root.TryGetProperty("result", out _))
if (root.TryGetProperty("result", out _))
{
return JsonSerializer.Deserialize<JsonRpcResponse>(rawText, options);
}
throw new JsonException("Response must have either result or error");
}
// Messages with a method but no id are notifications
else if (hasMethod && !hasId)
if (hasMethod && !hasId)
{
return JsonSerializer.Deserialize<JsonRpcNotification>(rawText, options);
}
// Messages with both method and id are requests
else if (hasMethod && hasId)
if (hasMethod && hasId)
{
return JsonSerializer.Deserialize<JsonRpcRequest>(rawText, options);
}