Make logging optional in all public APIs (#72)

This commit is contained in:
Stephen Toub 2025-03-13 18:16:21 -04:00 • committed by GitHub
commit 729c8b5eb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 36 additions and 13 deletions

View file

@ -4,6 +4,7 @@ using McpDotNet.Configuration;
using McpDotNet.Logging;
using McpDotNet.Protocol.Transport;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
namespace McpDotNet.Client;
/// <summary>
@ -43,10 +44,22 @@ public class McpClientFactory : IDisposable
public McpClientFactory(
IEnumerable<McpServerConfig> serverConfigs,
McpClientOptions clientOptions,
ILoggerFactory loggerFactory,
ILoggerFactory? loggerFactory = null,
Func<McpServerConfig, IClientTransport>? transportFactoryMethod = null,
Func<IClientTransport, McpServerConfig, McpClientOptions, IMcpClient>? clientFactoryMethod = null)
{
if (serverConfigs is null)
{
throw new ArgumentNullException(nameof(serverConfigs));
}
if (clientOptions is null)
{
throw new ArgumentNullException(nameof(clientOptions));
}
loggerFactory ??= NullLoggerFactory.Instance;
_serverConfigs = serverConfigs.ToDictionary(c => c.Id);
_clientOptions = clientOptions;
_loggerFactory = loggerFactory;

View file

@ -8,6 +8,7 @@ using McpDotNet.Protocol.Messages;
using McpDotNet.Utils;
using McpDotNet.Utils.Json;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
namespace McpDotNet.Protocol.Transport;
@ -18,7 +19,7 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
{
private readonly StdioClientTransportOptions _options;
private readonly McpServerConfig _serverConfig;
private readonly ILogger<StdioClientTransport> _logger;
private readonly ILogger _logger;
private readonly JsonSerializerOptions _jsonOptions;
private Process? _process;
private Task? _readTask;
@ -33,12 +34,22 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
/// <param name="options">Configuration options for the transport.</param>
/// <param name="serverConfig">The server configuration for the transport.</param>
/// <param name="loggerFactory">A logger factory for creating loggers.</param>
public StdioClientTransport(StdioClientTransportOptions options, McpServerConfig serverConfig, ILoggerFactory loggerFactory)
public StdioClientTransport(StdioClientTransportOptions options, McpServerConfig serverConfig, ILoggerFactory? loggerFactory = null)
: base(loggerFactory)
{
if (options is null)
{
throw new ArgumentNullException(nameof(options));
}
if (serverConfig is null)
{
throw new ArgumentNullException(nameof(serverConfig));
}
_options = options;
_serverConfig = serverConfig;
_logger = loggerFactory.CreateLogger<StdioClientTransport>();
_logger = (ILogger?)loggerFactory?.CreateLogger<StdioClientTransport>() ?? NullLogger.Instance;
_jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions;
}

View file

@ -1,5 +1,6 @@
using McpDotNet.Protocol.Transport;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
namespace McpDotNet.Server;
@ -31,11 +32,16 @@ public class McpServerFactory : IMcpServerFactory
/// <param name="serviceProvider">Optional service provider to create new instances.</param>
/// <param name="loggerFactory">Logger factory to use for logging</param>
/// <param name="serverDelegates"></param>
public McpServerFactory(IServerTransport serverTransport, McpServerOptions options, ILoggerFactory loggerFactory, IOptions<McpServerDelegates>? serverDelegates = null, IServiceProvider? serviceProvider = null)
public McpServerFactory(
IServerTransport serverTransport,
McpServerOptions options,
ILoggerFactory? loggerFactory = null,
IOptions<McpServerDelegates>? serverDelegates = null,
IServiceProvider? serviceProvider = null)
{
_serverTransport = serverTransport ?? throw new ArgumentNullException(nameof(serverTransport));
_options = options ?? throw new ArgumentNullException(nameof(options));
_loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory));
_loggerFactory = loggerFactory ?? NullLoggerFactory.Instance;
_serverDelegates = serverDelegates?.Value;
_serviceProvider = serviceProvider;
}

View file

@ -53,13 +53,6 @@ public class McpServerFactoryTests
Assert.Throws<ArgumentNullException>(() => new McpServerFactory(_serverTransport.Object, null!, _loggerFactory.Object, _serverDelegates.Object, _serviceProvider));
}
[Fact]
public void Constructor_Throws_For_Null_Logger()
{
// Arrange, Act & Assert
Assert.Throws<ArgumentNullException>(() => new McpServerFactory(_serverTransport.Object, _options, null!, _serverDelegates.Object, _serviceProvider));
}
[Fact]
public void Constructor_Does_Not_Throw_For_Null_ServerDelegates()
{