Skip to content

Commit 6d600ee

Browse files
halter73Copilot
andcommitted
Move sending logs inside outgoing filter and add logging verification tests
Move LogSendingRequest/LogSendingMessage from SendRequestAsync/SendMessageAsync into SendToRelatedTransportAsync's inner next function. This ensures 'sending' logs only appear when the message is actually sent through the transport, not when an outgoing filter skips sending. Add 4 tests verifying logging behavior when filters skip or call next: - AddOutgoingMessageFilter_SkipNext_DoesNotLogSending - AddOutgoingMessageFilter_CallsNext_LogsSending - AddIncomingMessageFilter_SkipNext_DoesNotLogSendingResponse - AddIncomingMessageFilter_CallsNext_LogsSendingResponse Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3433372 commit 6d600ee

3 files changed

Lines changed: 165 additions & 80 deletions

File tree

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -576,15 +576,6 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
576576
AddTags(ref tags, activity, request, method, target);
577577
}
578578

579-
if (_logger.IsEnabled(LogLevel.Trace))
580-
{
581-
LogSendingRequestSensitive(EndpointName, request.Method, JsonSerializer.Serialize(request, McpJsonUtilities.JsonContext.Default.JsonRpcMessage));
582-
}
583-
else
584-
{
585-
LogSendingRequest(EndpointName, request.Method);
586-
}
587-
588579
await SendToRelatedTransportAsync(request, cancellationToken).ConfigureAwait(false);
589580

590581
// Now that the request has been sent, register for cancellation. If we registered before,
@@ -671,15 +662,6 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
671662
AddTags(ref tags, activity, message, method, target);
672663
}
673664

674-
if (_logger.IsEnabled(LogLevel.Trace))
675-
{
676-
LogSendingMessageSensitive(EndpointName, JsonSerializer.Serialize(message, McpJsonUtilities.JsonContext.Default.JsonRpcMessage));
677-
}
678-
else
679-
{
680-
LogSendingMessage(EndpointName);
681-
}
682-
683665
await SendToRelatedTransportAsync(message, cancellationToken).ConfigureAwait(false);
684666

685667
// If the sent notification was a cancellation notification, cancel the pending request's await, as either the
@@ -708,7 +690,32 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
708690
// the HTTP response body for the POST request containing the corresponding JSON-RPC request.
709691
private Task SendToRelatedTransportAsync(JsonRpcMessage message, CancellationToken cancellationToken)
710692
=> _outgoingMessageFilter((msg, ct) =>
711-
(msg.Context?.RelatedTransport ?? _transport).SendMessageAsync(msg, ct))(message, cancellationToken);
693+
{
694+
if (msg is JsonRpcRequest request)
695+
{
696+
if (_logger.IsEnabled(LogLevel.Trace))
697+
{
698+
LogSendingRequestSensitive(EndpointName, request.Method, JsonSerializer.Serialize(msg, McpJsonUtilities.JsonContext.Default.JsonRpcMessage));
699+
}
700+
else
701+
{
702+
LogSendingRequest(EndpointName, request.Method);
703+
}
704+
}
705+
else
706+
{
707+
if (_logger.IsEnabled(LogLevel.Trace))
708+
{
709+
LogSendingMessageSensitive(EndpointName, JsonSerializer.Serialize(msg, McpJsonUtilities.JsonContext.Default.JsonRpcMessage));
710+
}
711+
else
712+
{
713+
LogSendingMessage(EndpointName);
714+
}
715+
}
716+
717+
return (msg.Context?.RelatedTransport ?? _transport).SendMessageAsync(msg, ct);
718+
})(message, cancellationToken);
712719

713720
private static CancelledNotificationParams? GetCancelledNotificationParams(JsonNode? notificationParams)
714721
{

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpTests.cs

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,10 @@ await client.CallToolAsync("echo_with_user_name",
328328
}
329329

330330
[Fact]
331-
public async Task OutgoingFilter_SeesResponses()
331+
public async Task OutgoingFilter_SeesResponsesAndRequests()
332332
{
333+
Assert.SkipWhen(Stateless, "Server-originated requests are not supported in stateless mode.");
334+
333335
var observedMessageTypes = new List<string>();
334336

335337
Builder.Services.AddMcpServer()
@@ -338,6 +340,7 @@ public async Task OutgoingFilter_SeesResponses()
338340
{
339341
var typeName = context.JsonRpcMessage switch
340342
{
343+
JsonRpcRequest request => $"request:{request.Method}",
341344
JsonRpcResponse r when r.Result is JsonObject obj && obj.ContainsKey("protocolVersion") => "initialize-response",
342345
JsonRpcResponse r when r.Result is JsonObject obj && obj.ContainsKey("tools") => "tools-list-response",
343346
JsonRpcResponse r when r.Result is JsonObject obj && obj.ContainsKey("content") => "tool-call-response",
@@ -351,42 +354,7 @@ public async Task OutgoingFilter_SeesResponses()
351354

352355
await next(context, cancellationToken);
353356
}))
354-
.WithTools<ClaimsPrincipalTools>();
355-
356-
await using var app = Builder.Build();
357-
app.MapMcp();
358-
await app.StartAsync(TestContext.Current.CancellationToken);
359-
360-
await using var client = await ConnectAsync();
361-
362-
await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
363-
await client.CallToolAsync("echo_claims_principal",
364-
new Dictionary<string, object?> { ["message"] = "hi" },
365-
cancellationToken: TestContext.Current.CancellationToken);
366-
367-
Assert.Contains("initialize-response", observedMessageTypes);
368-
Assert.Contains("tools-list-response", observedMessageTypes);
369-
Assert.Contains("tool-call-response", observedMessageTypes);
370-
}
371-
372-
[Fact]
373-
public async Task OutgoingFilter_SeesServerOriginatedRequests()
374-
{
375-
Assert.SkipWhen(Stateless, "Server-originated requests are not supported in stateless mode.");
376-
377-
var observedMethods = new List<string>();
378-
379-
Builder.Services.AddMcpServer()
380-
.WithHttpTransport(ConfigureStateless)
381-
.WithMessageFilters(filters => filters.AddOutgoingFilter((next) => async (context, cancellationToken) =>
382-
{
383-
if (context.JsonRpcMessage is JsonRpcRequest request)
384-
{
385-
observedMethods.Add(request.Method);
386-
}
387-
388-
await next(context, cancellationToken);
389-
}))
357+
.WithTools<ClaimsPrincipalTools>()
390358
.WithTools<SamplingRegressionTools>();
391359

392360
await using var app = Builder.Build();
@@ -408,11 +376,18 @@ public async Task OutgoingFilter_SeesServerOriginatedRequests()
408376

409377
await using var client = await ConnectAsync(clientOptions: clientOptions);
410378

379+
await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
380+
await client.CallToolAsync("echo_claims_principal",
381+
new Dictionary<string, object?> { ["message"] = "hi" },
382+
cancellationToken: TestContext.Current.CancellationToken);
411383
await client.CallToolAsync("sampling-tool",
412384
new Dictionary<string, object?> { ["prompt"] = "Hello" },
413385
cancellationToken: TestContext.Current.CancellationToken);
414386

415-
Assert.Contains(RequestMethods.SamplingCreateMessage, observedMethods);
387+
Assert.Contains("initialize-response", observedMessageTypes);
388+
Assert.Contains("tools-list-response", observedMessageTypes);
389+
Assert.Contains("tool-call-response", observedMessageTypes);
390+
Assert.Contains($"request:{RequestMethods.SamplingCreateMessage}", observedMessageTypes);
416391
}
417392

418393
[Fact]

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsMessageFilterTests.cs

Lines changed: 126 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ await Assert.ThrowsAnyAsync<OperationCanceledException>(async () =>
317317
}
318318

319319
[Fact]
320-
public async Task AddOutgoingMessageFilter_Sees_Initialize_Progress_And_Response()
320+
public async Task AddOutgoingMessageFilter_Sees_Responses_Notifications_And_Requests()
321321
{
322322
var observedMessages = new List<string>();
323323

@@ -326,6 +326,9 @@ public async Task AddOutgoingMessageFilter_Sees_Initialize_Progress_And_Response
326326
{
327327
switch (context.JsonRpcMessage)
328328
{
329+
case JsonRpcRequest request:
330+
observedMessages.Add($"request:{request.Method}");
331+
break;
329332
case JsonRpcResponse response when response.Result is JsonObject result:
330333
if (result.ContainsKey("protocolVersion"))
331334
{
@@ -343,22 +346,41 @@ public async Task AddOutgoingMessageFilter_Sees_Initialize_Progress_And_Response
343346

344347
await next(context, cancellationToken);
345348
}))
346-
.WithTools<ProgressTool>();
349+
.WithTools<ProgressTool>()
350+
.WithTools<SamplingTool>();
347351

348352
StartServer();
349353

350-
await using McpClient client = await CreateMcpClientForServer();
354+
var clientOptions = new McpClientOptions
355+
{
356+
Capabilities = new() { Sampling = new() },
357+
Handlers = new()
358+
{
359+
SamplingHandler = (_, _, _) => new(new CreateMessageResult
360+
{
361+
Content = [new TextContentBlock { Text = "sampled" }],
362+
Model = "test-model",
363+
}),
364+
},
365+
};
366+
367+
await using McpClient client = await CreateMcpClientForServer(clientOptions);
351368

352369
IProgress<ProgressNotificationValue> progress = new Progress<ProgressNotificationValue>(_ => { });
353370
await client.CallToolAsync("progress-tool", progress: progress, cancellationToken: TestContext.Current.CancellationToken);
354371

372+
await client.CallToolAsync("sampling-tool", new Dictionary<string, object?> { ["prompt"] = "Hello" },
373+
cancellationToken: TestContext.Current.CancellationToken);
374+
355375
int initializeIndex = observedMessages.IndexOf("initialize");
356376
int progressIndex = observedMessages.IndexOf("progress");
357377
int responseIndex = observedMessages.LastIndexOf("response");
378+
int requestIndex = observedMessages.IndexOf($"request:{RequestMethods.SamplingCreateMessage}");
358379

359380
Assert.True(initializeIndex >= 0);
360381
Assert.True(progressIndex > initializeIndex);
361382
Assert.True(responseIndex > progressIndex);
383+
Assert.True(requestIndex >= 0);
362384
}
363385

364386
[Fact]
@@ -429,43 +451,124 @@ public async Task AddOutgoingMessageFilter_Can_Send_Additional_Messages()
429451
}
430452

431453
[Fact]
432-
public async Task AddOutgoingMessageFilter_Sees_ServerOriginatedRequests()
454+
public async Task AddOutgoingMessageFilter_SkipNext_DoesNotLogSending()
433455
{
434-
var observedMethods = new List<string>();
456+
ServiceCollection.AddLogging(builder => builder.SetMinimumLevel(LogLevel.Debug));
435457

436458
McpServerBuilder
437-
.WithMessageFilters(filters => filters.AddOutgoingFilter((next) => async (context, cancellationToken) =>
459+
.WithMessageFilters(filters => filters.AddOutgoingFilter((next) => (context, cancellationToken) =>
438460
{
439-
if (context.JsonRpcMessage is JsonRpcRequest request)
461+
// Skip sending tool list responses
462+
if (context.JsonRpcMessage is JsonRpcResponse response && response.Result is JsonObject result && result.ContainsKey("tools"))
440463
{
441-
observedMethods.Add(request.Method);
464+
return Task.CompletedTask;
442465
}
443466

444-
await next(context, cancellationToken);
467+
return next(context, cancellationToken);
445468
}))
446-
.WithTools<SamplingTool>();
469+
.WithTools<TestTool>();
447470

448471
StartServer();
449472

450-
var clientOptions = new McpClientOptions
473+
await using McpClient client = await CreateMcpClientForServer();
474+
475+
// Clear any logs from initialization
476+
while (MockLoggerProvider.LogMessages.TryDequeue(out _)) { }
477+
478+
using var requestCts = new CancellationTokenSource(TimeSpan.FromSeconds(2));
479+
await Assert.ThrowsAnyAsync<OperationCanceledException>(async () =>
451480
{
452-
Capabilities = new() { Sampling = new() },
453-
Handlers = new()
481+
await client.ListToolsAsync(cancellationToken: requestCts.Token);
482+
});
483+
484+
// Since the filter skipped next, no "sending message" log should appear for the skipped response
485+
Assert.DoesNotContain(MockLoggerProvider.LogMessages, m =>
486+
m.Category.Contains("McpServer") && m.Message.Contains("sending message", StringComparison.OrdinalIgnoreCase));
487+
}
488+
489+
[Fact]
490+
public async Task AddOutgoingMessageFilter_CallsNext_LogsSending()
491+
{
492+
ServiceCollection.AddLogging(builder => builder.SetMinimumLevel(LogLevel.Debug));
493+
494+
McpServerBuilder
495+
.WithTools<TestTool>();
496+
497+
StartServer();
498+
499+
await using McpClient client = await CreateMcpClientForServer();
500+
501+
// Clear any logs from initialization
502+
while (MockLoggerProvider.LogMessages.TryDequeue(out _)) { }
503+
504+
await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
505+
506+
// The response should have been sent, producing a "sending message" log from the server
507+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
508+
m.Category.Contains("McpServer") && m.Message.Contains("sending message", StringComparison.OrdinalIgnoreCase));
509+
}
510+
511+
[Fact]
512+
public async Task AddIncomingMessageFilter_SkipNext_DoesNotLogSendingResponse()
513+
{
514+
ServiceCollection.AddLogging(builder => builder.SetMinimumLevel(LogLevel.Debug));
515+
516+
McpServerBuilder
517+
.WithMessageFilters(filters => filters.AddIncomingFilter((next) => (context, cancellationToken) =>
454518
{
455-
SamplingHandler = (_, _, _) => new(new CreateMessageResult
519+
// Skip processing tools/list requests — handler never runs, no response sent
520+
if (context.JsonRpcMessage is JsonRpcRequest request && request.Method == RequestMethods.ToolsList)
456521
{
457-
Content = [new TextContentBlock { Text = "sampled" }],
458-
Model = "test-model",
459-
}),
460-
},
461-
};
522+
return Task.CompletedTask;
523+
}
462524

463-
await using McpClient client = await CreateMcpClientForServer(clientOptions);
525+
return next(context, cancellationToken);
526+
}))
527+
.WithTools<TestTool>();
464528

465-
await client.CallToolAsync("sampling-tool", new Dictionary<string, object?> { ["prompt"] = "Hello" },
466-
cancellationToken: TestContext.Current.CancellationToken);
529+
StartServer();
530+
531+
await using McpClient client = await CreateMcpClientForServer();
532+
533+
// Clear any logs from initialization
534+
while (MockLoggerProvider.LogMessages.TryDequeue(out _)) { }
535+
536+
using var requestCts = new CancellationTokenSource(TimeSpan.FromSeconds(2));
537+
await Assert.ThrowsAnyAsync<OperationCanceledException>(async () =>
538+
{
539+
await client.ListToolsAsync(cancellationToken: requestCts.Token);
540+
});
541+
542+
// Since the incoming filter skipped next, no handler ran, so no response was sent
543+
Assert.DoesNotContain(MockLoggerProvider.LogMessages, m =>
544+
m.Category.Contains("McpServer") && m.Message.Contains("sending message", StringComparison.OrdinalIgnoreCase));
545+
}
546+
547+
[Fact]
548+
public async Task AddIncomingMessageFilter_CallsNext_LogsSendingResponse()
549+
{
550+
ServiceCollection.AddLogging(builder => builder.SetMinimumLevel(LogLevel.Debug));
551+
552+
McpServerBuilder
553+
.WithMessageFilters(filters => filters.AddIncomingFilter((next) => (context, cancellationToken) =>
554+
{
555+
// Pass through — handler runs, response is sent
556+
return next(context, cancellationToken);
557+
}))
558+
.WithTools<TestTool>();
559+
560+
StartServer();
561+
562+
await using McpClient client = await CreateMcpClientForServer();
563+
564+
// Clear any logs from initialization
565+
while (MockLoggerProvider.LogMessages.TryDequeue(out _)) { }
566+
567+
await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
467568

468-
Assert.Contains(RequestMethods.SamplingCreateMessage, observedMethods);
569+
// The handler ran and sent a response, producing a "sending message" log from the server
570+
Assert.Contains(MockLoggerProvider.LogMessages, m =>
571+
m.Category.Contains("McpServer") && m.Message.Contains("sending message", StringComparison.OrdinalIgnoreCase));
469572
}
470573

471574
[Fact]

0 commit comments

Comments
 (0)