Skip to content

Commit a47be0d

Browse files
Copilotstephentoub
andcommitted
Extract primitive values from JsonElements for .NET Framework compatibility
Co-authored-by: stephentoub <[email protected]>
1 parent 3082e35 commit a47be0d

File tree

2 files changed

+29
-28
lines changed

2 files changed

+29
-28
lines changed

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -455,19 +455,36 @@ 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.
460+
// We extract primitive values (strings, numbers, bools) for broader compatibility,
461+
// as JsonElement is not [Serializable] and cannot be stored in Exception.Data on .NET Framework.
463462
if (error.Error.Data is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object)
464463
{
465464
foreach (var property in jsonElement.EnumerateObject())
466465
{
467-
exception.Data[property.Name] = property.Value;
466+
object? value = property.Value.ValueKind switch
467+
{
468+
JsonValueKind.String => property.Value.GetString(),
469+
JsonValueKind.Number => property.Value.GetDouble(),
470+
JsonValueKind.True => true,
471+
JsonValueKind.False => false,
472+
JsonValueKind.Null => null,
473+
#if NET
474+
// Objects and arrays are stored as JsonElement on .NET Core only
475+
_ => property.Value,
476+
#else
477+
// Skip objects/arrays on .NET Framework as JsonElement is not serializable
478+
_ => (object?)null,
479+
#endif
480+
};
481+
482+
if (value is not null || property.Value.ValueKind == JsonValueKind.Null)
483+
{
484+
exception.Data[property.Name] = value;
485+
}
468486
}
469487
}
470-
#endif
471488

472489
throw exception;
473490
}

tests/ModelContextProtocol.Tests/McpProtocolExceptionDataTests.cs

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ namespace ModelContextProtocol.Tests;
1111
/// Tests for McpProtocolException.Data propagation to JSON-RPC error responses.
1212
/// </summary>
1313
/// <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.
14+
/// Primitive values (strings, numbers, bools) are extracted from JsonElements and stored directly,
15+
/// which works on all platforms including .NET Framework. Complex objects and arrays are stored as
16+
/// JsonElement on .NET Core, but skipped on .NET Framework (where JsonElement is not serializable).
1717
/// </remarks>
1818
public class McpProtocolExceptionDataTests : ClientServerTestBase
1919
{
@@ -79,12 +79,6 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client()
7979
Assert.Equal("Request failed (remote): Resource not found", exception.Message);
8080
Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode);
8181

82-
// Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data
83-
if (PlatformDetection.IsNetFramework)
84-
{
85-
return;
86-
}
87-
8882
// Verify the data was propagated to the exception
8983
// The Data collection should contain the expected keys
9084
var hasUri = false;
@@ -100,12 +94,9 @@ public async Task Exception_With_Serializable_Data_Propagates_To_Client()
10094
Assert.True(hasUri, "Exception.Data should contain 'uri' key");
10195
Assert.True(hasCode, "Exception.Data should contain 'code' key");
10296

103-
// Verify the values (they should be JsonElements)
104-
var uriValue = Assert.IsType<JsonElement>(exception.Data["uri"]);
105-
Assert.Equal("file:///path/to/resource", uriValue.GetString());
106-
107-
var codeValue = Assert.IsType<JsonElement>(exception.Data["code"]);
108-
Assert.Equal(404, codeValue.GetInt32());
97+
// Verify the values - primitives are extracted as their native types (string, double, bool)
98+
Assert.Equal("file:///path/to/resource", exception.Data["uri"]);
99+
Assert.Equal(404.0, exception.Data["code"]); // Numbers are stored as double
109100
}
110101

111102
[Fact]
@@ -122,12 +113,6 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_
122113
Assert.Equal("Request failed (remote): Resource not found", exception.Message);
123114
Assert.Equal((McpErrorCode)(-32002), exception.ErrorCode);
124115

125-
// Skip data verification on .NET Framework since JsonElement cannot be stored in Exception.Data
126-
if (PlatformDetection.IsNetFramework)
127-
{
128-
return;
129-
}
130-
131116
// Verify that only the serializable data was propagated (non-serializable was filtered out)
132117
var hasUri = false;
133118
var hasNonSerializable = false;
@@ -142,8 +127,7 @@ public async Task Exception_With_NonSerializable_Data_Still_Propagates_Error_To_
142127
Assert.True(hasUri, "Exception.Data should contain 'uri' key");
143128
Assert.False(hasNonSerializable, "Exception.Data should not contain 'nonSerializable' key");
144129

145-
var uriValue = Assert.IsType<JsonElement>(exception.Data["uri"]);
146-
Assert.Equal("file:///path/to/resource", uriValue.GetString());
130+
Assert.Equal("file:///path/to/resource", exception.Data["uri"]);
147131
}
148132

149133
[Fact]

0 commit comments

Comments
 (0)