From c629834f139d52d7fb50e719cb4085e9f0868276 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 15:47:36 +0000 Subject: [PATCH 01/16] Initial plan From 12d56839c9268cefef76a91b83bae17f323cbb94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 16:00:21 +0000 Subject: [PATCH 02/16] Add support for data to McpProtocolException in JSON-RPC errors Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 25 +++++++- .../Server/McpServerTests.cs | 60 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index fcd7980d9..dd567494d 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -186,10 +186,10 @@ ex is OperationCanceledException && { Code = (int)mcpProtocolException.ErrorCode, Message = mcpProtocolException.Message, + Data = ConvertExceptionData(mcpProtocolException.Data), } : ex is McpException mcpException ? new() { - Code = (int)McpErrorCode.InternalError, Message = mcpException.Message, } : @@ -769,6 +769,29 @@ private static TimeSpan GetElapsed(long startingTimestamp) => return null; } + /// + /// Converts the Exception.Data dictionary to a serializable Dictionary<string, object?>. + /// Returns null if the data dictionary is empty. + /// + private static Dictionary? ConvertExceptionData(System.Collections.IDictionary data) + { + if (data.Count == 0) + { + return null; + } + + var result = new Dictionary(data.Count); + foreach (System.Collections.DictionaryEntry entry in data) + { + if (entry.Key is string key) + { + result[key] = entry.Value; + } + } + + return result.Count > 0 ? result : null; + } + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} message processing canceled.")] private partial void LogEndpointMessageProcessingCanceled(string endpointName); diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index ab2537b6b..75248e99f 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -671,6 +671,66 @@ await transport.SendMessageAsync( await runTask; } + [Fact] + public async Task Can_Handle_Call_Tool_Requests_With_McpProtocolException_And_Data() + { + const string errorMessage = "Resource not found"; + const McpErrorCode errorCode = (McpErrorCode)(-32002); + const string resourceUri = "file:///path/to/resource"; + + await using var transport = new TestServerTransport(); + var options = CreateOptions(new ServerCapabilities { Tools = new() }); + options.Handlers.CallToolHandler = async (request, ct) => + { + throw new McpProtocolException(errorMessage, errorCode) + { + Data = + { + { "uri", resourceUri } + } + }; + }; + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + + await using var server = McpServer.Create(transport, options, LoggerFactory); + + var runTask = server.RunAsync(TestContext.Current.CancellationToken); + + var receivedMessage = new TaskCompletionSource(); + + transport.OnMessageSent = (message) => + { + if (message is JsonRpcError error && error.Id.ToString() == "55") + receivedMessage.SetResult(error); + }; + + await transport.SendMessageAsync( + new JsonRpcRequest + { + Method = RequestMethods.ToolsCall, + Id = new RequestId(55) + }, + TestContext.Current.CancellationToken + ); + + var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + Assert.NotNull(error); + Assert.NotNull(error.Error); + Assert.Equal((int)errorCode, error.Error.Code); + Assert.Equal(errorMessage, error.Error.Message); + Assert.NotNull(error.Error.Data); + + // Verify the data contains the uri + var dataJson = JsonSerializer.Serialize(error.Error.Data, McpJsonUtilities.DefaultOptions); + var dataObject = JsonSerializer.Deserialize(dataJson, McpJsonUtilities.DefaultOptions); + Assert.NotNull(dataObject); + Assert.True(dataObject.ContainsKey("uri")); + Assert.Equal(resourceUri, dataObject["uri"]?.GetValue()); + + await transport.DisposeAsync(); + await runTask; + } + private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action? configureOptions, Action assertResult) { await using var transport = new TestServerTransport(); From 64f2c58a70d0e5266e86ba186b8de39319a1050d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 16:08:11 +0000 Subject: [PATCH 03/16] Address code review feedback Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- src/ModelContextProtocol.Core/McpSessionHandler.cs | 5 ++++- tests/ModelContextProtocol.Tests/Server/McpServerTests.cs | 8 +++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index dd567494d..f072f4f1b 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -771,8 +771,11 @@ private static TimeSpan GetElapsed(long startingTimestamp) => /// /// Converts the Exception.Data dictionary to a serializable Dictionary<string, object?>. - /// Returns null if the data dictionary is empty. + /// Returns null if the data dictionary is empty or contains no string keys. /// + /// + /// Only entries with string keys are included in the result. Entries with non-string keys are ignored. + /// private static Dictionary? ConvertExceptionData(System.Collections.IDictionary data) { if (data.Count == 0) diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index 75248e99f..6c095bb60 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -721,11 +721,9 @@ await transport.SendMessageAsync( Assert.NotNull(error.Error.Data); // Verify the data contains the uri - var dataJson = JsonSerializer.Serialize(error.Error.Data, McpJsonUtilities.DefaultOptions); - var dataObject = JsonSerializer.Deserialize(dataJson, McpJsonUtilities.DefaultOptions); - Assert.NotNull(dataObject); - Assert.True(dataObject.ContainsKey("uri")); - Assert.Equal(resourceUri, dataObject["uri"]?.GetValue()); + var dataDict = Assert.IsType>(error.Error.Data); + Assert.True(dataDict.ContainsKey("uri")); + Assert.Equal(resourceUri, dataDict["uri"]); await transport.DisposeAsync(); await runTask; From 9b8cf68960c4a029acb59015608cc862d5676fae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 16:16:32 +0000 Subject: [PATCH 04/16] Use PascalCasing for const variables in test Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Server/McpServerTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index 6c095bb60..b1875d786 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -674,19 +674,19 @@ await transport.SendMessageAsync( [Fact] public async Task Can_Handle_Call_Tool_Requests_With_McpProtocolException_And_Data() { - const string errorMessage = "Resource not found"; - const McpErrorCode errorCode = (McpErrorCode)(-32002); - const string resourceUri = "file:///path/to/resource"; + const string ErrorMessage = "Resource not found"; + const McpErrorCode ErrorCode = (McpErrorCode)(-32002); + const string ResourceUri = "file:///path/to/resource"; await using var transport = new TestServerTransport(); var options = CreateOptions(new ServerCapabilities { Tools = new() }); options.Handlers.CallToolHandler = async (request, ct) => { - throw new McpProtocolException(errorMessage, errorCode) + throw new McpProtocolException(ErrorMessage, ErrorCode) { Data = { - { "uri", resourceUri } + { "uri", ResourceUri } } }; }; @@ -716,14 +716,14 @@ await transport.SendMessageAsync( var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); Assert.NotNull(error); Assert.NotNull(error.Error); - Assert.Equal((int)errorCode, error.Error.Code); - Assert.Equal(errorMessage, error.Error.Message); + Assert.Equal((int)ErrorCode, error.Error.Code); + Assert.Equal(ErrorMessage, error.Error.Message); Assert.NotNull(error.Error.Data); // Verify the data contains the uri var dataDict = Assert.IsType>(error.Error.Data); Assert.True(dataDict.ContainsKey("uri")); - Assert.Equal(resourceUri, dataDict["uri"]); + Assert.Equal(ResourceUri, dataDict["uri"]); await transport.DisposeAsync(); await runTask; From 580743d1277c7bd3bfa51f231d25998f517d7db1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 17:04:34 +0000 Subject: [PATCH 05/16] Add test for non-serializable Exception.Data and fix exception handling Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 13 ++- .../Server/McpServerTests.cs | 89 +++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index f072f4f1b..6ff54d2be 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -206,7 +206,18 @@ ex is OperationCanceledException && Error = detail, Context = new JsonRpcMessageContext { RelatedTransport = request.Context?.RelatedTransport }, }; - await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); + + try + { + await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); + } + catch (Exception sendException) when ((sendException is JsonException || sendException is NotSupportedException) && detail.Data is not null) + { + // If serialization fails (e.g., non-serializable data in Exception.Data), + // retry without the data to ensure the client receives an error response. + detail.Data = null; + await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); + } } else if (ex is not OperationCanceledException) { diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index b1875d786..36373186e 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -6,6 +6,7 @@ using System.Runtime.InteropServices; using System.Text.Json; using System.Text.Json.Nodes; +using System.Threading.Channels; namespace ModelContextProtocol.Tests.Server; @@ -729,6 +730,94 @@ await transport.SendMessageAsync( await runTask; } + [Fact] + public async Task Can_Handle_Call_Tool_Requests_With_McpProtocolException_And_NonSerializableData() + { + const string ErrorMessage = "Resource not found"; + const McpErrorCode ErrorCode = (McpErrorCode)(-32002); + + await using var transport = new SerializingTestServerTransport(); + var options = CreateOptions(new ServerCapabilities { Tools = new() }); + options.Handlers.CallToolHandler = async (request, ct) => + { + throw new McpProtocolException(ErrorMessage, ErrorCode) + { + Data = + { + // Add a non-serializable object (an object with circular reference) + { "nonSerializable", new NonSerializableObject() } + } + }; + }; + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + + await using var server = McpServer.Create(transport, options, LoggerFactory); + + var runTask = server.RunAsync(TestContext.Current.CancellationToken); + + var receivedMessage = new TaskCompletionSource(); + + transport.OnMessageSent = (message) => + { + if (message is JsonRpcError error && error.Id.ToString() == "55") + receivedMessage.SetResult(error); + }; + + await transport.SendMessageAsync( + new JsonRpcRequest + { + Method = RequestMethods.ToolsCall, + Id = new RequestId(55) + }, + TestContext.Current.CancellationToken + ); + + // Client should still receive an error response, even though the data couldn't be serialized + var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + Assert.NotNull(error); + Assert.NotNull(error.Error); + Assert.Equal((int)ErrorCode, error.Error.Code); + Assert.Equal(ErrorMessage, error.Error.Message); + // Data should be null since it couldn't be serialized + Assert.Null(error.Error.Data); + + await transport.DisposeAsync(); + await runTask; + } + + /// + /// A class that cannot be serialized by System.Text.Json due to circular reference. + /// + private sealed class NonSerializableObject + { + public NonSerializableObject() => Self = this; + public NonSerializableObject Self { get; set; } + } + + /// + /// A test transport that simulates JSON serialization failure for non-serializable data. + /// + private sealed class SerializingTestServerTransport : ITransport + { + private readonly TestServerTransport _inner = new(); + + public bool IsConnected => _inner.IsConnected; + public ChannelReader MessageReader => _inner.MessageReader; + public string? SessionId => _inner.SessionId; + public Action? OnMessageSent { get => _inner.OnMessageSent; set => _inner.OnMessageSent = value; } + + public Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default) + { + // Serialize the message to verify it can be serialized (this will throw JsonException if not) + // We serialize synchronously before any async operations to ensure the exception propagates correctly + _ = JsonSerializer.Serialize(message, McpJsonUtilities.DefaultOptions); + + return _inner.SendMessageAsync(message, cancellationToken); + } + + public ValueTask DisposeAsync() => _inner.DisposeAsync(); + } + private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action? configureOptions, Action assertResult) { await using var transport = new TestServerTransport(); From adcffc6927dea79d7c3f2ae8acab7dc3dd2b6b03 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:59:56 +0000 Subject: [PATCH 06/16] Address PR feedback: improve catch syntax and fix XML doc Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- src/ModelContextProtocol.Core/McpSessionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 6ff54d2be..aac07344a 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -211,7 +211,7 @@ ex is OperationCanceledException && { await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); } - catch (Exception sendException) when ((sendException is JsonException || sendException is NotSupportedException) && detail.Data is not null) + catch (Exception sendException) when (sendException is JsonException or NotSupportedException && detail.Data is not null) { // If serialization fails (e.g., non-serializable data in Exception.Data), // retry without the data to ensure the client receives an error response. @@ -781,7 +781,7 @@ private static TimeSpan GetElapsed(long startingTimestamp) => } /// - /// Converts the Exception.Data dictionary to a serializable Dictionary<string, object?>. + /// Converts the dictionary to a serializable . /// Returns null if the data dictionary is empty or contains no string keys. /// /// From 688a956b53d29178c9c658b86bd13d1295db94a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 22:22:12 +0000 Subject: [PATCH 07/16] Serialize Exception.Data upfront to prevent stream corruption Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 44 ++++--- .../McpProtocolExceptionDataTests.cs | 113 ++++++++++++++++++ .../Server/McpServerTests.cs | 95 +-------------- 3 files changed, 144 insertions(+), 108 deletions(-) create mode 100644 tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index aac07344a..b85e5776e 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -207,17 +207,7 @@ ex is OperationCanceledException && Context = new JsonRpcMessageContext { RelatedTransport = request.Context?.RelatedTransport }, }; - try - { - await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); - } - catch (Exception sendException) when (sendException is JsonException or NotSupportedException && detail.Data is not null) - { - // If serialization fails (e.g., non-serializable data in Exception.Data), - // retry without the data to ensure the client receives an error response. - detail.Data = null; - await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); - } + await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); } else if (ex is not OperationCanceledException) { @@ -782,28 +772,50 @@ private static TimeSpan GetElapsed(long startingTimestamp) => /// /// Converts the dictionary to a serializable . - /// Returns null if the data dictionary is empty or contains no string keys. + /// Returns null if the data dictionary is empty or contains no string keys with serializable values. /// /// + /// /// Only entries with string keys are included in the result. Entries with non-string keys are ignored. + /// + /// + /// Each value is serialized to a to ensure it can be safely included in the + /// JSON-RPC error response. Values that cannot be serialized are silently skipped. + /// /// - private static Dictionary? ConvertExceptionData(System.Collections.IDictionary data) + private static Dictionary? ConvertExceptionData(System.Collections.IDictionary data) { if (data.Count == 0) { return null; } - var result = new Dictionary(data.Count); + var typeInfo = McpJsonUtilities.DefaultOptions.GetTypeInfo(); + + Dictionary? result = null; foreach (System.Collections.DictionaryEntry entry in data) { if (entry.Key is string key) { - result[key] = entry.Value; + try + { + // Serialize each value upfront to catch any serialization issues + // before attempting to send the message. If the value is already a + // JsonElement, use it directly. + var element = entry.Value is JsonElement je + ? je + : JsonSerializer.SerializeToElement(entry.Value, typeInfo); + result ??= new(data.Count); + result[key] = element; + } + catch (Exception ex) when (ex is JsonException or NotSupportedException) + { + // Skip non-serializable values silently + } } } - return result.Count > 0 ? result : null; + return result?.Count > 0 ? result : null; } [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} message processing canceled.")] diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs new file mode 100644 index 000000000..063426239 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -0,0 +1,113 @@ +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using ModelContextProtocol.Tests.Utils; + +namespace ModelContextProtocol.Tests; + +/// +/// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. +/// +public class McpProtocolExceptionDataTests : ClientServerTestBase +{ + public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper) + : base(testOutputHelper) + { + } + + protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder) + { + mcpServerBuilder.WithCallToolHandler((request, cancellationToken) => + { + var toolName = request.Params?.Name; + + switch (toolName) + { + case "throw_with_serializable_data": + throw new McpProtocolException("Resource not found", (McpErrorCode)(-32002)) + { + Data = + { + { "uri", "file:///path/to/resource" }, + { "code", 404 } + } + }; + + case "throw_with_nonserializable_data": + throw new McpProtocolException("Resource not found", (McpErrorCode)(-32002)) + { + Data = + { + // Circular reference - cannot be serialized + { "nonSerializable", new NonSerializableObject() }, + // This one should still be included + { "uri", "file:///path/to/resource" } + } + }; + + case "throw_with_only_nonserializable_data": + throw new McpProtocolException("Resource not found", (McpErrorCode)(-32002)) + { + Data = + { + // Only non-serializable data - should result in null data + { "nonSerializable", new NonSerializableObject() } + } + }; + + default: + throw new McpProtocolException($"Unknown tool: '{toolName}'", McpErrorCode.InvalidParams); + } + }); + } + + [Fact] + public async Task Exception_With_Serializable_Data_Propagates_To_Client() + { + await using McpClient client = await CreateMcpClientForServer(); + + var exception = await Assert.ThrowsAsync(async () => + await client.CallToolAsync("throw_with_serializable_data", cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal("Request failed (remote): Resource not found", exception.Message); + Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + } + + [Fact] + public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_Client() + { + await using McpClient client = await CreateMcpClientForServer(); + + // The tool throws McpProtocolException with non-serializable data in Exception.Data. + // The server should still send a proper error response to the client, with non-serializable + // values filtered out. + var exception = await Assert.ThrowsAsync(async () => + await client.CallToolAsync("throw_with_nonserializable_data", cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal("Request failed (remote): Resource not found", exception.Message); + Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + } + + [Fact] + public async Task Exception_With_Only_NonSerializable_Data_Still_Propagates_Error_To_Client() + { + await using McpClient client = await CreateMcpClientForServer(); + + // When all data is non-serializable, the error should still be sent (with null data) + var exception = await Assert.ThrowsAsync(async () => + await client.CallToolAsync("throw_with_only_nonserializable_data", cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal("Request failed (remote): Resource not found", exception.Message); + Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + } + + /// + /// A class that cannot be serialized by System.Text.Json due to circular reference. + /// + private sealed class NonSerializableObject + { + public NonSerializableObject() => Self = this; + public NonSerializableObject Self { get; set; } + } +} diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index 36373186e..8ca4709bc 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -6,7 +6,6 @@ using System.Runtime.InteropServices; using System.Text.Json; using System.Text.Json.Nodes; -using System.Threading.Channels; namespace ModelContextProtocol.Tests.Server; @@ -721,103 +720,15 @@ await transport.SendMessageAsync( Assert.Equal(ErrorMessage, error.Error.Message); Assert.NotNull(error.Error.Data); - // Verify the data contains the uri - var dataDict = Assert.IsType>(error.Error.Data); + // Verify the data contains the uri (values are now JsonElements after serialization) + var dataDict = Assert.IsType>(error.Error.Data); Assert.True(dataDict.ContainsKey("uri")); - Assert.Equal(ResourceUri, dataDict["uri"]); + Assert.Equal(ResourceUri, dataDict["uri"].GetString()); await transport.DisposeAsync(); await runTask; } - [Fact] - public async Task Can_Handle_Call_Tool_Requests_With_McpProtocolException_And_NonSerializableData() - { - const string ErrorMessage = "Resource not found"; - const McpErrorCode ErrorCode = (McpErrorCode)(-32002); - - await using var transport = new SerializingTestServerTransport(); - var options = CreateOptions(new ServerCapabilities { Tools = new() }); - options.Handlers.CallToolHandler = async (request, ct) => - { - throw new McpProtocolException(ErrorMessage, ErrorCode) - { - Data = - { - // Add a non-serializable object (an object with circular reference) - { "nonSerializable", new NonSerializableObject() } - } - }; - }; - options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); - - await using var server = McpServer.Create(transport, options, LoggerFactory); - - var runTask = server.RunAsync(TestContext.Current.CancellationToken); - - var receivedMessage = new TaskCompletionSource(); - - transport.OnMessageSent = (message) => - { - if (message is JsonRpcError error && error.Id.ToString() == "55") - receivedMessage.SetResult(error); - }; - - await transport.SendMessageAsync( - new JsonRpcRequest - { - Method = RequestMethods.ToolsCall, - Id = new RequestId(55) - }, - TestContext.Current.CancellationToken - ); - - // Client should still receive an error response, even though the data couldn't be serialized - var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); - Assert.NotNull(error); - Assert.NotNull(error.Error); - Assert.Equal((int)ErrorCode, error.Error.Code); - Assert.Equal(ErrorMessage, error.Error.Message); - // Data should be null since it couldn't be serialized - Assert.Null(error.Error.Data); - - await transport.DisposeAsync(); - await runTask; - } - - /// - /// A class that cannot be serialized by System.Text.Json due to circular reference. - /// - private sealed class NonSerializableObject - { - public NonSerializableObject() => Self = this; - public NonSerializableObject Self { get; set; } - } - - /// - /// A test transport that simulates JSON serialization failure for non-serializable data. - /// - private sealed class SerializingTestServerTransport : ITransport - { - private readonly TestServerTransport _inner = new(); - - public bool IsConnected => _inner.IsConnected; - public ChannelReader MessageReader => _inner.MessageReader; - public string? SessionId => _inner.SessionId; - public Action? OnMessageSent { get => _inner.OnMessageSent; set => _inner.OnMessageSent = value; } - - public Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default) - { - // Serialize the message to verify it can be serialized (this will throw JsonException if not) - // We serialize synchronously before any async operations to ensure the exception propagates correctly - _ = JsonSerializer.Serialize(message, McpJsonUtilities.DefaultOptions); - - return _inner.SendMessageAsync(message, cancellationToken); - } - - public ValueTask DisposeAsync() => _inner.DisposeAsync(); - } - private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action? configureOptions, Action assertResult) { await using var transport = new TestServerTransport(); From f34c30269f567db49400ec7518041566482a8559 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 22:45:34 +0000 Subject: [PATCH 08/16] Deserialize error data and populate exception.Data on client Co-authored-by: halter73 <54385+halter73@users.noreply.github.com> --- .../McpSessionHandler.cs | 24 +++++++++- .../McpProtocolExceptionDataTests.cs | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index b85e5776e..5517dc03e 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -453,7 +453,29 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc if (response is JsonRpcError error) { LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); - throw new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); + var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); + + // Populate exception.Data with the error data if present + if (error.Error.Data is not null) + { + // The data could be a Dictionary or a JsonElement containing an object + if (error.Error.Data is Dictionary dataDict) + { + foreach (var kvp in dataDict) + { + exception.Data[kvp.Key] = kvp.Value; + } + } + else if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) + { + foreach (var property in jsonElement.EnumerateObject()) + { + exception.Data[property.Name] = property.Value; + } + } + } + + throw exception; } if (response is JsonRpcResponse success) diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index 063426239..05f004485 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -3,6 +3,7 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using ModelContextProtocol.Tests.Utils; +using System.Text.Json; namespace ModelContextProtocol.Tests; @@ -72,6 +73,28 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Verify the data was propagated to the exception + // The Data collection should contain the expected keys + var hasUri = false; + var hasCode = false; + foreach (System.Collections.DictionaryEntry entry in exception.Data) + { + if (entry.Key is string key) + { + if (key == "uri") hasUri = true; + if (key == "code") hasCode = true; + } + } + Assert.True(hasUri, "Exception.Data should contain 'uri' key"); + Assert.True(hasCode, "Exception.Data should contain 'code' key"); + + // Verify the values (they should be JsonElements) + var uriValue = Assert.IsType(exception.Data["uri"]); + Assert.Equal("file:///path/to/resource", uriValue.GetString()); + + var codeValue = Assert.IsType(exception.Data["code"]); + Assert.Equal(404, codeValue.GetInt32()); } [Fact] @@ -87,6 +110,23 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Verify that only the serializable data was propagated (non-serializable was filtered out) + var hasUri = false; + var hasNonSerializable = false; + foreach (System.Collections.DictionaryEntry entry in exception.Data) + { + if (entry.Key is string key) + { + if (key == "uri") hasUri = true; + if (key == "nonSerializable") hasNonSerializable = true; + } + } + Assert.True(hasUri, "Exception.Data should contain 'uri' key"); + Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key"); + + var uriValue = Assert.IsType(exception.Data["uri"]); + Assert.Equal("file:///path/to/resource", uriValue.GetString()); } [Fact] @@ -100,6 +140,10 @@ public async Task Exception_With_Only_NonSerializable_Data_Still_Propagates_Erro Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // When all data is non-serializable, the Data collection should be empty + // (the server's ConvertExceptionData returns null when no serializable data exists) + Assert.Empty(exception.Data); } /// From c4d150034ebe75d332d55fbb64d709f925fa469f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 23:01:09 +0000 Subject: [PATCH 09/16] Simplify error data deserialization to handle only JsonElement Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 5517dc03e..cf2f1a93f 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -455,23 +455,13 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); - // Populate exception.Data with the error data if present - if (error.Error.Data is not null) + // Populate exception.Data with the error data if present. + // When deserializing JSON, Data will be a JsonElement. + if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) { - // The data could be a Dictionary or a JsonElement containing an object - if (error.Error.Data is Dictionary dataDict) + foreach (var property in jsonElement.EnumerateObject()) { - foreach (var kvp in dataDict) - { - exception.Data[kvp.Key] = kvp.Value; - } - } - else if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) - { - foreach (var property in jsonElement.EnumerateObject()) - { - exception.Data[property.Name] = property.Value; - } + exception.Data[property.Name] = property.Value; } } From 23d25d8dff7826294ad50a4b3e8aa2475fdd7aa9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 03:57:31 +0000 Subject: [PATCH 10/16] Fix .NET Framework compatibility for Exception.Data with JsonElement Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 11 +++++++- .../McpProtocolExceptionDataTests.cs | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index cf2f1a93f..215e7571b 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -461,7 +461,16 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc { foreach (var property in jsonElement.EnumerateObject()) { - exception.Data[property.Name] = property.Value; + try + { + exception.Data[property.Name] = property.Value; + } + catch (ArgumentException) + { + // On .NET Framework, Exception.Data uses ListDictionaryInternal which requires + // values to be serializable. JsonElement is not marked as [Serializable], so + // setting it throws ArgumentException. We silently skip such entries. + } } } diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index 05f004485..532674d1c 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -10,6 +10,11 @@ namespace ModelContextProtocol.Tests; /// /// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. /// +/// +/// Note: On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. +/// Since JsonElement is not marked as serializable, the data population on the client side is skipped +/// on that platform. These tests verify the error message and code are correct regardless of platform. +/// public class McpProtocolExceptionDataTests : ClientServerTestBase { public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper) @@ -63,6 +68,15 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer }); } + // On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. + // JsonElement is not marked as serializable, so these tests are skipped on .NET Framework. + private static bool IsNetFramework => +#if NET + false; +#else + true; +#endif + [Fact] public async Task Exception_With_Serializable_Data_Propagates_To_Client() { @@ -73,6 +87,12 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data + if (IsNetFramework) + { + return; + } // Verify the data was propagated to the exception // The Data collection should contain the expected keys @@ -110,6 +130,12 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data + if (IsNetFramework) + { + return; + } // Verify that only the serializable data was propagated (non-serializable was filtered out) var hasUri = false; From 32094149320f9198b80a10c1ae662ce2c575941b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 04:04:58 +0000 Subject: [PATCH 11/16] Use PlatformDetection class for .NET Framework detection Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpProtocolExceptionDataTests.cs | 13 ++----------- .../ModelContextProtocol.Tests/PlatformDetection.cs | 9 +++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index 532674d1c..55795c7cb 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -68,15 +68,6 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer }); } - // On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. - // JsonElement is not marked as serializable, so these tests are skipped on .NET Framework. - private static bool IsNetFramework => -#if NET - false; -#else - true; -#endif - [Fact] public async Task Exception_With_Serializable_Data_Propagates_To_Client() { @@ -89,7 +80,7 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (IsNetFramework) + if (PlatformDetection.IsNetFramework) { return; } @@ -132,7 +123,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (IsNetFramework) + if (PlatformDetection.IsNetFramework) { return; } diff --git a/tests/ModelContextProtocol.Tests/PlatformDetection.cs b/tests/ModelContextProtocol.Tests/PlatformDetection.cs index f439147ff..594ccf40b 100644 --- a/tests/ModelContextProtocol.Tests/PlatformDetection.cs +++ b/tests/ModelContextProtocol.Tests/PlatformDetection.cs @@ -6,4 +6,13 @@ internal static class PlatformDetection { public static bool IsMonoRuntime { get; } = Type.GetType("Mono.Runtime") is not null; public static bool IsWindows { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + // On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. + // JsonElement is not marked as serializable, so certain features are not available on that platform. + public static bool IsNetFramework { get; } = +#if NET + false; +#else + true; +#endif } \ No newline at end of file From c8f8a276d45ab4a5552d6278ecd24afdd7e6444c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 04:18:54 +0000 Subject: [PATCH 12/16] Use #if NET to compile out error data population on .NET Framework Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 215e7571b..5cf5d8fe4 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -454,25 +454,20 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc { LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); - + +#if NET // Populate exception.Data with the error data if present. // When deserializing JSON, Data will be a JsonElement. + // Note: This is not supported on .NET Framework because Exception.Data uses ListDictionaryInternal + // which requires values to be marked with [Serializable], and JsonElement is not serializable. if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) { foreach (var property in jsonElement.EnumerateObject()) { - try - { - exception.Data[property.Name] = property.Value; - } - catch (ArgumentException) - { - // On .NET Framework, Exception.Data uses ListDictionaryInternal which requires - // values to be serializable. JsonElement is not marked as [Serializable], so - // setting it throws ArgumentException. We silently skip such entries. - } + exception.Data[property.Name] = property.Value; } } +#endif throw exception; } From f757558ba2867874dabb61c2f70d78833af79e9e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 13:36:53 +0000 Subject: [PATCH 13/16] Make Exception.Data work on .NET Framework using SerializableJsonElement wrapper Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 8 ++-- .../Protocol/SerializableJsonElement.cs | 48 +++++++++++++++++++ .../McpProtocolExceptionDataTests.cs | 39 +++++++-------- .../PlatformDetection.cs | 9 ---- 4 files changed, 69 insertions(+), 35 deletions(-) create mode 100644 src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 5cf5d8fe4..2b350da85 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -455,19 +455,17 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); -#if NET // Populate exception.Data with the error data if present. // When deserializing JSON, Data will be a JsonElement. - // Note: This is not supported on .NET Framework because Exception.Data uses ListDictionaryInternal - // which requires values to be marked with [Serializable], and JsonElement is not serializable. if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) { foreach (var property in jsonElement.EnumerateObject()) { - exception.Data[property.Name] = property.Value; + // On .NET Framework, Exception.Data requires values to be [Serializable]. + // JsonElement is not serializable, so we wrap it in a serializable container. + exception.Data[property.Name] = SerializableJsonElement.Wrap(property.Value); } } -#endif throw exception; } diff --git a/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs b/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs new file mode 100644 index 000000000..3959bac44 --- /dev/null +++ b/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs @@ -0,0 +1,48 @@ +using System.Text.Json; + +namespace ModelContextProtocol.Protocol; + +/// +/// A serializable wrapper for that can be stored in +/// on .NET Framework, where values must be marked with . +/// +/// +/// On .NET Core/.NET 5+, can be stored directly in , +/// but on .NET Framework the underlying ListDictionaryInternal requires values to be serializable. +/// This wrapper stores the JSON as a string and deserializes it back to a on demand. +/// +[Serializable] +public sealed class SerializableJsonElement +{ + private readonly string _json; + + private SerializableJsonElement(string json) + { + _json = json; + } + + /// + /// Gets the value. + /// + public JsonElement Value => JsonDocument.Parse(_json).RootElement; + + /// + /// Creates a serializable wrapper for the specified . + /// + /// The JSON element to wrap. + /// + /// On .NET Core/.NET 5+, returns the directly. + /// On .NET Framework, returns a wrapper. + /// + internal static object Wrap(JsonElement element) + { +#if NET + return element; +#else + return new SerializableJsonElement(element.GetRawText()); +#endif + } + + /// + public override string ToString() => _json; +} diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index 55795c7cb..c869c7a9d 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -10,11 +10,6 @@ namespace ModelContextProtocol.Tests; /// /// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. /// -/// -/// Note: On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. -/// Since JsonElement is not marked as serializable, the data population on the client side is skipped -/// on that platform. These tests verify the error message and code are correct regardless of platform. -/// public class McpProtocolExceptionDataTests : ClientServerTestBase { public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper) @@ -68,6 +63,20 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer }); } + /// + /// Gets the JsonElement from an Exception.Data value, handling both direct JsonElement (on .NET Core) + /// and SerializableJsonElement wrapper (on .NET Framework). + /// + private static JsonElement GetJsonElement(object? value) + { + return value switch + { + JsonElement je => je, + SerializableJsonElement sje => sje.Value, + _ => throw new InvalidOperationException($"Expected JsonElement or SerializableJsonElement, got {value?.GetType().Name ?? "null"}") + }; + } + [Fact] public async Task Exception_With_Serializable_Data_Propagates_To_Client() { @@ -78,12 +87,6 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); - - // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (PlatformDetection.IsNetFramework) - { - return; - } // Verify the data was propagated to the exception // The Data collection should contain the expected keys @@ -100,11 +103,11 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.True(hasCode, "Exception.Data should contain 'code' key"); - // Verify the values (they should be JsonElements) - var uriValue = Assert.IsType(exception.Data["uri"]); + // Verify the values (they are JsonElements on .NET Core, SerializableJsonElement on .NET Framework) + var uriValue = GetJsonElement(exception.Data["uri"]); Assert.Equal("file:///path/to/resource", uriValue.GetString()); - var codeValue = Assert.IsType(exception.Data["code"]); + var codeValue = GetJsonElement(exception.Data["code"]); Assert.Equal(404, codeValue.GetInt32()); } @@ -121,12 +124,6 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); - - // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (PlatformDetection.IsNetFramework) - { - return; - } // Verify that only the serializable data was propagated (non-serializable was filtered out) var hasUri = false; @@ -142,7 +139,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key"); - var uriValue = Assert.IsType(exception.Data["uri"]); + var uriValue = GetJsonElement(exception.Data["uri"]); Assert.Equal("file:///path/to/resource", uriValue.GetString()); } diff --git a/tests/ModelContextProtocol.Tests/PlatformDetection.cs b/tests/ModelContextProtocol.Tests/PlatformDetection.cs index 594ccf40b..f439147ff 100644 --- a/tests/ModelContextProtocol.Tests/PlatformDetection.cs +++ b/tests/ModelContextProtocol.Tests/PlatformDetection.cs @@ -6,13 +6,4 @@ internal static class PlatformDetection { public static bool IsMonoRuntime { get; } = Type.GetType("Mono.Runtime") is not null; public static bool IsWindows { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); - - // On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. - // JsonElement is not marked as serializable, so certain features are not available on that platform. - public static bool IsNetFramework { get; } = -#if NET - false; -#else - true; -#endif } \ No newline at end of file From 3082e35d3931ded6e56d1b5c94a32a2a878740d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 20:48:28 +0000 Subject: [PATCH 14/16] Revert SerializableJsonElement wrapper, restore #if NET approach Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 8 ++-- .../Protocol/SerializableJsonElement.cs | 48 ------------------- .../McpProtocolExceptionDataTests.cs | 39 ++++++++------- .../PlatformDetection.cs | 9 ++++ 4 files changed, 35 insertions(+), 69 deletions(-) delete mode 100644 src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 2b350da85..5cf5d8fe4 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -455,17 +455,19 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); +#if NET // Populate exception.Data with the error data if present. // When deserializing JSON, Data will be a JsonElement. + // Note: This is not supported on .NET Framework because Exception.Data uses ListDictionaryInternal + // which requires values to be marked with [Serializable], and JsonElement is not serializable. if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) { foreach (var property in jsonElement.EnumerateObject()) { - // On .NET Framework, Exception.Data requires values to be [Serializable]. - // JsonElement is not serializable, so we wrap it in a serializable container. - exception.Data[property.Name] = SerializableJsonElement.Wrap(property.Value); + exception.Data[property.Name] = property.Value; } } +#endif throw exception; } diff --git a/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs b/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs deleted file mode 100644 index 3959bac44..000000000 --- a/src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System.Text.Json; - -namespace ModelContextProtocol.Protocol; - -/// -/// A serializable wrapper for that can be stored in -/// on .NET Framework, where values must be marked with . -/// -/// -/// On .NET Core/.NET 5+, can be stored directly in , -/// but on .NET Framework the underlying ListDictionaryInternal requires values to be serializable. -/// This wrapper stores the JSON as a string and deserializes it back to a on demand. -/// -[Serializable] -public sealed class SerializableJsonElement -{ - private readonly string _json; - - private SerializableJsonElement(string json) - { - _json = json; - } - - /// - /// Gets the value. - /// - public JsonElement Value => JsonDocument.Parse(_json).RootElement; - - /// - /// Creates a serializable wrapper for the specified . - /// - /// The JSON element to wrap. - /// - /// On .NET Core/.NET 5+, returns the directly. - /// On .NET Framework, returns a wrapper. - /// - internal static object Wrap(JsonElement element) - { -#if NET - return element; -#else - return new SerializableJsonElement(element.GetRawText()); -#endif - } - - /// - public override string ToString() => _json; -} diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index c869c7a9d..55795c7cb 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -10,6 +10,11 @@ namespace ModelContextProtocol.Tests; /// /// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. /// +/// +/// Note: On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. +/// Since JsonElement is not marked as serializable, the data population on the client side is skipped +/// on that platform. These tests verify the error message and code are correct regardless of platform. +/// public class McpProtocolExceptionDataTests : ClientServerTestBase { public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper) @@ -63,20 +68,6 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer }); } - /// - /// Gets the JsonElement from an Exception.Data value, handling both direct JsonElement (on .NET Core) - /// and SerializableJsonElement wrapper (on .NET Framework). - /// - private static JsonElement GetJsonElement(object? value) - { - return value switch - { - JsonElement je => je, - SerializableJsonElement sje => sje.Value, - _ => throw new InvalidOperationException($"Expected JsonElement or SerializableJsonElement, got {value?.GetType().Name ?? "null"}") - }; - } - [Fact] public async Task Exception_With_Serializable_Data_Propagates_To_Client() { @@ -87,6 +78,12 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data + if (PlatformDetection.IsNetFramework) + { + return; + } // Verify the data was propagated to the exception // The Data collection should contain the expected keys @@ -103,11 +100,11 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.True(hasCode, "Exception.Data should contain 'code' key"); - // Verify the values (they are JsonElements on .NET Core, SerializableJsonElement on .NET Framework) - var uriValue = GetJsonElement(exception.Data["uri"]); + // Verify the values (they should be JsonElements) + var uriValue = Assert.IsType(exception.Data["uri"]); Assert.Equal("file:///path/to/resource", uriValue.GetString()); - var codeValue = GetJsonElement(exception.Data["code"]); + var codeValue = Assert.IsType(exception.Data["code"]); Assert.Equal(404, codeValue.GetInt32()); } @@ -124,6 +121,12 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); + + // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data + if (PlatformDetection.IsNetFramework) + { + return; + } // Verify that only the serializable data was propagated (non-serializable was filtered out) var hasUri = false; @@ -139,7 +142,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key"); - var uriValue = GetJsonElement(exception.Data["uri"]); + var uriValue = Assert.IsType(exception.Data["uri"]); Assert.Equal("file:///path/to/resource", uriValue.GetString()); } diff --git a/tests/ModelContextProtocol.Tests/PlatformDetection.cs b/tests/ModelContextProtocol.Tests/PlatformDetection.cs index f439147ff..594ccf40b 100644 --- a/tests/ModelContextProtocol.Tests/PlatformDetection.cs +++ b/tests/ModelContextProtocol.Tests/PlatformDetection.cs @@ -6,4 +6,13 @@ internal static class PlatformDetection { public static bool IsMonoRuntime { get; } = Type.GetType("Mono.Runtime") is not null; public static bool IsWindows { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + // On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. + // JsonElement is not marked as serializable, so certain features are not available on that platform. + public static bool IsNetFramework { get; } = +#if NET + false; +#else + true; +#endif } \ No newline at end of file From a47be0d0e0efe4d18fa3b01d8f2329d2f2fa02e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 21:01:54 +0000 Subject: [PATCH 15/16] Extract primitive values from JsonElements for .NET Framework compatibility Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpSessionHandler.cs | 27 +++++++++++++---- .../McpProtocolExceptionDataTests.cs | 30 +++++-------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 5cf5d8fe4..78de23ad4 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -455,19 +455,36 @@ public async Task SendRequestAsync(JsonRpcRequest request, Canc LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code); var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code); -#if NET // Populate exception.Data with the error data if present. // When deserializing JSON, Data will be a JsonElement. - // Note: This is not supported on .NET Framework because Exception.Data uses ListDictionaryInternal - // which requires values to be marked with [Serializable], and JsonElement is not serializable. + // We extract primitive values (strings, numbers, bools) for broader compatibility, + // as JsonElement is not [Serializable] and cannot be stored in Exception.Data on .NET Framework. if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) { foreach (var property in jsonElement.EnumerateObject()) { - exception.Data[property.Name] = property.Value; + object? value = property.Value.ValueKind switch + { + JsonValueKind.String => property.Value.GetString(), + JsonValueKind.Number => property.Value.GetDouble(), + JsonValueKind.True => true, + JsonValueKind.False => false, + JsonValueKind.Null => null, +#if NET + // Objects and arrays are stored as JsonElement on .NET Core only + _ => property.Value, +#else + // Skip objects/arrays on .NET Framework as JsonElement is not serializable + _ => (object?)null, +#endif + }; + + if (value is not null || property.Value.ValueKind == JsonValueKind.Null) + { + exception.Data[property.Name] = value; + } } } -#endif throw exception; } diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index 55795c7cb..f8d1f892e 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -11,9 +11,9 @@ namespace ModelContextProtocol.Tests; /// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. /// /// -/// Note: On .NET Framework, Exception.Data requires values to be serializable with [Serializable]. -/// Since JsonElement is not marked as serializable, the data population on the client side is skipped -/// on that platform. These tests verify the error message and code are correct regardless of platform. +/// Primitive values (strings, numbers, bools) are extracted from JsonElements and stored directly, +/// which works on all platforms including .NET Framework. Complex objects and arrays are stored as +/// JsonElement on .NET Core, but skipped on .NET Framework (where JsonElement is not serializable). /// public class McpProtocolExceptionDataTests : ClientServerTestBase { @@ -79,12 +79,6 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); - // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (PlatformDetection.IsNetFramework) - { - return; - } - // Verify the data was propagated to the exception // The Data collection should contain the expected keys var hasUri = false; @@ -100,12 +94,9 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.True(hasCode, "Exception.Data should contain 'code' key"); - // Verify the values (they should be JsonElements) - var uriValue = Assert.IsType(exception.Data["uri"]); - Assert.Equal("file:///path/to/resource", uriValue.GetString()); - - var codeValue = Assert.IsType(exception.Data["code"]); - Assert.Equal(404, codeValue.GetInt32()); + // Verify the values - primitives are extracted as their native types (string, double, bool) + Assert.Equal("file:///path/to/resource", exception.Data["uri"]); + Assert.Equal(404.0, exception.Data["code"]); // Numbers are stored as double } [Fact] @@ -122,12 +113,6 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("Request failed (remote): Resource not found", exception.Message); Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode); - // Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data - if (PlatformDetection.IsNetFramework) - { - return; - } - // Verify that only the serializable data was propagated (non-serializable was filtered out) var hasUri = false; var hasNonSerializable = false; @@ -142,8 +127,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.True(hasUri, "Exception.Data should contain 'uri' key"); Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key"); - var uriValue = Assert.IsType(exception.Data["uri"]); - Assert.Equal("file:///path/to/resource", uriValue.GetString()); + Assert.Equal("file:///path/to/resource", exception.Data["uri"]); } [Fact] From e5d0cbba01059e780e405a1bb23806dfd7313504 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 15:06:28 +0000 Subject: [PATCH 16/16] Fix tests: Skip non-serializable data tests on .NET Framework Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../McpProtocolExceptionDataTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs index f8d1f892e..e4579e6bb 100644 --- a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -17,6 +17,8 @@ namespace ModelContextProtocol.Tests; /// public class McpProtocolExceptionDataTests : ClientServerTestBase { + public static bool IsNotNetFramework => !PlatformDetection.IsNetFramework; + public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { @@ -99,7 +101,7 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client() Assert.Equal(404.0, exception.Data["code"]); // Numbers are stored as double } - [Fact] + [Fact(Skip = "Non-serializable test data not supported on .NET Framework", SkipUnless = nameof(IsNotNetFramework))] public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_Client() { await using McpClient client = await CreateMcpClientForServer(); @@ -130,7 +132,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_ Assert.Equal("file:///path/to/resource", exception.Data["uri"]); } - [Fact] + [Fact(Skip = "Non-serializable test data not supported on .NET Framework", SkipUnless = nameof(IsNotNetFramework))] public async Task Exception_With_Only_NonSerializable_Data_Still_Propagates_Error_To_Client() { await using McpClient client = await CreateMcpClientForServer();