Fix: swallow _receiveTask exceptions in SseClientSessionTransport.CloseAsync (#1432)
Co-authored-by: Xue Cai <xuecai@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
This commit is contained in:
parent
e03acd1e7c
commit
ec216b5202
2 changed files with 55 additions and 2 deletions
|
|
@ -66,7 +66,7 @@ internal sealed partial class SseClientSessionTransport : TransportBase
|
||||||
{
|
{
|
||||||
LogTransportConnectFailed(Name, ex);
|
LogTransportConnectFailed(Name, ex);
|
||||||
await CloseAsync().ConfigureAwait(false);
|
await CloseAsync().ConfigureAwait(false);
|
||||||
throw new IOException("Failed to connect transport.", ex);
|
throw;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -198,7 +198,6 @@ internal sealed partial class SseClientSessionTransport : TransportBase
|
||||||
|
|
||||||
LogTransportReadMessagesFailed(Name, ex);
|
LogTransportReadMessagesFailed(Name, ex);
|
||||||
_connectionEstablished.TrySetException(ex);
|
_connectionEstablished.TrySetException(ex);
|
||||||
throw;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
finally
|
finally
|
||||||
|
|
|
||||||
|
|
@ -49,6 +49,60 @@ public class HttpClientTransportAutoDetectTests(ITestOutputHelper testOutputHelp
|
||||||
Assert.NotNull(session);
|
Assert.NotNull(session);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task AutoDetectMode_WhenBothTransportsFail_PreservesStreamableHttpException()
|
||||||
|
{
|
||||||
|
// Regression test: when Streamable HTTP POST fails (e.g. 403) and the SSE GET
|
||||||
|
// fallback also fails (e.g. 405), the original Streamable HTTP error should
|
||||||
|
// be preserved. The SSE connection failure is available as its inner exception.
|
||||||
|
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<HttpRequestException>(
|
||||||
|
() => McpClient.CreateAsync(transport, cancellationToken: TestContext.Current.CancellationToken));
|
||||||
|
|
||||||
|
Assert.Contains("403", ex.Message);
|
||||||
|
Assert.IsType<HttpRequestException>(ex.InnerException);
|
||||||
|
Assert.Contains("405", ex.InnerException.Message);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails()
|
public async Task AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue