Skip to content

Commit 580743d

Browse files
Copilotstephentoub
andcommitted
Add test for non-serializable Exception.Data and fix exception handling
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 9b8cf68 commit 580743d

File tree

2 files changed

+101
-1
lines changed

2 files changed

+101
-1
lines changed

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,18 @@ ex is OperationCanceledException &&
206206
Error = detail,
207207
Context = new JsonRpcMessageContext { RelatedTransport = request.Context?.RelatedTransport },
208208
};
209-
await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false);
209+
210+
try
211+
{
212+
await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false);
213+
}
214+
catch (Exception sendException) when ((sendException is JsonException || sendException is NotSupportedException) && detail.Data is not null)
215+
{
216+
// If serialization fails (e.g., non-serializable data in Exception.Data),
217+
// retry without the data to ensure the client receives an error response.
218+
detail.Data = null;
219+
await SendMessageAsync(errorMessage, cancellationToken).ConfigureAwait(false);
220+
}
210221
}
211222
else if (ex is not OperationCanceledException)
212223
{

tests/ModelContextProtocol.Tests/Server/McpServerTests.cs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Runtime.InteropServices;
77
using System.Text.Json;
88
using System.Text.Json.Nodes;
9+
using System.Threading.Channels;
910

1011
namespace ModelContextProtocol.Tests.Server;
1112

@@ -729,6 +730,94 @@ await transport.SendMessageAsync(
729730
await runTask;
730731
}
731732

733+
[Fact]
734+
public async Task Can_Handle_Call_Tool_Requests_With_McpProtocolException_And_NonSerializableData()
735+
{
736+
const string ErrorMessage = "Resource not found";
737+
const McpErrorCode ErrorCode = (McpErrorCode)(-32002);
738+
739+
await using var transport = new SerializingTestServerTransport();
740+
var options = CreateOptions(new ServerCapabilities { Tools = new() });
741+
options.Handlers.CallToolHandler = async (request, ct) =>
742+
{
743+
throw new McpProtocolException(ErrorMessage, ErrorCode)
744+
{
745+
Data =
746+
{
747+
// Add a non-serializable object (an object with circular reference)
748+
{ "nonSerializable", new NonSerializableObject() }
749+
}
750+
};
751+
};
752+
options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException();
753+
754+
await using var server = McpServer.Create(transport, options, LoggerFactory);
755+
756+
var runTask = server.RunAsync(TestContext.Current.CancellationToken);
757+
758+
var receivedMessage = new TaskCompletionSource<JsonRpcError>();
759+
760+
transport.OnMessageSent = (message) =>
761+
{
762+
if (message is JsonRpcError error && error.Id.ToString() == "55")
763+
receivedMessage.SetResult(error);
764+
};
765+
766+
await transport.SendMessageAsync(
767+
new JsonRpcRequest
768+
{
769+
Method = RequestMethods.ToolsCall,
770+
Id = new RequestId(55)
771+
},
772+
TestContext.Current.CancellationToken
773+
);
774+
775+
// Client should still receive an error response, even though the data couldn't be serialized
776+
var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
777+
Assert.NotNull(error);
778+
Assert.NotNull(error.Error);
779+
Assert.Equal((int)ErrorCode, error.Error.Code);
780+
Assert.Equal(ErrorMessage, error.Error.Message);
781+
// Data should be null since it couldn't be serialized
782+
Assert.Null(error.Error.Data);
783+
784+
await transport.DisposeAsync();
785+
await runTask;
786+
}
787+
788+
/// <summary>
789+
/// A class that cannot be serialized by System.Text.Json due to circular reference.
790+
/// </summary>
791+
private sealed class NonSerializableObject
792+
{
793+
public NonSerializableObject() => Self = this;
794+
public NonSerializableObject Self { get; set; }
795+
}
796+
797+
/// <summary>
798+
/// A test transport that simulates JSON serialization failure for non-serializable data.
799+
/// </summary>
800+
private sealed class SerializingTestServerTransport : ITransport
801+
{
802+
private readonly TestServerTransport _inner = new();
803+
804+
public bool IsConnected => _inner.IsConnected;
805+
public ChannelReader<JsonRpcMessage> MessageReader => _inner.MessageReader;
806+
public string? SessionId => _inner.SessionId;
807+
public Action<JsonRpcMessage>? OnMessageSent { get => _inner.OnMessageSent; set => _inner.OnMessageSent = value; }
808+
809+
public Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
810+
{
811+
// Serialize the message to verify it can be serialized (this will throw JsonException if not)
812+
// We serialize synchronously before any async operations to ensure the exception propagates correctly
813+
_ = JsonSerializer.Serialize(message, McpJsonUtilities.DefaultOptions);
814+
815+
return _inner.SendMessageAsync(message, cancellationToken);
816+
}
817+
818+
public ValueTask DisposeAsync() => _inner.DisposeAsync();
819+
}
820+
732821
private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action<McpServerOptions>? configureOptions, Action<McpServer, JsonNode?> assertResult)
733822
{
734823
await using var transport = new TestServerTransport();

0 commit comments

Comments
 (0)