From 5084a79068437fd3f7ac733570bc6a755aa8c836 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 17:45:39 -0500 Subject: [PATCH] Add ILoggerFactory to StreamableHttpServerTransport (#1213) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Co-authored-by: Stephen Toub --- .../StreamableHttpHandler.cs | 4 ++-- .../Server/StreamableHttpPostTransport.cs | 19 +++++++++++++++++-- .../Server/StreamableHttpServerTransport.cs | 16 ++++++++++++++-- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs b/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs index c50e513..22c8613 100644 --- a/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs +++ b/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs @@ -238,7 +238,7 @@ internal sealed class StreamableHttpHandler( if (!HttpServerTransportOptions.Stateless) { sessionId = MakeNewSessionId(); - transport = new() + transport = new(loggerFactory) { SessionId = sessionId, FlowExecutionContextFromRequests = !HttpServerTransportOptions.PerSessionExecutionContext, @@ -252,7 +252,7 @@ internal sealed class StreamableHttpHandler( // If in the future we support resuming stateless requests, we should populate // the event stream store and retry interval here as well. sessionId = ""; - transport = new() + transport = new(loggerFactory) { Stateless = true, }; diff --git a/src/ModelContextProtocol.Core/Server/StreamableHttpPostTransport.cs b/src/ModelContextProtocol.Core/Server/StreamableHttpPostTransport.cs index f0f94c2..b6f6b66 100644 --- a/src/ModelContextProtocol.Core/Server/StreamableHttpPostTransport.cs +++ b/src/ModelContextProtocol.Core/Server/StreamableHttpPostTransport.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging; using ModelContextProtocol.Protocol; using System.Diagnostics; using System.Net.ServerSentEvents; @@ -10,7 +11,11 @@ namespace ModelContextProtocol.Server; /// Handles processing the request/response body pairs for the Streamable HTTP transport. /// This is typically used via . /// -internal sealed class StreamableHttpPostTransport(StreamableHttpServerTransport parentTransport, Stream responseStream, CancellationToken sessionCancellationToken) : ITransport +internal sealed partial class StreamableHttpPostTransport( + StreamableHttpServerTransport parentTransport, + Stream responseStream, + CancellationToken sessionCancellationToken, + ILogger logger) : ITransport { private readonly SemaphoreSlim _messageLock = new(1, 1); private readonly TaskCompletionSource _httpResponseTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); @@ -199,7 +204,14 @@ internal sealed class StreamableHttpPostTransport(StreamableHttpServerTransport { using var _ = await _messageLock.LockAsync().ConfigureAwait(false); - await _storeSseWriter!.DisposeAsync().ConfigureAwait(false); + try + { + await _storeSseWriter!.DisposeAsync().ConfigureAwait(false); + } + catch (Exception ex) + { + LogStoreStreamDisposalFailed(ex); + } } } } @@ -222,4 +234,7 @@ internal sealed class StreamableHttpPostTransport(StreamableHttpServerTransport // Don't dispose the event stream writer here, as we may continue to write to the event store // after disposal if there are pending messages. } + + [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to dispose SSE event stream writer.")] + private partial void LogStoreStreamDisposalFailed(Exception exception); } diff --git a/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs b/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs index 55b6650..5822775 100644 --- a/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs +++ b/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using ModelContextProtocol.Protocol; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -21,7 +23,7 @@ namespace ModelContextProtocol.Server; /// such as when streaming completion results or providing progress updates during long-running operations. /// /// -public sealed class StreamableHttpServerTransport : ITransport +public sealed partial class StreamableHttpServerTransport : ITransport { /// /// The stream ID used for unsolicited messages sent via the standalone GET SSE stream. @@ -35,6 +37,7 @@ public sealed class StreamableHttpServerTransport : ITransport }); private readonly CancellationTokenSource _transportDisposedCts = new(); private readonly SemaphoreSlim _unsolicitedMessageLock = new(1, 1); + private readonly ILogger _logger; private SseEventWriter? _httpSseWriter; private ISseEventStreamWriter? _storeSseWriter; @@ -42,6 +45,15 @@ public sealed class StreamableHttpServerTransport : ITransport private bool _getHttpRequestStarted; private bool _getHttpResponseCompleted; + /// + /// Initializes a new instance of the class. + /// + /// Optional logger factory used for logging employed by the transport. + public StreamableHttpServerTransport(ILoggerFactory? loggerFactory = null) + { + _logger = loggerFactory?.CreateLogger() ?? NullLogger.Instance; + } + /// public string? SessionId { get; init; } @@ -161,7 +173,7 @@ public sealed class StreamableHttpServerTransport : ITransport Throw.IfNull(message); Throw.IfNull(responseStream); - var postTransport = new StreamableHttpPostTransport(this, responseStream, _transportDisposedCts.Token); + var postTransport = new StreamableHttpPostTransport(this, responseStream, _transportDisposedCts.Token, _logger); using var postCts = CancellationTokenSource.CreateLinkedTokenSource(_transportDisposedCts.Token, cancellationToken); await using (postTransport.ConfigureAwait(false)) {