Skip to content

fix(client): preserve underlying status code in AutoDetect probe#1530

Open
MukundaKatta wants to merge 2 commits intomodelcontextprotocol:mainfrom
MukundaKatta:fix/autodetect-preserve-415-status
Open

fix(client): preserve underlying status code in AutoDetect probe#1530
MukundaKatta wants to merge 2 commits intomodelcontextprotocol:mainfrom
MukundaKatta:fix/autodetect-preserve-415-status

Conversation

@MukundaKatta
Copy link
Copy Markdown

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. wrong Content-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 SSE GET — 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 against https://api.githubcopilot.com/mcp.

What

In AutoDetectingClientSessionTransport:

  • When the Streamable HTTP probe returns a non-success status, capture the status code and (best-effort, with a 5s read timeout and 1KB body cap, mirroring HttpResponseMessageExtensions.EnsureSuccessStatusCodeWithResponseBodyAsync) the response body, building an HttpRequestException via the existing shared helper.
  • Pass that captured error into the SSE fallback path. If the SSE attempt also fails (and isn't a cancellation), wrap both errors in an AggregateException whose 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.
  • Add a 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.
  • Behavior on the success path (Streamable HTTP works, or SSE fallback works) is unchanged. Cancellation is preserved.

Tested

  • Added AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturns415AndSseFallbackFails in tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs. Drives the AutoDetect probe via SendMessageAsync (since ConnectAsync for AutoDetect only constructs the transport — the probe is lazy), simulates the exact 415→405 sequence from the issue with MockHttpHandler, and walks the exception chain to assert the original 415 status and response body reach the caller.
  • Existing AutoDetect tests (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.

…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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AutoDetect transport hides the real error: surfaces 405 to the user when the server actually returned 415

1 participant