Skip to content

Commit f757558

Browse files
Copilotstephentoub
andcommitted
Make Exception.Data work on .NET Framework using SerializableJsonElement wrapper
Co-authored-by: stephentoub <[email protected]>
1 parent c8f8a27 commit f757558

File tree

4 files changed

+69
-35
lines changed

4 files changed

+69
-35
lines changed

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -455,19 +455,17 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
455455
LogSendingRequestFailed(EndpointName, request.Method, error.Error.Message, error.Error.Code);
456456
var exception = new McpProtocolException($"Request failed (remote): {error.Error.Message}", (McpErrorCode)error.Error.Code);
457457

458-
#if NET
459458
// Populate exception.Data with the error data if present.
460459
// When deserializing JSON, Data will be a JsonElement.
461-
// Note: This is not supported on .NET Framework because Exception.Data uses ListDictionaryInternal
462-
// which requires values to be marked with [Serializable], and JsonElement is not serializable.
463460
if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object)
464461
{
465462
foreach (var property in jsonElement.EnumerateObject())
466463
{
467-
exception.Data[property.Name] = property.Value;
464+
// On .NET Framework, Exception.Data requires values to be [Serializable].
465+
// JsonElement is not serializable, so we wrap it in a serializable container.
466+
exception.Data[property.Name] = SerializableJsonElement.Wrap(property.Value);
468467
}
469468
}
470-
#endif
471469

472470
throw exception;
473471
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
using System.Text.Json;
2+
3+
namespace ModelContextProtocol.Protocol;
4+
5+
/// <summary>
6+
/// A serializable wrapper for <see cref="JsonElement"/> that can be stored in <see cref="Exception.Data"/>
7+
/// on .NET Framework, where values must be marked with <see cref="SerializableAttribute"/>.
8+
/// </summary>
9+
/// <remarks>
10+
/// On .NET Core/.NET 5+, <see cref="JsonElement"/> can be stored directly in <see cref="Exception.Data"/>,
11+
/// but on .NET Framework the underlying <c>ListDictionaryInternal</c> requires values to be serializable.
12+
/// This wrapper stores the JSON as a string and deserializes it back to a <see cref="JsonElement"/> on demand.
13+
/// </remarks>
14+
[Serializable]
15+
public sealed class SerializableJsonElement
16+
{
17+
private readonly string _json;
18+
19+
private SerializableJsonElement(string json)
20+
{
21+
_json = json;
22+
}
23+
24+
/// <summary>
25+
/// Gets the <see cref="JsonElement"/> value.
26+
/// </summary>
27+
public JsonElement Value => JsonDocument.Parse(_json).RootElement;
28+
29+
/// <summary>
30+
/// Creates a serializable wrapper for the specified <see cref="JsonElement"/>.
31+
/// </summary>
32+
/// <param name="element">The JSON element to wrap.</param>
33+
/// <returns>
34+
/// On .NET Core/.NET 5+, returns the <see cref="JsonElement"/> directly.
35+
/// On .NET Framework, returns a <see cref="SerializableJsonElement"/> wrapper.
36+
/// </returns>
37+
internal static object Wrap(JsonElement element)
38+
{
39+
#if NET
40+
return element;
41+
#else
42+
return new SerializableJsonElement(element.GetRawText());
43+
#endif
44+
}
45+
46+
/// <inheritdoc/>
47+
public override string ToString() => _json;
48+
}

tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ namespace ModelContextProtocol.Tests;
1010
/// <summary>
1111
/// Tests for McpProtocolException.Data propagation to JSON-RPC error responses.
1212
/// </summary>
13-
/// <remarks>
14-
/// Note: On .NET Framework, Exception.Data requires values to be serializable with [Serializable].
15-
/// Since JsonElement is not marked as serializable, the data population on the client side is skipped
16-
/// on that platform. These tests verify the error message and code are correct regardless of platform.
17-
/// </remarks>
1813
public class McpProtocolExceptionDataTests : ClientServerTestBase
1914
{
2015
public McpProtocolExceptionDataTests(ITestOutputHelper testOutputHelper)
@@ -68,6 +63,20 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
6863
});
6964
}
7065

66+
/// <summary>
67+
/// Gets the JsonElement from an Exception.Data value, handling both direct JsonElement (on .NET Core)
68+
/// and SerializableJsonElement wrapper (on .NET Framework).
69+
/// </summary>
70+
private static JsonElement GetJsonElement(object? value)
71+
{
72+
return value switch
73+
{
74+
JsonElement je => je,
75+
SerializableJsonElement sje => sje.Value,
76+
_ => throw new InvalidOperationException($"Expected JsonElement or SerializableJsonElement, got {value?.GetType().Name ?? "null"}")
77+
};
78+
}
79+
7180
[Fact]
7281
public async Task Exception_With_Serializable_Data_Propagates_To_Client()
7382
{
@@ -78,12 +87,6 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client()
7887

7988
Assert.Equal("Request failed (remote): Resource not found", exception.Message);
8089
Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode);
81-
82-
// Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data
83-
if (PlatformDetection.IsNetFramework)
84-
{
85-
return;
86-
}
8790

8891
// Verify the data was propagated to the exception
8992
// The Data collection should contain the expected keys
@@ -100,11 +103,11 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client()
100103
Assert.True(hasUri, "Exception.Data should contain 'uri' key");
101104
Assert.True(hasCode, "Exception.Data should contain 'code' key");
102105

103-
// Verify the values (they should be JsonElements)
104-
var uriValue = Assert.IsType<JsonElement>(exception.Data["uri"]);
106+
// Verify the values (they are JsonElements on .NET Core, SerializableJsonElement on .NET Framework)
107+
var uriValue = GetJsonElement(exception.Data["uri"]);
105108
Assert.Equal("file:///path/to/resource", uriValue.GetString());
106109

107-
var codeValue = Assert.IsType<JsonElement>(exception.Data["code"]);
110+
var codeValue = GetJsonElement(exception.Data["code"]);
108111
Assert.Equal(404, codeValue.GetInt32());
109112
}
110113

@@ -121,12 +124,6 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_
121124

122125
Assert.Equal("Request failed (remote): Resource not found", exception.Message);
123126
Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode);
124-
125-
// Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data
126-
if (PlatformDetection.IsNetFramework)
127-
{
128-
return;
129-
}
130127

131128
// Verify that only the serializable data was propagated (non-serializable was filtered out)
132129
var hasUri = false;
@@ -142,7 +139,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_
142139
Assert.True(hasUri, "Exception.Data should contain 'uri' key");
143140
Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key");
144141

145-
var uriValue = Assert.IsType<JsonElement>(exception.Data["uri"]);
142+
var uriValue = GetJsonElement(exception.Data["uri"]);
146143
Assert.Equal("file:///path/to/resource", uriValue.GetString());
147144
}
148145

tests/ModelContextProtocol.Tests/PlatformDetection.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,4 @@ internal static class PlatformDetection
66
{
77
public static bool IsMonoRuntime { get; } = Type.GetType("Mono.Runtime") is not null;
88
public static bool IsWindows { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
9-
10-
// On .NET Framework, Exception.Data requires values to be serializable with [Serializable].
11-
// JsonElement is not marked as serializable, so certain features are not available on that platform.
12-
public static bool IsNetFramework { get; } =
13-
#if NET
14-
false;
15-
#else
16-
true;
17-
#endif
189
}

0 commit comments

Comments
 (0)