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

Remove InputOutputStreams and clean up StdioServerTransport ctors (#71)

Browse source 
- There doesn't appear to be a meaningful scenario for injecting different readers/writers than stdin/stdout, and having InputOutputStreams complicates the API.
- Logically folks talk about "input and output", "readers and writers", etc., and "stdio" is "standard input and output", so having input specified first is less likely to be a source of bugs.
- Logger should be optional, not required, to simplify the common and getting-started cases.
This commit is contained in:
Stephen Toub 2025-03-13 12:45:30 -04:00 • committed by GitHub
commit 9e92bc9ce0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 108 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

1
src/mcpdotnet/Configuration/McpServerServiceCollectionExtension.cs
View file

@ -39,7 +39,6 @@ public static class McpServerServiceCollectionExtension
services.AddSingleton<IMcpServerFactory, McpServerFactory>();
services.AddHostedService<McpServerHostedService>();
services.AddOptions();
services.AddSingleton(InputOutputStreams.Default);
services.AddSingleton<IMcpServer>(sp => sp.GetRequiredService<IMcpServerFactory>().CreateServer());

26
src/mcpdotnet/Protocol/Transport/InputOutputStreams.cs
View file

@ -1,26 +0,0 @@
namespace McpDotNet.Protocol.Transport;
/// <summary>
/// Container for the input and output streams.
/// </summary>
public class InputOutputStreams
{
/// <summary>
/// Gets the default input and output streams.
/// </summary>
public static readonly InputOutputStreams Default = new()
{
Output = Console.Out,
Input = Console.In
};
/// <summary>
/// Gets or sets the output stream.
/// </summary>
public required TextWriter Output { get; init; }
/// <summary>
/// Gets or sets the input stream.
/// </summary>
public required TextReader Input { get; init; }
}

110
src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs
View file

@ -1,4 +1,5 @@
using System.Text.Json;
using McpDotNet.Configuration;
using McpDotNet.Logging;
using McpDotNet.Protocol.Messages;
using McpDotNet.Server;
@ -6,10 +7,12 @@ using McpDotNet.Utils.Json;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
namespace McpDotNet.Protocol.Transport;
/// <summary>
/// Implements the MCP transport protocol over standard input/output streams.
/// Provides an implementation of the MCP transport protocol over standard input/output streams.
/// </summary>
public sealed class StdioServerTransport : TransportBase, IServerTransport
{
@ -24,47 +27,90 @@ public sealed class StdioServerTransport : TransportBase, IServerTransport
private string EndpointName => $"Server (stdio) ({_serverName})";
/// <summary>
/// Initializes a new instance of the StdioServerTransport class.
/// Initializes a new instance of the <see cref="StdioServerTransport"/> class, using
/// <see cref="Console.In"/> and <see cref="Console.Out"/> for input and output streams.
/// </summary>
/// <param name="serverOptions">The server options.</param>
/// <param name="loggerFactory">A logger factory for creating loggers.</param>
/// <param name="inputOutputStreams">Container for the input and output streams</param>
public StdioServerTransport(McpServerOptions serverOptions, ILoggerFactory? loggerFactory, InputOutputStreams inputOutputStreams)
: this(
serverOptions is not null ? serverOptions.ServerInfo.Name : throw new ArgumentNullException(nameof(serverOptions)),
loggerFactory,
inputOutputStreams is not null ? inputOutputStreams.Output : throw new ArgumentNullException(nameof(inputOutputStreams)),
inputOutputStreams.Input
)
/// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
/// <exception cref="ArgumentNullException"><paramref name="serverOptions"/> is <see langword="null"/> or contains a null name.</exception>
/// <remarks>
/// <para>
/// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
/// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
/// </para>
/// </remarks>
public StdioServerTransport(McpServerOptions serverOptions, ILoggerFactory? loggerFactory = null)
: this(GetServerName(serverOptions), loggerFactory)
{
}
/// <summary>
/// Initializes a new instance of the StdioServerTransport class.
/// Initializes a new instance of the <see cref="StdioServerTransport"/> class, using
/// <see cref="Console.In"/> and <see cref="Console.Out"/> for input and output streams.
/// </summary>
/// <param name="serverName">The name of the server.</param>
/// <param name="loggerFactory">A logger factory for creating loggers.</param>
public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory)
: this(serverName, loggerFactory, Console.Out, Console.In)
/// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
/// <exception cref="ArgumentNullException"><paramref name="serverName"/> is <see langword="null"/>.</exception>
/// <remarks>
/// <para>
/// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
/// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
/// </para>
/// </remarks>
public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory = null)
: this(serverName ?? throw new ArgumentNullException(nameof(serverName)), Console.In, Console.Out, loggerFactory)
{
}
/// <summary>
/// Initializes a new instance of the StdioServerTransport class.
/// Initializes a new instance of the <see cref="StdioServerTransport"/> class using the specified
/// <paramref name="input"/> and <paramref name="output"/> streams.
/// </summary>
/// <param name="serverOptions">The server options.</param>
/// <param name="input">The reader, from which all input is read.</param>
/// <param name="output">The writer, to which all output is written.</param>
/// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
/// <exception cref="ArgumentNullException"><paramref name="serverOptions"/> is <see langword="null"/> or contains a null name.</exception>
/// <exception cref="ArgumentNullException"><paramref name="input"/> is <see langword="null"/>.</exception>
/// <exception cref="ArgumentNullException"><paramref name="output"/> is <see langword="null"/>.</exception>
/// <remarks>
/// <para>
/// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
/// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
/// </para>
/// </remarks>
public StdioServerTransport(McpServerOptions serverOptions, TextReader input, TextWriter output, ILoggerFactory? loggerFactory = null)
: this(GetServerName(serverOptions), input, output, loggerFactory)
{
}
/// <summary>
/// Initializes a new instance of the <see cref="StdioServerTransport"/> class using the specified
/// <paramref name="input"/> and <paramref name="output"/> streams.
/// </summary>
/// <param name="serverName">The name of the server.</param>
/// <param name="loggerFactory">A logger factory for creating loggers.</param>
/// <param name="output">The output stream writer</param>
/// <param name="input">The input stream reader</param>
public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory, TextWriter output, TextReader input)
/// <param name="input">The reader, from which all input is read.</param>
/// <param name="output">The writer, to which all output is written.</param>
/// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
/// <exception cref="ArgumentNullException"><paramref name="serverName"/> is <see langword="null"/>.</exception>
/// <exception cref="ArgumentNullException"><paramref name="input"/> is <see langword="null"/>.</exception>
/// <exception cref="ArgumentNullException"><paramref name="output"/> is <see langword="null"/>.</exception>
/// <remarks>
/// <para>
/// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
/// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
/// </para>
/// </remarks>
public StdioServerTransport(string serverName, TextReader input, TextWriter output, ILoggerFactory? loggerFactory = null)
: base(loggerFactory)
{
_serverName = serverName;
_logger = loggerFactory is not null ? loggerFactory.CreateLogger<StdioClientTransport>() : NullLogger.Instance;
_jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions;
_serverName = serverName ?? throw new ArgumentNullException(nameof(serverName));
_input = input ?? throw new ArgumentNullException(nameof(input));
_output = output ?? throw new ArgumentNullException(nameof(output));
_logger = loggerFactory is not null ? loggerFactory.CreateLogger<StdioClientTransport>() : NullLogger.Instance;
_jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions;
}
/// <inheritdoc/>
@ -224,4 +270,20 @@ public sealed class StdioServerTransport : TransportBase, IServerTransport
SetConnected(false);
_logger.TransportCleanedUp(EndpointName);
}
/// <summary>Validates the <paramref name="serverOptions"/> and extracts from it the server name to use.</summary>
private static string GetServerName(McpServerOptions serverOptions)
{
if (serverOptions is null)
{
throw new ArgumentNullException(nameof(serverOptions));
}
if (serverOptions.ServerInfo is null)
{
throw new ArgumentNullException($"{nameof(serverOptions)}.{nameof(serverOptions.ServerInfo)}");
}
return serverOptions.ServerInfo.Name;
}
}

38
tests/mcpdotnet.Tests/Transport/StdioServerTransportTests.cs
View file

@ -31,7 +31,7 @@ public class StdioServerTransportTests
public async Task Constructor_Should_Initialize_With_Valid_Parameters()
{
// Act
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
await using var transport = new StdioServerTransport(_serverOptions);
// Assert
Assert.NotNull(transport);
@ -40,21 +40,27 @@ public class StdioServerTransportTests
[Fact]
public void Constructor_Throws_For_Null_Options()
{
var exception = Assert.Throws<ArgumentNullException>(() => new StdioServerTransport(serverOptions: null!, NullLoggerFactory.Instance, InputOutputStreams.Default));
Assert.Equal("serverOptions", exception.ParamName);
Assert.Throws<ArgumentNullException>("serverName", () => new StdioServerTransport((string)null!));
Assert.Throws<ArgumentNullException>("serverOptions", () => new StdioServerTransport((McpServerOptions)null!));
Assert.Throws<ArgumentNullException>("serverOptions.ServerInfo", () => new StdioServerTransport(new McpServerOptions() { ServerInfo = null! }));
Assert.Throws<ArgumentNullException>("serverName", () => new StdioServerTransport(new McpServerOptions() { ServerInfo = new() { Name = null!, Version = "" } }));
}
[Fact]
public void Constructor_Throws_For_Null_Streams()
{
var exception = Assert.Throws<ArgumentNullException>(() => new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, null!));
Assert.Equal("inputOutputStreams", exception.ParamName);
Assert.Throws<ArgumentNullException>("input", () => new StdioServerTransport(_serverOptions, null!, Console.Out));
Assert.Throws<ArgumentNullException>("input", () => new StdioServerTransport("name", null!, Console.Out));
Assert.Throws<ArgumentNullException>("output", () => new StdioServerTransport("name", Console.In, null!));
Assert.Throws<ArgumentNullException>("output", () => new StdioServerTransport(_serverOptions, Console.In, null!));
}
[Fact]
public async Task StartListeningAsync_Should_Set_Connected_State()
{
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
await using var transport = new StdioServerTransport(_serverOptions);
await transport.StartListeningAsync();
@ -64,8 +70,10 @@ public class StdioServerTransportTests
[Fact]
public async Task SendMessageAsync_Should_Send_Message()
{
var streams = new InputOutputStreams { Output = new StringWriter(), Input = new StringReader("") };
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, streams);
var input = new StringReader("");
var output = new StringWriter();
await using var transport = new StdioServerTransport(_serverOptions, input, output, NullLoggerFactory.Instance);
await transport.StartListeningAsync();
var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
@ -73,7 +81,7 @@ public class StdioServerTransportTests
await transport.SendMessageAsync(message);
var result = streams.Output.ToString()?.Trim();
var result = output.ToString()?.Trim();
var expected = JsonSerializer.Serialize(message, JsonSerializerOptionsExtensions.DefaultOptions);
Assert.Equal(expected, result);
@ -82,7 +90,7 @@ public class StdioServerTransportTests
[Fact]
public async Task SendMessageAsync_Throws_Exception_If_Not_Connected()
{
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
await using var transport = new StdioServerTransport(_serverOptions);
var message = new JsonRpcRequest { Method = "test" };
@ -92,7 +100,7 @@ public class StdioServerTransportTests
[Fact]
public async Task DisposeAsync_Should_Dispose_Resources()
{
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
await using var transport = new StdioServerTransport(_serverOptions);
await transport.DisposeAsync();
@ -105,10 +113,10 @@ public class StdioServerTransportTests
var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
var json = JsonSerializer.Serialize(message, JsonSerializerOptionsExtensions.DefaultOptions);
using var sr = new StringReader(json);
var input = new StringReader(json);
var output = new StringWriter();
var streams = new InputOutputStreams { Output = new StringWriter(), Input = new StringReader(json) };
await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, streams);
await using var transport = new StdioServerTransport(_serverOptions, input, output);
await transport.StartListeningAsync();
var canRead = await transport.MessageReader.WaitToReadAsync();
@ -123,7 +131,7 @@ public class StdioServerTransportTests
[Fact]
public async Task CleanupAsync_Should_Cleanup_Resources()
{
var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
var transport = new StdioServerTransport(_serverOptions);
await transport.StartListeningAsync();
await transport.DisposeAsync();