fix(client): preserve underlying status code in AutoDetect probe#1530
Open
MukundaKatta wants to merge 2 commits intomodelcontextprotocol:mainfrom
Open
fix(client): preserve underlying status code in AutoDetect probe#1530MukundaKatta wants to merge 2 commits intomodelcontextprotocol:mainfrom
MukundaKatta wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
…llback When the Streamable HTTP probe fails with a non-success status (e.g. 415 Unsupported Media Type) the AutoDetect transport falls back to SSE. If the SSE attempt also fails (e.g. a Streamable-HTTP-only server returns 405 to the GET), the SDK previously surfaced only the SSE error and silently dropped the original Streamable HTTP response, masking the real server diagnostic. Capture the original status + response body up front and, when the SSE fallback also fails, throw an AggregateException whose first inner is the Streamable HTTP error and whose second is the SSE failure, so the user sees the actual server response. Fixes modelcontextprotocol#1526.
…lback fails Regression test for modelcontextprotocol#1526. Triggers the AutoDetect probe via SendMessageAsync (ConnectAsync only constructs the transport) and walks the exception chain to verify the original 415 status and response body are preserved when the SSE fallback also fails (405 to the GET).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
HttpTransportMode.AutoDetect(the default) tries Streamable HTTP first and falls back to SSE on any non-success status. When Streamable HTTP returns 415 (e.g. wrongContent-Type, as GitHub Copilot's MCP endpoint does today) and the SSE fallback then fails — common against Streamable-HTTP-only servers that return 405 to the SSEGET— the SDK only surfaces the 405. The original 415 plus its body (the actual server diagnostic) is silently dropped.Result: users debug the wrong protocol. They see "405 Method Not Allowed" against an endpoint they never explicitly tried to
GET, with no hint that the real failure was a content-type mismatch on the Streamable HTTP attempt. Reported in #1526 with a reproducer againsthttps://api.githubcopilot.com/mcp.What
In
AutoDetectingClientSessionTransport:HttpResponseMessageExtensions.EnsureSuccessStatusCodeWithResponseBodyAsync) the response body, building anHttpRequestExceptionvia the existing shared helper.AggregateExceptionwhose first inner is the original Streamable HTTP error and whose second is the SSE failure. The user now sees the real server response without losing the fallback diagnostic.Warning-level log when the SSE fallback fails after Streamable HTTP also failed, so the dual-failure case is visible in logs even when callers swallow the exception.Tested
AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturns415AndSseFallbackFailsintests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs. Drives the AutoDetect probe viaSendMessageAsync(sinceConnectAsyncfor AutoDetect only constructs the transport — the probe is lazy), simulates the exact 415→405 sequence from the issue withMockHttpHandler, and walks the exception chain to assert the original415status and response body reach the caller.AutoDetectMode_UsesStreamableHttp_WhenServerSupportsIt,AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails) untouched and still pass conceptually — neither relied on a specific exception shape from the dual-failure path.Closes #1526.