From 729c8b5eb95c6912bc26fedd6d2fcf9d4c04fa78 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 13 Mar 2025 18:16:21 -0400 Subject: [PATCH] Make logging optional in all public APIs (#72) --- src/mcpdotnet/Client/McpClientFactory.cs | 15 ++++++++++++++- .../Protocol/Transport/StdioClientTransport.cs | 17 ++++++++++++++--- src/mcpdotnet/Server/McpServerFactory.cs | 10 ++++++++-- .../Server/McpServerFactoryTests.cs | 7 ------- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/mcpdotnet/Client/McpClientFactory.cs b/src/mcpdotnet/Client/McpClientFactory.cs index c4247b1..fa1bc26 100644 --- a/src/mcpdotnet/Client/McpClientFactory.cs +++ b/src/mcpdotnet/Client/McpClientFactory.cs @@ -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; /// @@ -43,10 +44,22 @@ public class McpClientFactory : IDisposable public McpClientFactory( IEnumerable serverConfigs, McpClientOptions clientOptions, - ILoggerFactory loggerFactory, + ILoggerFactory? loggerFactory = null, Func? transportFactoryMethod = null, Func? 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; diff --git a/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs b/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs index c084960..cb705c2 100644 --- a/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs @@ -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 _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 /// Configuration options for the transport. /// The server configuration for the transport. /// A logger factory for creating loggers. - 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(); + _logger = (ILogger?)loggerFactory?.CreateLogger() ?? NullLogger.Instance; _jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions; } diff --git a/src/mcpdotnet/Server/McpServerFactory.cs b/src/mcpdotnet/Server/McpServerFactory.cs index 07146d5..26e1e45 100644 --- a/src/mcpdotnet/Server/McpServerFactory.cs +++ b/src/mcpdotnet/Server/McpServerFactory.cs @@ -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 /// Optional service provider to create new instances. /// Logger factory to use for logging /// - public McpServerFactory(IServerTransport serverTransport, McpServerOptions options, ILoggerFactory loggerFactory, IOptions? serverDelegates = null, IServiceProvider? serviceProvider = null) + public McpServerFactory( + IServerTransport serverTransport, + McpServerOptions options, + ILoggerFactory? loggerFactory = null, + IOptions? 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; } diff --git a/tests/mcpdotnet.Tests/Server/McpServerFactoryTests.cs b/tests/mcpdotnet.Tests/Server/McpServerFactoryTests.cs index ff567b3..59fc5e3 100644 --- a/tests/mcpdotnet.Tests/Server/McpServerFactoryTests.cs +++ b/tests/mcpdotnet.Tests/Server/McpServerFactoryTests.cs @@ -53,13 +53,6 @@ public class McpServerFactoryTests Assert.Throws(() => new McpServerFactory(_serverTransport.Object, null!, _loggerFactory.Object, _serverDelegates.Object, _serviceProvider)); } - [Fact] - public void Constructor_Throws_For_Null_Logger() - { - // Arrange, Act & Assert - Assert.Throws(() => new McpServerFactory(_serverTransport.Object, _options, null!, _serverDelegates.Object, _serviceProvider)); - } - [Fact] public void Constructor_Does_Not_Throw_For_Null_ServerDelegates() {