Fix HTTP fallback error handling (#1791)
This commit is contained in:
parent
609499b298
commit
df11d7da3b
5 changed files with 146 additions and 24 deletions
|
|
@ -77,16 +77,19 @@ internal sealed partial class AutoDetectingClientSessionTransport : ITransport
|
|||
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
|
||||
{
|
||||
|
|
|
|||
|
|
@ -367,14 +367,6 @@ internal sealed partial class McpClientImpl : McpClient
|
|||
// 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))
|
||||
{
|
||||
// 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.
|
||||
|
|
|
|||
|
|
@ -80,8 +80,7 @@ internal sealed partial class StreamableHttpClientSessionTransport : TransportBa
|
|||
// 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);
|
||||
}
|
||||
|
|
@ -89,10 +88,11 @@ internal sealed partial class StreamableHttpClientSessionTransport : TransportBa
|
|||
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;
|
||||
|
||||
/// <summary>
|
||||
/// Reads a JSON-RPC error envelope from an <c>application/json</c> response body, returning
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ public class July2026ProtocolHttpFallbackTests(ITestOutputHelper outputHelper) :
|
|||
base.Dispose();
|
||||
}
|
||||
|
||||
private async Task StartServerAsync(RequestDelegate handler)
|
||||
private async Task StartServerAsync(RequestDelegate handler, bool acceptGet = false)
|
||||
{
|
||||
Builder.Services.Configure<JsonOptions>(options =>
|
||||
{
|
||||
|
|
@ -65,7 +65,14 @@ public class July2026ProtocolHttpFallbackTests(ITestOutputHelper outputHelper) :
|
|||
});
|
||||
|
||||
_app = Builder.Build();
|
||||
_app.MapPost("/mcp", handler);
|
||||
if (acceptGet)
|
||||
{
|
||||
_app.MapMethods("/mcp", [HttpMethods.Get, HttpMethods.Post], handler);
|
||||
}
|
||||
else
|
||||
{
|
||||
_app.MapPost("/mcp", handler);
|
||||
}
|
||||
await _app.StartAsync(TestContext.Current.CancellationToken);
|
||||
}
|
||||
|
||||
|
|
@ -302,6 +309,63 @@ public class July2026ProtocolHttpFallbackTests(ITestOutputHelper outputHelper) :
|
|||
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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -239,6 +239,28 @@ public class July2026ProtocolFallbackTests(ITestOutputHelper testOutputHelper) :
|
|||
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)]
|
||||
|
|
@ -317,6 +339,47 @@ public class July2026ProtocolFallbackTests(ITestOutputHelper testOutputHelper) :
|
|||
}
|
||||
};
|
||||
|
||||
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) };
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue