fix: fall back to SSE when AutoDetect probe gets 405 with JSON-RPC error body - #1849
fix: fall back to SSE when AutoDetect probe gets 405 with JSON-RPC error body#1849ump45nose wants to merge 1 commit into
Conversation
halter73
left a comment
halter73
left a comment
There was a problem hiding this comment.
#1791 merged after this PR was opened and now conflicts with these changes. Could you rebase and update this to apply the spec’s full 400/404/405 allowlist, while preserving 401/403/5xx without trying SSE? Tests covering both structured and unstructured responses would make this ready for another pass.
This should also supersede #1444 once merged.
| ActiveTransport = streamableHttpTransport; | ||
| } | ||
| else if (await StreamableHttpClientSessionTransport.TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError) | ||
| else if (response.StatusCode != HttpStatusCode.MethodNotAllowed && |
There was a problem hiding this comment.
This skips recognized modern errors on every 405, while unstructured 401/403/5xx responses still fall through to SSE. The spec limits this fallback to 400, 404, or 405 when the body is not a recognized modern JSON-RPC error. Can we classify the full response here instead of special-casing only 405?
| ex.GetStatusCode() is HttpStatusCode.BadRequest or HttpStatusCode.NotFound) | ||
| ex.GetStatusCode() is HttpStatusCode.BadRequest | ||
| or HttpStatusCode.NotFound | ||
| or HttpStatusCode.MethodNotAllowed) |
There was a problem hiding this comment.
A 405 here means the POST endpoint rejected the request, so retrying initialize over the same transport is not useful. The spec's 405 handling is the AutoDetect transport fallback to SSE. Can we keep that in AutoDetectingClientSessionTransport and remove 405 from this catch?
- 仅对 400、404、405 探测失败回退初始化握手。 - 保留 401、403、5xx 的 HTTP 语义且不发起 SSE 请求。 - 覆盖结构化与非结构化探测响应的回归场景。
3566f99 to
32ef18a
Compare
|
Rebased onto current main and resolved the #1791 conflict at 32ef18a. The probe fallback is now explicitly limited to 400/404/405 for both structured and unstructured responses; 401/403/5xx preserve their HTTP failure without an SSE GET. The regression matrix covers both transport modes and asserts that non-allowlisted AutoDetect failures do not request SSE.\n\nI could not run the local .NET suite because this host has only SDK 6.0.136 while global.json requires 10.0.101; I did run git diff --check. The fork CI workflows are currently awaiting maintainer approval.\n\nAI-assisted maintenance; I reviewed the conflict resolution and validation evidence. |
2 participants
Summary
fix: fall back to SSE when AutoDetect probe gets 405 with JSON-RPC error body
Verification
dotnet test tests/ModelContextProtocol.Tests --framework net10.0 -c Release --filter 'FullyQualifiedName
HttpClientTransportAutoDetectTests' (10 passed); 'FullyQualifiedNameJuly2026ProtocolFallbackTests|FullyQualifiedName~HttpClientTransportTests' (34 passed)Related to #1848