Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,19 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
else if (await StreamableHttpClientSessionTransport.TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError)
{
// A JSON-RPC error envelope in the body means the peer IS a Streamable HTTP server.
// It just rejected our specific request (e.g., -32022 UnsupportedProtocolVersion,
// -32021 MissingRequiredClientCapability, -32020 HeaderMismatch, or any other
// application-level error). Don't fall back to SSE — that would mask the real signal
// and surface a misleading "session id required" error from the SSE GET path.
// Adopt the Streamable HTTP transport and throw the structured exception so the
// connect-time fallback logic can react per spec PR #2844. Setting ActiveTransport
// first makes the catch filter below leave the now-owned transport alone.
// Adopt it before surfacing the failure so the catch filter leaves the now-owned
// transport alone, and never mask the response by attempting deprecated SSE.
LogUsingStreamableHttp(_name);
ActiveTransport = streamableHttpTransport;
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);

if (StreamableHttpClientSessionTransport.ShouldSurfaceJsonRpcErrorAsProtocolException(response.StatusCode, parsedError))
{
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
}

// TryReadJsonRpcErrorAsync buffered the content, so this preserves the same response
// body and status without consuming the network stream a second time.
throw await HttpResponseMessageExtensions.CreateHttpRequestExceptionWithBodyAsync(response, cancellationToken).ConfigureAwait(false);
}
else
{
Expand Down
8 changes: 0 additions & 8 deletions src/ModelContextProtocol.Core/Client/McpClientImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,6 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default)
// fallback): falling back to initialize wouldn't fix a malformed envelope.
throw;
}
catch (McpProtocolException ex) when (
ex.ErrorCode == McpErrorCode.InvalidRequest &&
ex.Message.Contains(McpHttpHeaders.SessionId, StringComparison.Ordinal))
Comment thread
halter73 marked this conversation as resolved.
{
// Local transport validation: a 2026-07-28+ response must not carry HTTP session state.
// This is not evidence of an initialize-handshake server, so do not fall back.
throw;
}
catch (McpProtocolException)
{
// Per spec PR #2844, the fallback MUST NOT be keyed to a single error code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,19 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation
// for robustness. Servers occasionally emit them with 4xx codes other than 400.
if (!response.IsSuccessStatusCode &&
await TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError &&
(response.StatusCode == HttpStatusCode.BadRequest ||
IsPerRequestMetadataProtocolErrorCode((McpErrorCode)parsedError.Error.Code)))
ShouldSurfaceJsonRpcErrorAsProtocolException(response.StatusCode, parsedError))
{
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
}

await response.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false);
}

private static bool IsPerRequestMetadataProtocolErrorCode(McpErrorCode code) =>
code is McpErrorCode.UnsupportedProtocolVersion
or McpErrorCode.MissingRequiredClientCapability
or McpErrorCode.HeaderMismatch;
internal static bool ShouldSurfaceJsonRpcErrorAsProtocolException(HttpStatusCode statusCode, JsonRpcError error) =>
statusCode == HttpStatusCode.BadRequest ||
(McpErrorCode)error.Error.Code is McpErrorCode.UnsupportedProtocolVersion
or McpErrorCode.MissingRequiredClientCapability
or McpErrorCode.HeaderMismatch;

@halter73 halter73 Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't new in this PR, but where did we get this list of three error codes? Looking at the "Backward Compatibility with Initialization-Based Versions" section in modelcontextprotocol/modelcontextprotocol#2844, I only see UnsupportedProtocolVersionError listed as indicating "the client retries with a supported version rather than falling back. Anything else identifies a legacy server."

I think this means that UnsupportedProtocolVersion should be treated differently in than the other error types like MissingRequiredClientCapability and HeaderMismatch for the purpose of server/discover fallback. In the future, when there's more than one protocol supporting server/discover, the supportedVersions included in this kind of error could be used to determine what older version of the protocol to make a subsequent server/discover call with.

I see modelcontextprotocol/modelcontextprotocol#2575 mentions MissingRequiredClientCapability, but not in a way that I think requires us to distinguish it from UnsupportedProtocolVersion.

Can we avoid classifying responses based on the JSON-RPC error code here? A non-successful HTTP response containing any JSON-RPC error should become an McpProtocolException, while other HTTP failures should remain HttpRequestException. McpClientImpl can then apply the protocol-specific retry or fallback policy.


/// <summary>
/// Reads a JSON-RPC error envelope from an <c>application/json</c> response body, returning
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,22 @@ public async ValueTask DisposeAsync()
base.Dispose();
}

private async Task StartServerAsync(RequestDelegate handler)
private async Task StartServerAsync(RequestDelegate handler, bool acceptGet = false)
{
Builder.Services.Configure<JsonOptions>(options =>
{
options.SerializerOptions.TypeInfoResolverChain.Add(McpJsonUtilities.DefaultOptions.TypeInfoResolver!);
});

_app = Builder.Build();
_app.MapPost("/mcp", handler);
if (acceptGet)
{
_app.MapMethods("/mcp", [HttpMethods.Get, HttpMethods.Post], handler);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could we just do this for all the tests calling StartServerAsync and remove the acceptGet parameter? For tests where this matters, the handler could just return a 405 for GET requests manually, but I don't think that will be necessary.

}
else
{
_app.MapPost("/mcp", handler);
}
await _app.StartAsync(TestContext.Current.CancellationToken);
}

Expand Down Expand Up @@ -302,6 +309,63 @@ await WriteJsonRpcErrorAsync(context, HttpStatusCode.BadRequest,
Assert.False(initializeReceived);
}

[Theory]
[InlineData(HttpStatusCode.Unauthorized)]
[InlineData(HttpStatusCode.Forbidden)]
[InlineData(HttpStatusCode.InternalServerError)]
public async Task AutoDetect_OnNonModernJsonRpcErrorOutside400_PreservesHttpFailure_NoFallback(HttpStatusCode statusCode)
{
var ct = TestContext.Current.CancellationToken;
var initializeRequests = 0;
var sseGetRequests = 0;

await StartServerAsync(async context =>
{
if (HttpMethods.IsGet(context.Request.Method))
{
sseGetRequests++;
context.Response.StatusCode = StatusCodes.Status405MethodNotAllowed;
return;
}

var message = await JsonSerializer.DeserializeAsync(
context.Request.Body,
GetJsonTypeInfo<JsonRpcMessage>(),
ct);

if (message is JsonRpcRequest { Method: RequestMethods.Initialize })
{
initializeRequests++;
}

await WriteJsonRpcErrorAsync(
context,
statusCode,
code: (int)McpErrorCode.InvalidRequest,
message: "non-modern structured error");
}, acceptGet: true);

await using var transport = new HttpClientTransport(new()
{
Endpoint = new("http://localhost:5000/mcp"),
TransportMode = HttpTransportMode.AutoDetect,
}, HttpClient, LoggerFactory);

var exception = await Assert.ThrowsAsync<HttpRequestException>(async () =>
{
await using var client = await McpClient.CreateAsync(
transport,
new McpClientOptions(),
loggerFactory: LoggerFactory,
cancellationToken: ct);
});

Assert.Equal(statusCode, exception.StatusCode);
Assert.Contains("non-modern structured error", exception.Message);
Assert.Equal(0, initializeRequests);
Assert.Equal(0, sseGetRequests);
}

[Fact]
public async Task Client_OnPerRequestMetadataResponseWithMcpSessionId_IgnoresSessionState()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,28 @@ public async Task Client_OnFallbackHttpStatusFromProbe_FallsBackTo_Initialize(
Assert.Equal(McpProtocolVersions.November2025ProtocolVersion, client.NegotiatedProtocolVersion);
}

[Theory]
[InlineData(HttpTransportMode.StreamableHttp)]
[InlineData(HttpTransportMode.AutoDetect)]
public async Task Client_OnStructuredInvalidRequestFromHttpProbe_FallsBackTo_Initialize(
HttpTransportMode transportMode)
{
var ct = TestContext.Current.CancellationToken;
var initializeReceived = false;

using var mockHttpHandler = new MockHttpHandler();
using var httpClient = new HttpClient(mockHttpHandler);
mockHttpHandler.RequestHandler = CreateStructuredInvalidRequestProbeServer(
() => initializeReceived = true);

await using var transport = CreateTransport(httpClient, transportMode);
await using var client = await McpClient.CreateAsync(transport, new McpClientOptions(),
loggerFactory: LoggerFactory, cancellationToken: ct);

Assert.True(initializeReceived);
Assert.Equal(McpProtocolVersions.November2025ProtocolVersion, client.NegotiatedProtocolVersion);
}

[Theory]
[InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.StreamableHttp)]
[InlineData(HttpStatusCode.Forbidden, HttpTransportMode.StreamableHttp)]
Expand Down Expand Up @@ -317,6 +339,47 @@ private static Func<HttpRequestMessage, Task<HttpResponseMessage>> CreateProbeRe
}
};

private static Func<HttpRequestMessage, Task<HttpResponseMessage>> CreateStructuredInvalidRequestProbeServer(
Action onInitialize)
=> async request =>
{
if (request.Method == HttpMethod.Get)
return EmptyResponse(HttpStatusCode.MethodNotAllowed);

var body = await request.Content!.ReadAsStringAsync();
using var doc = JsonDocument.Parse(body);
if (!doc.RootElement.TryGetProperty("method", out var methodElement))
return EmptyResponse(HttpStatusCode.Accepted);

if (methodElement.GetString() == RequestMethods.ServerDiscover)
{
var id = doc.RootElement.GetProperty("id").GetRawText();
var error = "{\"jsonrpc\":\"2.0\",\"id\":" + id
+ ",\"error\":{\"code\":-32600,\"message\":\"Mcp-Session-Id header is required\"}}";
return new HttpResponseMessage(HttpStatusCode.BadRequest)
{
Content = new StringContent(error, Encoding.UTF8, "application/json"),
};
}

if (methodElement.GetString() == RequestMethods.Initialize)
{
onInitialize();
var id = doc.RootElement.GetProperty("id").GetRawText();
var result = "{\"jsonrpc\":\"2.0\",\"id\":" + id
+ ",\"result\":{\"protocolVersion\":\"" + McpProtocolVersions.November2025ProtocolVersion
+ "\",\"capabilities\":{},\"serverInfo\":{\"name\":\"test\",\"version\":\"1.0\"}}}";
var response = new HttpResponseMessage(HttpStatusCode.OK)
{
Content = new StringContent(result, Encoding.UTF8, "application/json"),
};
response.Headers.Add("mcp-session-id", "test-session");
return response;
}

return EmptyResponse(HttpStatusCode.Accepted);
};

private static HttpResponseMessage EmptyResponse(HttpStatusCode status)
=> new(status) { Content = new StringContent(string.Empty) };

Expand Down