From db0f4d13b4c9780e237d2c2e26b134130de8ddd7 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 3 Dec 2025 15:37:54 -0800 Subject: [PATCH] Add support for `data` to `McpProtocolException` (#1028) --- .../McpSessionHandler.cs | 86 +++++++++- .../McpProtocolExceptionDataTests.cs | 160 ++++++++++++++++++ .../PlatformDetection.cs | 9 + .../Server/McpServerTests.cs | 58 +++++++ 4 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index fcd7980..78de23a 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -186,10 +186,10 @@ internal sealed partial class McpSessionHandler : IAsyncDisposable { Code = (int)mcpProtocolException.ErrorCode, Message = mcpProtocolException.Message, + Data = ConvertExceptionData(mcpProtocolException.Data), } : ex is McpException mcpException ? new() { - Code = (int)McpErrorCode.InternalError, Message = mcpException.Message, } : @@ -206,6 +206,7 @@ internal sealed partial class McpSessionHandler : IAsyncDisposable Error = detail, Context = new JsonRpcMessageContext { RelatedTransport = request.Context?.RelatedTransport }, }; + await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false); } else if (ex is not OperationCanceledException) @@ -452,7 +453,40 @@ internal sealed partial class McpSessionHandler : IAsyncDisposable 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. + // When deserializing JSON, Data will be a JsonElement. + // 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()) + { + 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; + } + } + } + + throw exception; } if (response is JsonRpcResponse success) @@ -769,6 +803,54 @@ internal sealed partial class McpSessionHandler : IAsyncDisposable return null; } + /// + /// Converts the dictionary to a serializable . + /// 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) + { + if (data.Count == 0) + { + return null; + } + + var typeInfo = McpJsonUtilities.DefaultOptions.GetTypeInfo(); + + Dictionary? result = null; + foreach (System.Collections.DictionaryEntry entry in data) + { + if (entry.Key is string key) + { + 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; + } + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} message processing canceled.")] private partial void LogEndpointMessageProcessingCanceled(string endpointName); diff --git a/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs new file mode 100644 index 0000000..e4579e6 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs @@ -0,0 +1,160 @@ +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using ModelContextProtocol.Tests.Utils; +using System.Text.Json; + +namespace ModelContextProtocol.Tests; + +/// +/// Tests for McpProtocolException.Data propagation to JSON-RPC error responses. +/// +/// +/// 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 +{ + public static bool IsNotNetFramework => !PlatformDetection.IsNetFramework; + + 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); + + // 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 - 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(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(); + + // 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); + + // 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"); + + Assert.Equal("file:///path/to/resource", exception.Data["uri"]); + } + + [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(); + + // 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); + + // 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); + } + + /// + /// 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/PlatformDetection.cs b/tests/ModelContextProtocol.Tests/PlatformDetection.cs index f439147..594ccf4 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 diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs index ab2537b..8ca4709 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs @@ -671,6 +671,64 @@ public class McpServerTests : LoggedTest 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 (values are now JsonElements after serialization) + var dataDict = Assert.IsType>(error.Error.Data); + Assert.True(dataDict.ContainsKey("uri")); + Assert.Equal(ResourceUri, dataDict["uri"].GetString()); + + 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();