Make ClientTransportClosedException public and unify transport exception handling (#1467)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Co-authored-by: halter73 <54385+halter73@users.noreply.github.com> Co-authored-by: Stephen Toub <stoub@microsoft.com>
This commit is contained in:
parent
498de089fc
commit
7662ce6b70
2 changed files with 253 additions and 38 deletions
|
|
@ -4,6 +4,48 @@ namespace ModelContextProtocol.Tests.Client;
|
|||
|
||||
public class ClientCompletionDetailsTests
|
||||
{
|
||||
[Fact]
|
||||
public void ClientTransportClosedException_ExposesDetails()
|
||||
{
|
||||
var details = new StdioClientCompletionDetails
|
||||
{
|
||||
ExitCode = 42,
|
||||
ProcessId = 12345,
|
||||
StandardErrorTail = ["error line"],
|
||||
Exception = new IOException("process exited"),
|
||||
};
|
||||
|
||||
var exception = new ClientTransportClosedException(details);
|
||||
|
||||
Assert.IsType<StdioClientCompletionDetails>(exception.Details);
|
||||
var stdioDetails = (StdioClientCompletionDetails)exception.Details;
|
||||
Assert.Equal(42, stdioDetails.ExitCode);
|
||||
Assert.Equal(12345, stdioDetails.ProcessId);
|
||||
Assert.Equal(["error line"], stdioDetails.StandardErrorTail);
|
||||
Assert.Equal("process exited", exception.Message);
|
||||
Assert.IsType<IOException>(exception.InnerException);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ClientTransportClosedException_WithNullException_HasDefaultMessage()
|
||||
{
|
||||
var details = new ClientCompletionDetails();
|
||||
|
||||
var exception = new ClientTransportClosedException(details);
|
||||
|
||||
Assert.Equal("The transport was closed.", exception.Message);
|
||||
Assert.Null(exception.InnerException);
|
||||
Assert.Same(details, exception.Details);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ClientTransportClosedException_IsIOException()
|
||||
{
|
||||
var details = new ClientCompletionDetails();
|
||||
IOException exception = new ClientTransportClosedException(details);
|
||||
Assert.IsType<ClientTransportClosedException>(exception);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ClientCompletionDetails_PropertiesRoundtrip()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1,12 +1,14 @@
|
|||
using Microsoft.Extensions.Logging;
|
||||
using ModelContextProtocol.Client;
|
||||
using ModelContextProtocol.Protocol;
|
||||
using ModelContextProtocol.Tests.Utils;
|
||||
using System.IO.Pipelines;
|
||||
using System.Text.Json;
|
||||
using System.Threading.Channels;
|
||||
|
||||
namespace ModelContextProtocol.Tests.Client;
|
||||
|
||||
public class McpClientCreationTests
|
||||
public class McpClientCreationTests(ITestOutputHelper testOutputHelper) : LoggedTest(testOutputHelper)
|
||||
{
|
||||
[Fact]
|
||||
public async Task CreateAsync_WithInvalidArgs_Throws()
|
||||
|
|
@ -101,6 +103,51 @@ public class McpClientCreationTests
|
|||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateAsync_TransportChannelClosed_ThrowsClientTransportClosedException()
|
||||
{
|
||||
// Arrange - transport completes its read channel with ClientTransportClosedException
|
||||
// when the client tries to send the initialize request (simulating a server process
|
||||
// exit detected by the reader loop). SendMessageAsync returns successfully —
|
||||
// only the read side fails.
|
||||
var transport = new ChannelClosedDuringInitTransport();
|
||||
|
||||
// Act & Assert
|
||||
var ex = await Assert.ThrowsAsync<ClientTransportClosedException>(
|
||||
() => McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
|
||||
|
||||
var details = Assert.IsType<StdioClientCompletionDetails>(ex.Details);
|
||||
Assert.Equal(42, details.ExitCode);
|
||||
Assert.Equal(9999, details.ProcessId);
|
||||
Assert.NotNull(details.StandardErrorTail);
|
||||
Assert.Equal("Feature disabled", details.StandardErrorTail![0]);
|
||||
|
||||
// Verify initialization error was logged
|
||||
Assert.Contains(MockLoggerProvider.LogMessages, log =>
|
||||
log.LogLevel == LogLevel.Error &&
|
||||
log.Message.Contains("client initialization error"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateAsync_SendFails_PropagatesOriginalIOException()
|
||||
{
|
||||
// Arrange - transport throws IOException from SendMessageAsync, but the channel
|
||||
// is not completed with ClientTransportClosedException. The original IOException should
|
||||
// propagate without being wrapped in ClientTransportClosedException.
|
||||
var transport = new SendFailsDuringInitTransport();
|
||||
|
||||
// Act & Assert
|
||||
var ex = await Assert.ThrowsAsync<IOException>(
|
||||
() => McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
|
||||
|
||||
Assert.Equal(SendFailsDuringInitTransport.ExpectedMessage, ex.Message);
|
||||
|
||||
// Verify initialization error was logged
|
||||
Assert.Contains(MockLoggerProvider.LogMessages, log =>
|
||||
log.LogLevel == LogLevel.Error &&
|
||||
log.Message.Contains("client initialization error"));
|
||||
}
|
||||
|
||||
private class NopTransport : ITransport, IClientTransport
|
||||
{
|
||||
private readonly Channel<JsonRpcMessage> _channel = Channel.CreateUnbounded<JsonRpcMessage>();
|
||||
|
|
@ -155,4 +202,60 @@ public class McpClientCreationTests
|
|||
throw new InvalidOperationException(ExpectedMessage);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Simulates a transport where the read channel closes with structured completion details during
|
||||
/// initialization, as happens when a stdio server process exits before completing the handshake.
|
||||
/// The send succeeds — only the read side carries the failure.
|
||||
/// </summary>
|
||||
private sealed class ChannelClosedDuringInitTransport : ITransport, IClientTransport
|
||||
{
|
||||
private readonly Channel<JsonRpcMessage> _channel = Channel.CreateUnbounded<JsonRpcMessage>();
|
||||
|
||||
public bool IsConnected => true;
|
||||
public string? SessionId => null;
|
||||
|
||||
public ChannelReader<JsonRpcMessage> MessageReader => _channel.Reader;
|
||||
|
||||
public Task<ITransport> ConnectAsync(CancellationToken cancellationToken = default) => Task.FromResult<ITransport>(this);
|
||||
|
||||
public ValueTask DisposeAsync()
|
||||
{
|
||||
_channel.Writer.TryComplete();
|
||||
return default;
|
||||
}
|
||||
|
||||
public string Name => "Test ChannelClosed Transport";
|
||||
|
||||
public Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
|
||||
{
|
||||
// Simulate the server process exiting: complete the channel with a
|
||||
// ClientTransportClosedException carrying structured completion details.
|
||||
// The send itself succeeds — the failure comes from the read side.
|
||||
var details = new StdioClientCompletionDetails
|
||||
{
|
||||
ExitCode = 42,
|
||||
ProcessId = 9999,
|
||||
StandardErrorTail = ["Feature disabled"],
|
||||
Exception = new IOException("MCP server process exited unexpectedly (exit code: 42)"),
|
||||
};
|
||||
|
||||
_channel.Writer.TryComplete(new ClientTransportClosedException(details));
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Simulates a transport where SendMessageAsync throws IOException but the channel
|
||||
/// doesn't carry a ClientTransportClosedException (e.g., a write pipe break without structured details).
|
||||
/// </summary>
|
||||
private sealed class SendFailsDuringInitTransport : NopTransport
|
||||
{
|
||||
public const string ExpectedMessage = "Failed to write to transport";
|
||||
|
||||
public override Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
|
||||
{
|
||||
throw new IOException(ExpectedMessage);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue