From 7c85ac6706d9c22ba9b2e2229c785da2b62b7b8d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 17:23:21 -0500 Subject: [PATCH] Include response body in HttpRequestException for transport client errors (#1193) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- src/Common/HttpResponseMessageExtensions.cs | 66 +++++++++++++++++ .../Authentication/ClientOAuthProvider.cs | 4 +- .../Client/SseClientSessionTransport.cs | 9 ++- .../StreamableHttpClientSessionTransport.cs | 2 +- .../ModelContextProtocol.Core.csproj | 1 + .../Transport/HttpClientTransportTests.cs | 73 +++++++++++++++++++ 6 files changed, 149 insertions(+), 6 deletions(-) create mode 100644 src/Common/HttpResponseMessageExtensions.cs diff --git a/src/Common/HttpResponseMessageExtensions.cs b/src/Common/HttpResponseMessageExtensions.cs new file mode 100644 index 0000000..05ef092 --- /dev/null +++ b/src/Common/HttpResponseMessageExtensions.cs @@ -0,0 +1,66 @@ +using System.Net; +using System.Net.Http; + +namespace ModelContextProtocol; + +/// +/// Extension methods for . +/// +internal static class HttpResponseMessageExtensions +{ + private const int MaxResponseBodyLength = 1024; + + /// + /// Throws an if the property is . + /// Unlike , this method includes the response body in the exception message + /// to help diagnose issues when the server returns error details in the response body. + /// + /// The HTTP response message to check. + /// The token to monitor for cancellation requests. + /// A task that represents the asynchronous operation. + /// The response status code does not indicate success. + public static async Task EnsureSuccessStatusCodeWithResponseBodyAsync(this HttpResponseMessage response, CancellationToken cancellationToken = default) + { + if (!response.IsSuccessStatusCode) + { + string? responseBody = null; + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(TimeSpan.FromSeconds(5)); + responseBody = await response.Content.ReadAsStringAsync(cts.Token).ConfigureAwait(false); + + if (responseBody.Length > MaxResponseBodyLength) + { + responseBody = responseBody.Substring(0, MaxResponseBodyLength) + "..."; + } + } + catch + { + // Ignore all errors reading the response body (e.g., stream closed, timeout, cancellation) - we'll throw without it. + } + + throw CreateHttpRequestException(response, responseBody); + } + } + + /// + /// Creates an for a non-success response, including the response body in the message. + /// + /// The HTTP response message. + /// The response body content, if available. + /// An with the response details. + public static HttpRequestException CreateHttpRequestException(HttpResponseMessage response, string? responseBody) + { + int statusCodeInt = (int)response.StatusCode; + string message = string.IsNullOrEmpty(responseBody) + ? $"Response status code does not indicate success: {statusCodeInt} ({response.ReasonPhrase})." + : $"Response status code does not indicate success: {statusCodeInt} ({response.ReasonPhrase}). Response body: {responseBody}"; + +#if NET + return new HttpRequestException(message, inner: null, response.StatusCode); +#else + return new HttpRequestException(message); +#endif + } +} diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 7512655..6b45e44 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -499,7 +499,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient }; using var httpResponse = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); - httpResponse.EnsureSuccessStatusCode(); + await httpResponse.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); var tokens = await HandleSuccessfulTokenResponseAsync(httpResponse, cancellationToken).ConfigureAwait(false); LogOAuthAuthorizationCompleted(); @@ -544,7 +544,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient using var httpResponse = await _httpClient.GetAsync(metadataUrl, cancellationToken).ConfigureAwait(false); if (requireSuccess) { - httpResponse.EnsureSuccessStatusCode(); + await httpResponse.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); } else if (!httpResponse.IsSuccessStatusCode) { diff --git a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs index fa9ee97..085f8cc 100644 --- a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs @@ -88,16 +88,19 @@ internal sealed partial class SseClientSessionTransport : TransportBase if (!response.IsSuccessStatusCode) { + // Read the response body once to include in both logging and exception + string responseBody = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + if (_logger.IsEnabled(LogLevel.Trace)) { - LogRejectedPostSensitive(Name, messageId, await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false)); + LogRejectedPostSensitive(Name, messageId, responseBody); } else { LogRejectedPost(Name, messageId); } - response.EnsureSuccessStatusCode(); + throw HttpResponseMessageExtensions.CreateHttpRequestException(response, responseBody); } } @@ -148,7 +151,7 @@ internal sealed partial class SseClientSessionTransport : TransportBase using var response = await _httpClient.SendAsync(request, message: null, cancellationToken).ConfigureAwait(false); - response.EnsureSuccessStatusCode(); + await response.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs index c33d353..c80b6a4 100644 --- a/src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs @@ -61,7 +61,7 @@ internal sealed partial class StreamableHttpClientSessionTransport : TransportBa { // Immediately dispose the response. SendHttpRequestAsync only returns the response so the auto transport can look at it. using var response = await SendHttpRequestAsync(message, cancellationToken).ConfigureAwait(false); - response.EnsureSuccessStatusCode(); + await response.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); } // This is used by the auto transport so it can fall back and try SSE given a non-200 response without catching an exception. diff --git a/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj b/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj index a1ac1a6..63b7bbd 100644 --- a/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj +++ b/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj @@ -25,6 +25,7 @@ + diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs index fc1ac2d..d1f03b7 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs @@ -82,6 +82,32 @@ public class HttpClientTransportTests : LoggedTest Assert.Equal(1, retries); } + [Fact] + public async Task ConnectAsync_Throws_HttpRequestException_With_ResponseBody_On_ErrorStatusCode() + { + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(_transportOptions, httpClient, LoggerFactory); + + const string errorDetails = "Bad request: Invalid MCP protocol version"; + mockHttpHandler.RequestHandler = (request) => + { + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.BadRequest, + ReasonPhrase = "Bad Request", + Content = new StringContent(errorDetails) + }); + }; + + var httpException = await Assert.ThrowsAsync(() => transport.ConnectAsync(TestContext.Current.CancellationToken)); + Assert.Contains(errorDetails, httpException.Message); + Assert.Contains("400", httpException.Message); +#if NET + Assert.Equal(HttpStatusCode.BadRequest, httpException.StatusCode); +#endif + } + [Fact] public async Task SendMessageAsync_Handles_Accepted_Response() { @@ -120,6 +146,53 @@ public class HttpClientTransportTests : LoggedTest Assert.True(true); } + [Fact] + public async Task SendMessageAsync_Throws_HttpRequestException_With_ResponseBody_On_ErrorStatusCode() + { + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(_transportOptions, httpClient, LoggerFactory); + + var firstCall = true; + const string errorDetails = "Invalid JSON-RPC message format: missing 'id' field"; + + mockHttpHandler.RequestHandler = (request) => + { + if (request.Method == HttpMethod.Post && request.RequestUri?.AbsoluteUri == "http://localhost:8080/sseendpoint") + { + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.BadRequest, + ReasonPhrase = "Bad Request", + Content = new StringContent(errorDetails) + }); + } + else + { + if (!firstCall) + throw new IOException("Abort"); + else + firstCall = false; + + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent("event: endpoint\r\ndata: /sseendpoint\r\n\r\n") + }); + } + }; + + await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); + var httpException = await Assert.ThrowsAsync(() => + session.SendMessageAsync(new JsonRpcRequest { Method = RequestMethods.Initialize, Id = new RequestId(44) }, CancellationToken.None)); + + Assert.Contains(errorDetails, httpException.Message); + Assert.Contains("400", httpException.Message); +#if NET + Assert.Equal(HttpStatusCode.BadRequest, httpException.StatusCode); +#endif + } + [Fact] public async Task ReceiveMessagesAsync_Handles_Messages() {