Revert SerializableJsonElement wrapper, restore #if NET approach

Co-authored-by: stephentoub <[email protected]>

Author: Copilot

src/ModelContextProtocol.Core/McpSessionHandler.cs

@@ -455,17 +455,19 @@ public async Task<JsonRpcResponse> 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;
             }

src/ModelContextProtocol.Core/Protocol/SerializableJsonElement.cs

@@ -10,6 +10,11 @@ namespace ModelContextProtocol.Tests;
 /// <summary>
 /// Tests for McpProtocolException.Data propagation to JSON-RPC error responses.
 /// </summary>
+/// <remarks>
+/// 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.
+/// </remarks>
 public class McpProtocolExceptionDataTests : ClientServerTestBase
 {
     public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper)
@@ -63,20 +68,6 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
         });
     }
 
-    /// <summary>
-    /// Gets the JsonElement from an Exception.Data value, handling both direct JsonElement (on .NET Core)
-    /// and SerializableJsonElement wrapper (on .NET Framework).
-    /// </summary>
-    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<JsonElement>(exception.Data["uri"]);
         Assert.Equal("file:///path/to/resource", uriValue.GetString());
 
-        var codeValue = GetJsonElement(exception.Data["code"]);
+        var codeValue = Assert.IsType<JsonElement>(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<JsonElement>(exception.Data["uri"]);
         Assert.Equal("file:///path/to/resource", uriValue.GetString());
     }
 

tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.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
 }