Make TransportClosedException public and unify transport exception handling#1467
Make TransportClosedException public and unify transport exception handling#1467
Conversation
… through CreateAsync - Make TransportClosedException public with proper XML documentation - Update ProcessMessagesCoreAsync to propagate channel completion exception to pending requests instead of a generic IOException - Update McpClient.CreateAsync to throw TransportClosedException with structured completion details when the transport closes during initialization, while preserving existing behavior for cases where the original exception already carries the relevant information - Add unit tests for TransportClosedException construction and details access - Add integration test verifying TransportClosedException with StdioClientCompletionDetails when CreateAsync fails Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/dfc5fff0-2e51-4b55-a663-e9cd3477cad4 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…tion Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/dfc5fff0-2e51-4b55-a663-e9cd3477cad4 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…eAsync ConnectAsync already calls DisposeAsync (which awaits Completion) before re-throwing, so the session is already disposed and Completion is resolved by the time CreateAsync's catch block runs. Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/dc4efc81-4b40-45c3-a743-59ade1c1c6df Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
GetCompletionDetailsAsync already catches all exceptions internally and always returns a ClientCompletionDetails, so Completion never faults. Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/e1a0f673-063d-4707-916d-51824e00b557 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
ConnectAsync's DisposeAsync already awaits Completion, so by the time CreateAsync's catch block runs, it's guaranteed to be completed. The assert documents this invariant. Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/69e63301-5b59-4d6e-bf84-d1e02e421c67 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
tests/ModelContextProtocol.Tests/Client/McpClientCreationTests.cs
Outdated
Show resolved
Hide resolved
|
@copilot While not exactly the same thing, would it make sense to incorporate any of my fixes from https://github.com/modelcontextprotocol/csharp-sdk/tree/halter73/transport-exceptions into this PR? If it makes sense to just merge these changes into this PR, it'd definitely save me time, so I'd appreciate it 😆 |
|
@copilot To be clear, I'm not talking about anything more than the last two commits in https://github.com/modelcontextprotocol/csharp-sdk/tree/halter73/transport-exceptions. Ignore all the previous MRTR stuff |
Split the TransportClosedDuringInit test into two deterministic tests: - ChannelClosed test: SendMessageAsync returns successfully, only the read channel completes with TransportClosedException. Verifies the structured completion details and error log. - SendFails test: SendMessageAsync throws IOException without TransportClosedException in the channel. Verifies the original IOException propagates and error log is produced. Make test class inherit from LoggedTest to verify initialization error logs via MockLoggerProvider. Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/97b170d4-86a2-4f08-998c-65b1c13dbfdb Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Cherry-pick from halter73/transport-exceptions (fd2d71e, 19a1f2c): - StreamClientSessionTransport.SendMessageAsync: re-throw OperationCanceledException when the caller's token triggers it, instead of wrapping in IOException. - StreamServerTransport.SendMessageAsync: same pattern. - SseClientSessionTransport.ConnectAsync: add OperationCanceledException guard and change InvalidOperationException to IOException for consistency (connection failure is I/O, not invalid state). Co-authored-by: Stephen Halter <halter73@gmail.com> Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/f1ba1333-443c-4239-8205-c32db49e9f22 Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Incorporated the last two commits from
|
When a stdio server process exits during
McpClient.CreateAsync(before completing the initialization handshake), callers cannot accessStdioClientCompletionDetails(ExitCode, ProcessId, StandardErrorTail). TheMcpClientis created and disposed internally byCreateAsync, soCompletionis never reachable. The only workaround is parsingIOExceptionmessages or reflecting on internal types.Changes
Make
TransportClosedExceptionpublic — was alreadyinternalwith a comment saying "could be made public in the future." It extendsIOException, so existing catch blocks are unaffected. Carries aDetailsproperty of typeClientCompletionDetails.Propagate channel completion exception to pending requests (
McpSessionHandler.ProcessMessagesCoreAsync) — instead of always failing pending requests with a genericIOException("The server shut down unexpectedly."), propagate theTransportClosedExceptionfrom the transport's channel completion when available.Throw
TransportClosedExceptionfromCreateAsyncwhen completion details add value — after initialization failure, check the session's completion details. Only throwTransportClosedExceptionwhen the details carry an exception not already present in the original exception chain (reference equality walk). This gives stdio callers structured exit info while preserving existing HTTP exception behavior (e.g.,HttpRequestExceptionwithStatusCode).Propagate caller-triggered
OperationCanceledExceptionin transports — when a caller'sCancellationTokentriggers anOperationCanceledExceptionduring a transport send or connect, re-throw it instead of wrapping it inIOException. Applied toStreamClientSessionTransport.SendMessageAsync,StreamServerTransport.SendMessageAsync, andSseClientSessionTransport.ConnectAsync. Also changedSseClientSessionTransport.ConnectAsyncfrom throwingInvalidOperationExceptiontoIOExceptionfor consistency (connection failure is I/O, not invalid state).Usage
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.