diff --git a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs index c9e821adf..6f85a62f1 100644 --- a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs @@ -96,8 +96,6 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can // behavior. Capture the underlying error (status + body) before falling back so that, // if SSE also fails, we can surface the real Streamable HTTP diagnostic to the caller // instead of dropping it on the floor (see https://github.com/modelcontextprotocol/csharp-sdk/issues/1526). - LogStreamableHttpFailed(_name, response.StatusCode); - // This reads the response body a second time for the application/json case, where // TryReadJsonRpcErrorAsync above already read it. HttpContent buffers after the first // read, so this returns the same buffered content and is safe (not a second stream @@ -105,6 +103,26 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can // TryReadJsonRpcErrorAsync returns early on the content type, so there is no double read. var streamableHttpError = await HttpResponseMessageExtensions.CreateHttpRequestExceptionWithBodyAsync(response, cancellationToken).ConfigureAwait(false); + if (IsDiscoverProbeRejection(message, response.StatusCode)) + { + // The server/discover probe is protocol negotiation, not transport detection. A server + // predating SEP-2575 rejects the session-less POST with 400 (can't parse the request) or + // 404 (requires Mcp-Session-Id on every non-initialize POST) whether it speaks Streamable + // HTTP or SSE, so neither status is evidence about which transport to use. McpClientImpl + // .ConnectAsync treats exactly these two statuses as "initialize-handshake server" and + // immediately retries with initialize on this same transport — and that attempt still + // falls back to SSE, so an SSE-only server is reached one POST later rather than not at + // all. Attempting SSE here instead spends a GET whose result is discarded on every + // connect to a Streamable-HTTP-only server that predates SEP-2575, and logs a "falling + // back to SSE transport" line that misreports settled protocol negotiation as a failure. + LogSkippingSseFallbackForDiscoverProbe(_name, response.StatusCode); + + await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); + throw streamableHttpError; + } + + LogStreamableHttpFailed(_name, response.StatusCode); + await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); await InitializeSseTransportAsync(message, streamableHttpError, cancellationToken).ConfigureAwait(false); } @@ -119,6 +137,15 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can } } + /// + /// Returns when the failed request was the SEP-2575 server/discover probe and the + /// status is one of the two already reads as "this server requires the initialize + /// handshake", meaning the SSE fallback cannot contribute anything the initialize retry won't. + /// + private static bool IsDiscoverProbeRejection(JsonRpcMessage message, HttpStatusCode statusCode) => + statusCode is HttpStatusCode.BadRequest or HttpStatusCode.NotFound && + message is JsonRpcRequest { Method: RequestMethods.ServerDiscover }; + private async Task InitializeSseTransportAsync(JsonRpcMessage message, HttpRequestException? streamableHttpError, CancellationToken cancellationToken) { if (_options.KnownSessionId is not null) @@ -181,6 +208,9 @@ public async ValueTask DisposeAsync() [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} streamable HTTP transport failed with status code {StatusCode}, falling back to SSE transport.")] private partial void LogStreamableHttpFailed(string endpointName, HttpStatusCode statusCode); + [LoggerMessage(Level = LogLevel.Debug, Message = "{EndpointName} server/discover probe rejected with status code {StatusCode}; skipping the SSE fallback so the initialize handshake is attempted instead.")] + private partial void LogSkippingSseFallbackForDiscoverProbe(string endpointName, HttpStatusCode statusCode); + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} using Streamable HTTP transport.")] private partial void LogUsingStreamableHttp(string endpointName); diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs index 7100e728a..c46383306 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs @@ -505,4 +505,155 @@ await Assert.ThrowsAnyAsync(() => MockLoggerProvider.LogMessages, m => m.LogLevel == LogLevel.Warning && m.Message.Contains("SSE fallback failed")); } + + // A 400/404 on the SEP-2575 server/discover probe is protocol negotiation, not transport detection: + // a pre-SEP-2575 server rejects the session-less POST whether it speaks Streamable HTTP or SSE, and + // McpClientImpl.ConnectAsync reads exactly those two statuses as "initialize-handshake server" and + // retries with initialize. The SSE GET can therefore only waste a round trip here. + [Theory] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.BadRequest)] + public async Task AutoDetectMode_SkipsSseFallback_WhenDiscoverProbeIsRejected(HttpStatusCode statusCode) + { + var options = new HttpClientTransportOptions + { + Endpoint = new Uri("http://localhost"), + TransportMode = HttpTransportMode.AutoDetect, + Name = "AutoDetect discover probe test client" + }; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); + var getCount = 0; + + mockHttpHandler.RequestHandler = request => + { + if (request.Method == HttpMethod.Get) + { + getCount++; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.MethodNotAllowed)); + } + + return Task.FromResult(new HttpResponseMessage(statusCode) + { + Content = new StringContent("Not Found"), + }); + }; + + await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); + + var ex = await Assert.ThrowsAsync(() => + session.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) }, + TestContext.Current.CancellationToken)); + + Assert.Equal(0, getCount); + Assert.Equal(statusCode, ex.Data["ModelContextProtocol.HttpStatusCode"]); + Assert.DoesNotContain( + MockLoggerProvider.LogMessages, + m => m.Message.Contains("falling back to SSE transport")); + } + + // Skipping the SSE attempt on the discover probe must not strand an SSE-only server: the initialize + // retry that McpClientImpl.ConnectAsync issues next goes through this same transport and still falls + // back to SSE, so such a server is reached one POST later rather than not at all. + [Fact] + public async Task AutoDetectMode_StillFallsBackToSse_WhenInitializeFollowsRejectedDiscoverProbe() + { + var options = new HttpClientTransportOptions + { + Endpoint = new Uri("http://localhost"), + TransportMode = HttpTransportMode.AutoDetect, + Name = "AutoDetect discover probe then initialize test client" + }; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); + var ssePipe = new Pipe(); + var sseEndpointPostCount = 0; + + await ssePipe.Writer.WriteAsync( + System.Text.Encoding.UTF8.GetBytes("event: endpoint\r\ndata: /sse-endpoint\r\n\r\n"), + TestContext.Current.CancellationToken); + await ssePipe.Writer.FlushAsync(TestContext.Current.CancellationToken); + + mockHttpHandler.RequestHandler = request => + { + if (request.Method == HttpMethod.Get) + { + var content = new StreamContent(ssePipe.Reader.AsStream()); + content.Headers.ContentType = new("text/event-stream"); + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = content }); + } + + if (request.RequestUri?.AbsolutePath == "/sse-endpoint") + { + sseEndpointPostCount++; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.Accepted)); + } + + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new StringContent("Mcp-Session-Id required"), + }); + }; + + await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); + + await Assert.ThrowsAsync(() => + session.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) }, + TestContext.Current.CancellationToken)); + + await session.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.Initialize, Id = new RequestId(2) }, + TestContext.Current.CancellationToken); + + Assert.Equal(1, sseEndpointPostCount); + + await ssePipe.Writer.CompleteAsync(); + } + + // The skip is scoped to the two statuses ConnectAsync acts on. Any other failure on the discover probe + // keeps the original fallback, because it is not evidence that an initialize retry is coming. + [Fact] + public async Task AutoDetectMode_FallsBackToSse_WhenDiscoverProbeFailsWithUnrelatedStatus() + { + var options = new HttpClientTransportOptions + { + Endpoint = new Uri("http://localhost"), + TransportMode = HttpTransportMode.AutoDetect, + Name = "AutoDetect discover probe 415 test client" + }; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); + var getCount = 0; + + mockHttpHandler.RequestHandler = request => + { + if (request.Method == HttpMethod.Get) + { + getCount++; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.MethodNotAllowed)); + } + + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.UnsupportedMediaType) + { + Content = new StringContent("Content-Type must be 'application/json'"), + }); + }; + + await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); + + await Assert.ThrowsAsync(() => + session.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) }, + TestContext.Current.CancellationToken)); + + Assert.Equal(1, getCount); + } }