diff --git a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs index 0bcc69417..8e70b1582 100644 --- a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs @@ -115,7 +115,23 @@ private async Task CloseAsync() { if (_receiveTask != null) { - await _receiveTask.ConfigureAwait(false); + // Swallow exceptions from _receiveTask so that callers (e.g. ConnectAsync's + // catch block) are not disrupted. The exception was already observed and + // forwarded via _connectionEstablished.TrySetException in ReceiveMessagesAsync. + try + { + await _receiveTask.ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Expected during normal shutdown via _connectionCts cancellation. + } + catch (Exception) + { + // Already observed by ReceiveMessagesAsync — logged and set on + // _connectionEstablished. Swallowing here prevents the exception + // from escaping CloseAsync and preempting the caller's own throw. + } } } finally diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs index 768ebf7ea..d509c4baa 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs @@ -47,6 +47,60 @@ public async Task AutoDetectMode_UsesStreamableHttp_WhenServerSupportsIt() Assert.NotNull(session); } + [Fact] + public async Task AutoDetectMode_WhenBothTransportsFail_ThrowsInvalidOperationException() + { + // Regression test: when Streamable HTTP POST fails (e.g. 403) and the SSE GET + // fallback also fails (e.g. 405), ConnectAsync should wrap the error in an + // InvalidOperationException. Previously, CloseAsync() would re-throw the + // HttpRequestException from the faulted _receiveTask, preempting the wrapping. + var options = new HttpClientTransportOptions + { + Endpoint = new Uri("http://localhost"), + TransportMode = HttpTransportMode.AutoDetect, + Name = "AutoDetect test client" + }; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); + + mockHttpHandler.RequestHandler = (request) => + { + if (request.Method == HttpMethod.Post) + { + // Streamable HTTP POST fails with 403 (auth error) + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.Forbidden, + Content = new StringContent("Forbidden") + }); + } + + if (request.Method == HttpMethod.Get) + { + // SSE GET fallback fails with 405 + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.MethodNotAllowed, + Content = new StringContent("Method Not Allowed") + }); + } + + throw new InvalidOperationException($"Unexpected request: {request.Method}"); + }; + + // ConnectAsync for AutoDetect mode just creates the transport without sending + // any HTTP request. The auto-detection is triggered lazily by the first + // SendMessageAsync call, which happens inside McpClient.CreateAsync when it + // sends the JSON-RPC "initialize" message. + var ex = await Assert.ThrowsAsync( + () => McpClient.CreateAsync(transport, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal("Failed to connect transport", ex.Message); + Assert.IsType(ex.InnerException); + } + [Fact] public async Task AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails() { diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs index 60384d3c2..62be34fcd 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs @@ -78,8 +78,11 @@ public async Task ConnectAsync_Throws_Exception_On_Failure() throw new Exception("Test exception"); }; - var exception = await Assert.ThrowsAsync(() => transport.ConnectAsync(TestContext.Current.CancellationToken)); - Assert.Equal("Test exception", exception.Message); + // SseClientSessionTransport.ConnectAsync wraps errors in InvalidOperationException + var exception = await Assert.ThrowsAsync(() => transport.ConnectAsync(TestContext.Current.CancellationToken)); + Assert.Equal("Failed to connect transport", exception.Message); + Assert.IsType(exception.InnerException); + Assert.Equal("Test exception", exception.InnerException!.Message); Assert.Equal(1, retries); } @@ -101,7 +104,10 @@ public async Task ConnectAsync_Throws_HttpRequestException_With_ResponseBody_On_ }); }; - var httpException = await Assert.ThrowsAsync(() => transport.ConnectAsync(TestContext.Current.CancellationToken)); + // SseClientSessionTransport.ConnectAsync wraps errors in InvalidOperationException + var wrappedException = await Assert.ThrowsAsync(() => transport.ConnectAsync(TestContext.Current.CancellationToken)); + Assert.Equal("Failed to connect transport", wrappedException.Message); + var httpException = Assert.IsType(wrappedException.InnerException); Assert.Contains(errorDetails, httpException.Message); Assert.Contains("400", httpException.Message); #if NET