From 9b21522964666740c9dc27139df38cb43085c29e Mon Sep 17 00:00:00 2001
From: Xue Cai <xuecai@microsoft.com>
Date: Sat, 14 Mar 2026 12:14:27 -0700
Subject: [PATCH] Fix: swallow _receiveTask exceptions in
 SseClientSessionTransport.CloseAsync

When SseClientSessionTransport.ConnectAsync fails (e.g. the SSE GET
returns 405), its catch block calls CloseAsync() before wrapping the
error in InvalidOperationException("Failed to connect transport", ex).

However, CloseAsync() awaits _receiveTask which is already faulted
with the same HttpRequestException. Since there is no try-catch around
that await, the exception propagates out of CloseAsync, out of the
catch block, and the InvalidOperationException wrapping is never
reached.

This means callers of ConnectAsync receive an unwrapped
HttpRequestException instead of the documented InvalidOperationException
wrapper, which breaks exception filtering in downstream code.

The fix wraps `await _receiveTask` in CloseAsync with a try-catch that
swallows both OperationCanceledException (normal shutdown) and other
exceptions (already observed and forwarded via
_connectionEstablished.TrySetException in ReceiveMessagesAsync).

Includes a regression test that verifies ConnectAsync throws
InvalidOperationException (not HttpRequestException) when both
Streamable HTTP POST and SSE GET fallback fail.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../Client/SseClientSessionTransport.cs       | 18 ++++++-
 .../HttpClientTransportAutoDetectTests.cs     | 50 +++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

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..2a0714219 100644
--- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs
+++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs
@@ -47,6 +47,56 @@ 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}");
+        };
+
+        var ex = await Assert.ThrowsAsync<InvalidOperationException>(
+            () => transport.ConnectAsync(TestContext.Current.CancellationToken));
+
+        Assert.Equal("Failed to connect transport", ex.Message);
+        Assert.IsType<HttpRequestException>(ex.InnerException);
+    }
+
     [Fact] 
     public async Task AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails()
     {