Skip to content

Commit 9853e17

Browse files
halter73Copilot
halter73
and
Copilot
committed
Simplify server filter wiring and polish client transport fallback
Post-review cleanup pass over the draft-protocol plumbing. McpServerImpl: - Fold the built-in draft state-sync filter and the user incoming filters into a single PrependDraftStateSyncFilter(inner), removing the trivial ComposeFilters helper and the duplicate filter locals. - Drop the redundant AdoptProtocolVersionFromRequestContext helper and its two call sites; the state-sync filter already sets the negotiated protocol version before any handler runs. - Collapse the two complementary stateless-HTTP list-changed blocks into one if/else. Client transports: - Share a single TryReadJsonRpcErrorAsync helper between the streamable HTTP transport and the auto-detecting transport instead of duplicating the content-type check and body parse. - In the auto-detecting transport, throw the structured protocol exception directly once the streamable transport is adopted rather than stashing it and throwing after the try/catch. - Restore the fail-fast McpException when a server returns an InputRequiredResult carrying neither inputRequests nor requestState, instead of retrying the unchanged request until the retry cap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c5630fb commit 9853e17

6 files changed

Lines changed: 85 additions & 150 deletions

File tree

‎src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs‎

Lines changed: 5 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using ModelContextProtocol.Protocol;
44
using System.Net;
55
using System.Net.Http;
6-
using System.Text.Json;
76
using System.Threading.Channels;
87

98
namespace ModelContextProtocol.Client;
@@ -64,7 +63,6 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
6463
{
6564
// Try StreamableHttp first
6665
var streamableHttpTransport = new StreamableHttpClientSessionTransport(_name, _options, _httpClient, _messageChannel, _loggerFactory);
67-
McpProtocolException? structuredError = null;
6866

6967
try
7068
{
@@ -76,18 +74,19 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
7674
LogUsingStreamableHttp(_name);
7775
ActiveTransport = streamableHttpTransport;
7876
}
79-
else if (await TryGetJsonRpcErrorFromResponseAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError)
77+
else if (await StreamableHttpClientSessionTransport.TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError)
8078
{
8179
// A JSON-RPC error envelope in the body means the peer IS a Streamable HTTP server
8280
// — it just rejected our specific request (e.g., -32004 UnsupportedProtocolVersion,
8381
// -32003 MissingRequiredClientCapability, -32001 HeaderMismatch, or any other
8482
// application-level error). Don't fall back to SSE — that would mask the real signal
8583
// and surface a misleading "session id required" error from the SSE GET path.
86-
// Adopt the Streamable HTTP transport and surface the structured exception to the
87-
// caller so the connect-time fallback logic can react per spec PR #2844.
84+
// Adopt the Streamable HTTP transport and throw the structured exception so the
85+
// connect-time fallback logic can react per spec PR #2844. Setting ActiveTransport
86+
// first makes the catch filter below leave the now-owned transport alone.
8887
LogUsingStreamableHttp(_name);
8988
ActiveTransport = streamableHttpTransport;
90-
structuredError = McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
89+
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
9190
}
9291
else
9392
{
@@ -109,43 +108,6 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
109108
await streamableHttpTransport.DisposeAsync().ConfigureAwait(false);
110109
throw;
111110
}
112-
113-
if (structuredError is not null)
114-
{
115-
throw structuredError;
116-
}
117-
}
118-
119-
private static async Task<JsonRpcError?> TryGetJsonRpcErrorFromResponseAsync(HttpResponseMessage response, CancellationToken cancellationToken)
120-
{
121-
if (response.Content.Headers.ContentType?.MediaType != "application/json")
122-
{
123-
return null;
124-
}
125-
126-
string body;
127-
try
128-
{
129-
body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
130-
}
131-
catch
132-
{
133-
return null;
134-
}
135-
136-
if (string.IsNullOrEmpty(body))
137-
{
138-
return null;
139-
}
140-
141-
try
142-
{
143-
return JsonSerializer.Deserialize(body, McpJsonUtilities.JsonContext.Default.JsonRpcMessage) as JsonRpcError;
144-
}
145-
catch
146-
{
147-
return null;
148-
}
149111
}
150112

151113
private async Task InitializeSseTransportAsync(JsonRpcMessage message, CancellationToken cancellationToken)

‎src/ModelContextProtocol.Core/Client/McpClient.Methods.cs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,27 +1382,22 @@ public ValueTask<CancelTaskResult> CancelTaskAsync(
13821382

13831383
// Per SEP-2663 §51, the per-request opt-in uses the SEP-2575 capabilities envelope:
13841384
// _meta/io.modelcontextprotocol/clientCapabilities/extensions/io.modelcontextprotocol/tasks = {}
1385-
// TODO: replace the literal with a shared NotificationMethods.ClientCapabilitiesMetaKey once
1386-
// the SEP-2575 plumbing lands and drop the local consts.
1387-
private const string ClientCapabilitiesMetaKey = "io.modelcontextprotocol/clientCapabilities";
1388-
private const string ExtensionsKey = "extensions";
1389-
13901385
private static JsonObject GetMetaWithTaskCapability(JsonObject? existingMeta)
13911386
{
13921387
JsonObject meta = existingMeta is not null
13931388
? (JsonObject)existingMeta.DeepClone()
13941389
: [];
13951390

1396-
if (meta[ClientCapabilitiesMetaKey] is not JsonObject capsRoot)
1391+
if (meta[NotificationMethods.ClientCapabilitiesMetaKey] is not JsonObject capsRoot)
13971392
{
13981393
capsRoot = [];
1399-
meta[ClientCapabilitiesMetaKey] = capsRoot;
1394+
meta[NotificationMethods.ClientCapabilitiesMetaKey] = capsRoot;
14001395
}
14011396

1402-
if (capsRoot[ExtensionsKey] is not JsonObject extensionsRoot)
1397+
if (capsRoot[NotificationMethods.ClientCapabilityExtensionsKey] is not JsonObject extensionsRoot)
14031398
{
14041399
extensionsRoot = [];
1405-
capsRoot[ExtensionsKey] = extensionsRoot;
1400+
capsRoot[NotificationMethods.ClientCapabilityExtensionsKey] = extensionsRoot;
14061401
}
14071402

14081403
extensionsRoot.TryAdd(McpExtensions.Tasks, new JsonObject());

‎src/ModelContextProtocol.Core/Client/McpClientImpl.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,13 @@ request.Params is System.Text.Json.Nodes.JsonObject paramsObjForHeaders &&
677677
request = new JsonRpcRequest { Method = request.Method, Params = paramsObj, Context = request.Context };
678678
InjectDraftMetaIfNeeded(request);
679679
}
680+
else
681+
{
682+
// An input_required result carrying neither inputRequests nor requestState is
683+
// malformed: there is nothing to resolve and nothing to continue, so retrying the
684+
// unchanged request would just loop until maxRetries. Fail fast instead.
685+
throw new McpException("Server returned an InputRequiredResult without inputRequests or requestState.");
686+
}
680687

681688
continue; // retry with the updated request
682689
}

‎src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -77,25 +77,11 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation
7777
// The three modern draft-protocol error codes are also surfaced for non-400 status codes
7878
// for robustness — servers occasionally emit them with 4xx codes other than 400.
7979
if (!response.IsSuccessStatusCode &&
80-
response.Content.Headers.ContentType?.MediaType == "application/json")
80+
await TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError &&
81+
(response.StatusCode == HttpStatusCode.BadRequest ||
82+
IsModernDraftErrorCode((McpErrorCode)parsedError.Error.Code)))
8183
{
82-
string body;
83-
try
84-
{
85-
body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
86-
}
87-
catch
88-
{
89-
body = string.Empty;
90-
}
91-
92-
if (!string.IsNullOrEmpty(body) &&
93-
TryParseJsonRpcError(body, out var parsedError) &&
94-
(response.StatusCode == HttpStatusCode.BadRequest ||
95-
IsModernDraftErrorCode((McpErrorCode)parsedError.Error.Code)))
96-
{
97-
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
98-
}
84+
throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError);
9985
}
10086

10187
await response.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false);
@@ -106,24 +92,43 @@ code is McpErrorCode.UnsupportedProtocolVersion
10692
or McpErrorCode.MissingRequiredClientCapability
10793
or McpErrorCode.HeaderMismatch;
10894

109-
private static bool TryParseJsonRpcError(string body, out JsonRpcError parsedError)
95+
/// <summary>
96+
/// Reads a JSON-RPC error envelope from an <c>application/json</c> response body, returning
97+
/// <see langword="null"/> when the response isn't JSON, is empty, or doesn't parse to a
98+
/// <see cref="JsonRpcError"/>. Shared with the auto-detecting transport so it can tell an MCP
99+
/// server that rejected the request apart from a non-MCP endpoint without throwing.
100+
/// </summary>
101+
internal static async Task<JsonRpcError?> TryReadJsonRpcErrorAsync(HttpResponseMessage response, CancellationToken cancellationToken)
110102
{
103+
if (response.Content.Headers.ContentType?.MediaType != "application/json")
104+
{
105+
return null;
106+
}
107+
108+
string body;
111109
try
112110
{
113-
var message = JsonSerializer.Deserialize(body, McpJsonUtilities.JsonContext.Default.JsonRpcMessage);
114-
if (message is JsonRpcError rpcError)
115-
{
116-
parsedError = rpcError;
117-
return true;
118-
}
111+
body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
119112
}
120113
catch
121114
{
122-
// Not a valid JSON-RPC error response — fall through to the standard HTTP exception path.
115+
return null;
123116
}
124117

125-
parsedError = null!;
126-
return false;
118+
if (string.IsNullOrEmpty(body))
119+
{
120+
return null;
121+
}
122+
123+
try
124+
{
125+
return JsonSerializer.Deserialize(body, McpJsonUtilities.JsonContext.Default.JsonRpcMessage) as JsonRpcError;
126+
}
127+
catch
128+
{
129+
// Not a valid JSON-RPC error response — fall through to the standard HTTP exception path.
130+
return null;
131+
}
127132
}
128133

129134
// This is used by the auto transport so it can fall back and try SSE given a non-200 response without catching an exception.

‎src/ModelContextProtocol.Core/Protocol/NotificationMethods.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ public static class NotificationMethods
213213
/// </remarks>
214214
public const string ClientCapabilitiesMetaKey = "io.modelcontextprotocol/clientCapabilities";
215215

216+
/// <summary>
217+
/// The sub-key of <see cref="ClientCapabilitiesMetaKey"/> that holds per-request capability
218+
/// extensions (such as the tasks extension). Matches <see cref="Protocol.ClientCapabilities.Extensions"/>.
219+
/// </summary>
220+
public const string ClientCapabilityExtensionsKey = "extensions";
221+
216222
/// <summary>
217223
/// The metadata key used to specify the desired log level for a request's resulting log notifications.
218224
/// </summary>

0 commit comments

Comments
 (0)