From c7e65159495774502f89e5a4756cdec4bbe0c135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Akbar=20D=C4=B1zaj=C4=B1?= Date: Sun, 9 Aug 2026 10:16:25 +0400 Subject: [PATCH] Add opt-in exception summarization for server-side logging Adds McpServerOptions.ExceptionSummarizer, an optional delegate that maps an Exception to a sanitized description. When set, server-side failure paths log that description instead of attaching the raw Exception. When null (the default), every callsite logs exactly as before. The delegate runs only when the level of the event being written is enabled, matching the enabled-check the generated logging methods perform internally, and each summarized variant reuses its raw counterpart's EventName so both emit the same EventId. Core stays free of a hard dependency on Microsoft.Extensions.Diagnostics.ExceptionSummarization; the DI package takes the package reference and McpServerOptionsSetup populates the delegate from an optionally-registered IExceptionSummarizer. Fixes #1690 --- Directory.Packages.props | 1 + docs/concepts/logging/logging.md | 39 ++ .../ExceptionSummaryHelper.cs | 35 ++ .../McpSessionHandler.cs | 85 +++- .../Server/McpServerImpl.cs | 92 +++- .../Server/McpServerOptions.cs | 31 ++ .../McpServerOptionsSetup.cs | 22 +- .../ModelContextProtocol.csproj | 1 + .../Server/ExceptionSummarizationTests.cs | 410 ++++++++++++++++++ 9 files changed, 699 insertions(+), 17 deletions(-) create mode 100644 src/ModelContextProtocol.Core/ExceptionSummaryHelper.cs create mode 100644 tests/ModelContextProtocol.Tests/Server/ExceptionSummarizationTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index d204d871f..bfc9522a7 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -13,6 +13,7 @@ + diff --git a/docs/concepts/logging/logging.md b/docs/concepts/logging/logging.md index 2b381aa54..1fff75a50 100644 --- a/docs/concepts/logging/logging.md +++ b/docs/concepts/logging/logging.md @@ -85,3 +85,42 @@ Lastly, the client must configure a notification handler for , which most logging providers render as the exception message plus its stack trace. +That output can contain sensitive or overly detailed runtime data. + +Set to log a sanitized description instead. +When it is set, the failure paths log only the string the delegate returns, and the raw exception is not attached to +the log entry. The default is `null`, which preserves the existing behavior of logging the raw exception. + +```csharp +builder.Services.AddMcpServer(options => +{ + options.ExceptionSummarizer = ex => ex.GetType().Name; +}); +``` + +The summarized and raw forms of each event share one `EventId`, so filters and alerts keyed on `EventId` behave the +same whether or not a summarizer is configured. The delegate runs only when the corresponding log level is enabled, +so it costs nothing on a level that is filtered out. + +The `ModelContextProtocol` package also integrates with the standard +[Microsoft.Extensions.Diagnostics.ExceptionSummarization](https://learn.microsoft.com/dotnet/api/microsoft.extensions.diagnostics.exceptionsummarization) +abstractions. If an `IExceptionSummarizer` is registered in the container and `ExceptionSummarizer` has not been set +explicitly, the SDK populates it with `"{ExceptionType}: {Description}"` taken from the `ExceptionSummary`: + +```csharp +builder.Services.AddExceptionSummarizer(b => b.AddHttpProvider()); +builder.Services.AddMcpServer(); +``` + +Both of those fields are documented as free of privacy-sensitive information. `ExceptionSummary.AdditionalDetails` is +not, and `ExceptionSummary.ToString()` appends it, so neither is used. The exception type is included because +`Description` on its own is `"Unknown"` for exception types that no registered provider handles. + +If the delegate throws or returns `null`, the SDK falls back to logging the raw exception, so a faulty summarizer +can never fail the session. diff --git a/src/ModelContextProtocol.Core/ExceptionSummaryHelper.cs b/src/ModelContextProtocol.Core/ExceptionSummaryHelper.cs new file mode 100644 index 000000000..3f482c49f --- /dev/null +++ b/src/ModelContextProtocol.Core/ExceptionSummaryHelper.cs @@ -0,0 +1,35 @@ +using System.Diagnostics.CodeAnalysis; + +namespace ModelContextProtocol; + +/// +/// Provides the single, shared entry point used by exception-logging callsites to apply a +/// user-supplied exception summarizer. +/// +internal static class ExceptionSummaryHelper +{ + /// + /// Attempts to produce a sanitized description of using . + /// Callers check that a summarizer is configured, and that the event's level is enabled, before calling this. + /// + /// + /// if a summary was produced and the caller should log it in place of + /// ; otherwise, , in which case the caller must log + /// exactly as it would have without a summarizer. The summarizer is supplied + /// by the host, so throwing or returning both fall back rather than disrupt the session. + /// + public static bool TrySummarize(Func summarizer, Exception exception, [NotNullWhen(true)] out string? summary) + { + try + { + summary = summarizer(exception); + return summary is not null; + } + catch + { + // A faulty summarizer must never fail logging; fall back to the raw exception. + summary = null; + return false; + } + } +} diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index 61a1872f2..155362e43 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -95,6 +95,7 @@ internal static bool SupportsNaturalOutputSchemas(string? protocolVersion) /// private readonly ConcurrentDictionary _handlingRequests = new(); private readonly ILogger _logger; + private readonly Func? _exceptionSummarizer; // This _sessionId is solely used to identify the session in telemetry and logs. private readonly string _sessionId = Guid.NewGuid().ToString("N"); @@ -115,6 +116,10 @@ internal static bool SupportsNaturalOutputSchemas(string? protocolVersion) /// A filter that wraps incoming message processing. Takes the next handler and returns a wrapped handler. If null, a passthrough filter is used. /// A filter that wraps outgoing message processing. Takes the next handler and returns a wrapped handler. If null, a passthrough filter is used. /// The logger. + /// + /// An optional callback that produces a sanitized description of an exception. When non-, + /// exception logging callsites log that description instead of the raw . + /// public McpSessionHandler( bool isServer, ITransport transport, @@ -123,7 +128,8 @@ public McpSessionHandler( NotificationHandlers notificationHandlers, JsonRpcMessageFilter? incomingMessageFilter, JsonRpcMessageFilter? outgoingMessageFilter, - ILogger logger) + ILogger logger, + Func? exceptionSummarizer = null) { Throw.IfNull(transport); @@ -144,6 +150,7 @@ public McpSessionHandler( _incomingMessageFilter = incomingMessageFilter ?? (next => next); _outgoingMessageFilter = outgoingMessageFilter ?? (next => next); _logger = logger; + _exceptionSummarizer = exceptionSummarizer; // ping was removed in the 2026-07-28 protocol revision (SEP-2575). On the 2026-07-28 or later version, // return MethodNotFound; on an older version, the per-spec behavior is to always answer @@ -323,14 +330,7 @@ ex is OperationCanceledException && } else if (ex is not OperationCanceledException) { - if (_logger.IsEnabled(LogLevel.Trace)) - { - LogMessageHandlerExceptionSensitive(EndpointName, message.GetType().Name, JsonSerializer.Serialize(message, McpJsonUtilities.JsonContext.Default.JsonRpcMessage), ex); - } - else - { - LogMessageHandlerException(EndpointName, message.GetType().Name, ex); - } + LogMessageHandlerFailure(message, ex); } } finally @@ -470,7 +470,7 @@ await _incomingMessageFilter(async (msg, ct) => } catch (Exception ex) { - LogRequestHandlerException(EndpointName, request.Method, GetElapsed(requestStartingTimestamp).TotalMilliseconds, ex); + LogRequestHandlerFailure(EndpointName, request.Method, GetElapsed(requestStartingTimestamp).TotalMilliseconds, ex); throw; } @@ -1280,9 +1280,68 @@ internal static McpProtocolException CreateRemoteProtocolExceptionFromError(Json [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} method '{Method}' request handler completed in {ElapsedMilliseconds}ms.")] private partial void LogRequestHandlerCompleted(string endpointName, string method, double elapsedMilliseconds); + /// + /// Logs a failed request handler, substituting a sanitized description for the raw exception when a + /// summarizer is configured. The summarizer only runs when the event's level is enabled, matching the + /// enabled-check the generated logging methods perform internally. + /// + private void LogRequestHandlerFailure(string endpointName, string method, double elapsedMilliseconds, Exception exception) + { + if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Warning) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + LogRequestHandlerExceptionSummarized(endpointName, method, elapsedMilliseconds, exceptionSummary); + } + else + { + LogRequestHandlerException(endpointName, method, elapsedMilliseconds, exception); + } + } + + /// + /// Logs a failed message handler. The trace-vs-warning selection is unchanged from the non-summarizing + /// path; only the payload differs. The summarizer runs only when the selected event's level is enabled. + /// + private void LogMessageHandlerFailure(JsonRpcMessage message, Exception exception) + { + string messageType = message.GetType().Name; + + if (_logger.IsEnabled(LogLevel.Trace)) + { + if (_exceptionSummarizer is not null && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + LogMessageHandlerExceptionSensitiveSummarized(EndpointName, messageType, exceptionSummary, JsonSerializer.Serialize(message, McpJsonUtilities.JsonContext.Default.JsonRpcMessage)); + } + else + { + LogMessageHandlerExceptionSensitive(EndpointName, messageType, JsonSerializer.Serialize(message, McpJsonUtilities.JsonContext.Default.JsonRpcMessage), exception); + } + } + else if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Warning) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + LogMessageHandlerExceptionSummarized(EndpointName, messageType, exceptionSummary); + } + else + { + LogMessageHandlerException(EndpointName, messageType, exception); + } + } + + // Each summarized variant names its raw counterpart as its EventName so the pair emits one EventId and + // one event name, keeping consumers that filter on EventId working when a summarizer is configured. The + // generator derives the numeric id from the event name, which defaults to the method name, so only the + // summarized variant declares EventName: declaring it on both trips SYSLIB1025. + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} method '{Method}' request handler failed in {ElapsedMilliseconds}ms.")] private partial void LogRequestHandlerException(string endpointName, string method, double elapsedMilliseconds, Exception exception); + [LoggerMessage(Level = LogLevel.Warning, EventName = nameof(LogRequestHandlerException), Message = "{EndpointName} method '{Method}' request handler failed in {ElapsedMilliseconds}ms: {ExceptionSummary}.")] + private partial void LogRequestHandlerExceptionSummarized(string endpointName, string method, double elapsedMilliseconds, string exceptionSummary); + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} received request for unknown request ID '{RequestId}'.")] private partial void LogNoRequestFoundForMessageWithId(string endpointName, RequestId requestId); @@ -1313,9 +1372,15 @@ internal static McpProtocolException CreateRemoteProtocolExceptionFromError(Json [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} message handler {MessageType} failed.")] private partial void LogMessageHandlerException(string endpointName, string messageType, Exception exception); + [LoggerMessage(Level = LogLevel.Warning, EventName = nameof(LogMessageHandlerException), Message = "{EndpointName} message handler {MessageType} failed: {ExceptionSummary}.")] + private partial void LogMessageHandlerExceptionSummarized(string endpointName, string messageType, string exceptionSummary); + [LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} message handler {MessageType} failed. Message: '{Message}'.")] private partial void LogMessageHandlerExceptionSensitive(string endpointName, string messageType, string message, Exception exception); + [LoggerMessage(Level = LogLevel.Trace, EventName = nameof(LogMessageHandlerExceptionSensitive), Message = "{EndpointName} message handler {MessageType} failed: {ExceptionSummary}. Message: '{Message}'.")] + private partial void LogMessageHandlerExceptionSensitiveSummarized(string endpointName, string messageType, string exceptionSummary, string message); + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} received unexpected {MessageType} message type.")] private partial void LogEndpointHandlerUnexpectedMessageType(string endpointName, string messageType); diff --git a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs index f68aacdff..6188318a7 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs @@ -21,6 +21,7 @@ internal sealed partial class McpServerImpl : McpServer }; private readonly ILogger _logger; + private readonly Func? _exceptionSummarizer; private readonly ITransport _sessionTransport; private readonly bool _servicesScopePerRequest; private readonly List _disposables = []; @@ -88,6 +89,7 @@ public McpServerImpl(ITransport transport, McpServerOptions options, ILoggerFact _endpointName = _serverOnlyEndpointName; _servicesScopePerRequest = options.ScopeRequests; _logger = loggerFactory?.CreateLogger() ?? NullLogger.Instance; + _exceptionSummarizer = options.ExceptionSummarizer; _clientInfo = options.KnownClientInfo; _clientCapabilities = options.KnownClientCapabilities; @@ -161,7 +163,8 @@ void Register(McpServerPrimitiveCollection? collection, _notificationHandlers, incomingMessageFilter, outgoingMessageFilter, - _logger); + _logger, + _exceptionSummarizer); } /// @@ -1230,7 +1233,7 @@ await originalListResourceTemplatesHandler(request, cancellationToken).Configure } catch (Exception e) { - ReadResourceError(request.Params?.Uri ?? string.Empty, e); + LogReadResourceError(request.Params?.Uri ?? string.Empty, e); throw; } }); @@ -1344,7 +1347,7 @@ await originalListPromptsHandler(request, cancellationToken).ConfigureAwait(fals } catch (Exception e) { - GetPromptError(request.Params?.Name ?? string.Empty, e); + LogGetPromptError(request.Params?.Name ?? string.Empty, e); throw; } }); @@ -1597,7 +1600,7 @@ private McpRequestInvocationFilter handlerTask) } catch (Exception ex) { - MrtrHandlerError(ex); + LogMrtrHandlerError(ex); } finally { @@ -2407,21 +2410,95 @@ private async Task ObserveHandlerCompletionAsync(Task handlerTask) } } + // Each wrapper below gates the summarizer on the level declared by the event it replaces, so a + // host-supplied delegate never runs for a log entry that would be filtered out anyway. The generated + // logging methods perform the same enabled-check internally, so the raw path is unaffected. + + private void LogToolCallError(string toolName, Exception exception) + { + if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Error) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + ToolCallErrorSummarized(toolName, exceptionSummary); + } + else + { + ToolCallError(toolName, exception); + } + } + + private void LogGetPromptError(string promptName, Exception exception) + { + if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Error) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + GetPromptErrorSummarized(promptName, exceptionSummary); + } + else + { + GetPromptError(promptName, exception); + } + } + + private void LogReadResourceError(string resourceUri, Exception exception) + { + if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Error) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + ReadResourceErrorSummarized(resourceUri, exceptionSummary); + } + else + { + ReadResourceError(resourceUri, exception); + } + } + + private void LogMrtrHandlerError(Exception exception) + { + if (_exceptionSummarizer is not null && + _logger.IsEnabled(LogLevel.Debug) && + ExceptionSummaryHelper.TrySummarize(_exceptionSummarizer, exception, out string? exceptionSummary)) + { + MrtrHandlerErrorSummarized(exceptionSummary); + } + else + { + MrtrHandlerError(exception); + } + } + + // Each summarized variant names its raw counterpart as its EventName so the pair emits one EventId and + // one event name, keeping consumers that filter on EventId working when a summarizer is configured. The + // generator derives the numeric id from the event name, which defaults to the method name, so only the + // summarized variant declares EventName: declaring it on both trips SYSLIB1025. + [LoggerMessage(Level = LogLevel.Error, Message = "\"{ToolName}\" threw an unhandled exception.")] private partial void ToolCallError(string toolName, Exception exception); + [LoggerMessage(Level = LogLevel.Error, EventName = nameof(ToolCallError), Message = "\"{ToolName}\" threw an unhandled exception: {ExceptionSummary}.")] + private partial void ToolCallErrorSummarized(string toolName, string exceptionSummary); + [LoggerMessage(Level = LogLevel.Information, Message = "\"{ToolName}\" completed. IsError = {IsError}.")] private partial void ToolCallCompleted(string toolName, bool isError); [LoggerMessage(Level = LogLevel.Error, Message = "GetPrompt \"{PromptName}\" threw an unhandled exception.")] private partial void GetPromptError(string promptName, Exception exception); + [LoggerMessage(Level = LogLevel.Error, EventName = nameof(GetPromptError), Message = "GetPrompt \"{PromptName}\" threw an unhandled exception: {ExceptionSummary}.")] + private partial void GetPromptErrorSummarized(string promptName, string exceptionSummary); + [LoggerMessage(Level = LogLevel.Information, Message = "GetPrompt \"{PromptName}\" completed.")] private partial void GetPromptCompleted(string promptName); [LoggerMessage(Level = LogLevel.Error, Message = "ReadResource \"{ResourceUri}\" threw an unhandled exception.")] private partial void ReadResourceError(string resourceUri, Exception exception); + [LoggerMessage(Level = LogLevel.Error, EventName = nameof(ReadResourceError), Message = "ReadResource \"{ResourceUri}\" threw an unhandled exception: {ExceptionSummary}.")] + private partial void ReadResourceErrorSummarized(string resourceUri, string exceptionSummary); + [LoggerMessage(Level = LogLevel.Information, Message = "ReadResource \"{ResourceUri}\" completed.")] private partial void ReadResourceCompleted(string resourceUri); @@ -2431,6 +2508,9 @@ private async Task ObserveHandlerCompletionAsync(Task handlerTask) [LoggerMessage(Level = LogLevel.Debug, Message = "An MRTR handler threw an unhandled exception.")] private partial void MrtrHandlerError(Exception exception); + [LoggerMessage(Level = LogLevel.Debug, EventName = nameof(MrtrHandlerError), Message = "An MRTR handler threw an unhandled exception: {ExceptionSummary}.")] + private partial void MrtrHandlerErrorSummarized(string exceptionSummary); + [LoggerMessage(Level = LogLevel.Debug, Message = "Failed to deliver \"{NotificationMethod}\" to subscription \"{SubscriptionId}\".")] private partial void SubscriptionNotificationFailed(string notificationMethod, string subscriptionId, Exception exception); } diff --git a/src/ModelContextProtocol.Core/Server/McpServerOptions.cs b/src/ModelContextProtocol.Core/Server/McpServerOptions.cs index 2a26868a1..7d0ee0dc3 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerOptions.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerOptions.cs @@ -205,6 +205,37 @@ public McpServerFilters Filters [Obsolete(Obsoletions.DeprecatedSampling_Message, DiagnosticId = Obsoletions.Deprecated_DiagnosticId, UrlFormat = Obsoletions.Deprecated_Url)] public int MaxSamplingOutputTokens { get; set; } = 1000; + /// + /// Gets or sets an optional callback used to produce a sanitized description of an exception for logging. + /// + /// + /// A delegate that maps an to a short description to be logged in place of the + /// exception itself, or to log the raw . The default is + /// . + /// + /// + /// + /// By default, server-side failure paths pass the raw to , + /// which most logging providers render as the exception message plus its stack trace. That output can include + /// sensitive or overly detailed runtime data. Setting this property opts into logging only the string this + /// delegate returns; the raw is not attached to those log entries. + /// + /// + /// When the ModelContextProtocol package is used, this property is populated automatically from an + /// IExceptionSummarizer registered in the dependency injection container (for example, via + /// services.AddExceptionSummarizer()) if it has not already been set. + /// + /// + /// The delegate is invoked only when the log level of the event being written is enabled, and the summarized + /// entry carries the same as the raw one it replaces. + /// + /// + /// If the delegate throws or returns , the raw is logged instead, + /// so a faulty summarizer can never fail the session. + /// + /// + public Func? ExceptionSummarizer { get; set; } + /// /// Gets or sets custom request handlers to register with the server. /// diff --git a/src/ModelContextProtocol/McpServerOptionsSetup.cs b/src/ModelContextProtocol/McpServerOptionsSetup.cs index c46854460..167e0d969 100644 --- a/src/ModelContextProtocol/McpServerOptionsSetup.cs +++ b/src/ModelContextProtocol/McpServerOptionsSetup.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.ExceptionSummarization; using Microsoft.Extensions.Options; using ModelContextProtocol.Server; @@ -9,10 +11,12 @@ namespace ModelContextProtocol; /// The individually registered tools. /// The individually registered prompts. /// The individually registered resources. +/// The application's service provider, used to resolve optional services. internal sealed class McpServerOptionsSetup( IEnumerable serverTools, IEnumerable serverPrompts, - IEnumerable serverResources) : IConfigureOptions + IEnumerable serverResources, + IServiceProvider services) : IConfigureOptions { /// /// Configures the given McpServerOptions instance by setting server information @@ -67,5 +71,21 @@ public void Configure(McpServerOptions options) { options.ResourceCollection = resourceCollection; } + + // Default the summarizer from an optionally registered IExceptionSummarizer. An explicitly + // configured delegate always wins, whether it was set before this runs or by a later IConfigureOptions. + if (options.ExceptionSummarizer is null && + services.GetService() is { } summarizer) + { + options.ExceptionSummarizer = ex => + { + // ExceptionType and Description are both documented as free of privacy-sensitive information. + // AdditionalDetails is not, and ExceptionSummary.ToString() appends it, so neither is used here. + // Description alone is "Unknown" for exception types no provider handles, which would drop the + // type from the log entirely, so the type is prepended. + ExceptionSummary summary = summarizer.Summarize(ex); + return $"{summary.ExceptionType}: {summary.Description}"; + }; + } } } diff --git a/src/ModelContextProtocol/ModelContextProtocol.csproj b/src/ModelContextProtocol/ModelContextProtocol.csproj index 36c6a1736..bd2511093 100644 --- a/src/ModelContextProtocol/ModelContextProtocol.csproj +++ b/src/ModelContextProtocol/ModelContextProtocol.csproj @@ -37,6 +37,7 @@ + diff --git a/tests/ModelContextProtocol.Tests/Server/ExceptionSummarizationTests.cs b/tests/ModelContextProtocol.Tests/Server/ExceptionSummarizationTests.cs new file mode 100644 index 000000000..d1a5985f7 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/Server/ExceptionSummarizationTests.cs @@ -0,0 +1,410 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.ExceptionSummarization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using ModelContextProtocol.Tests.Utils; + +namespace ModelContextProtocol.Tests.Server; + +/// +/// Covers : the opt-in hook that logs a sanitized +/// description instead of the raw on server-side failure paths. +/// +public class ExceptionSummarizationTests : LoggedTest +{ + private const string HandlerFailureMessage = "handler blew up with sensitive details"; + private const string Summary = "sanitized-summary"; + + public ExceptionSummarizationTests(ITestOutputHelper testOutputHelper) + : base(testOutputHelper) + { + } + + private static McpServerOptions CreateOptions() => new() + { + ProtocolVersion = "2024-11-05", + InitializationTimeout = TimeSpan.FromSeconds(30), + Capabilities = new ServerCapabilities { Tools = new() }, + }; + + /// Records how many times it was invoked so tests can assert it never ran. + private sealed class CountingSummarizer + { + private int _invocations; + + public int Invocations => Volatile.Read(ref _invocations); + + public string Summarize(Exception exception) + { + Interlocked.Increment(ref _invocations); + return Summary; + } + } + + #region McpSessionHandler.LogRequestHandlerException + + [Fact] + public async Task RequestHandlerFailure_WithoutSummarizer_LogsRawException() + { + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + + var logs = await RunFailingListToolsRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.Message.Contains("request handler failed")); + Assert.Equal(LogLevel.Warning, log.LogLevel); + var exception = Assert.IsType(log.Exception); + Assert.Equal(HandlerFailureMessage, exception.Message); + Assert.DoesNotContain(Summary, log.Message); + } + + [Fact] + public async Task RequestHandlerFailure_WithSummarizer_LogsSummaryAndNoException() + { + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.ExceptionSummarizer = ex => Summary; + + var logs = await RunFailingListToolsRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.Message.Contains("request handler failed")); + Assert.Equal(LogLevel.Warning, log.LogLevel); + Assert.Contains(Summary, log.Message); + Assert.DoesNotContain(HandlerFailureMessage, log.Message); + Assert.Null(log.Exception); + } + + [Fact] + public async Task RequestHandlerFailure_WithThrowingSummarizer_FallsBackToRawException() + { + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.ExceptionSummarizer = ex => throw new NotSupportedException("summarizer is broken"); + + var logs = await RunFailingListToolsRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.Message.Contains("request handler failed")); + Assert.Equal(LogLevel.Warning, log.LogLevel); + var exception = Assert.IsType(log.Exception); + Assert.Equal(HandlerFailureMessage, exception.Message); + Assert.DoesNotContain("summarizer is broken", log.Message); + } + + [Fact] + public async Task RequestHandlerFailure_WithNullReturningSummarizer_FallsBackToRawException() + { + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.ExceptionSummarizer = ex => null!; + + var logs = await RunFailingListToolsRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.Message.Contains("request handler failed")); + Assert.IsType(log.Exception); + } + + #endregion + + #region McpServerImpl.ToolCallError + + [Fact] + public async Task ToolCallFailure_WithoutSummarizer_LogsRawException() + { + var options = CreateOptions(); + options.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + + var logs = await RunFailingCallToolRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.LogLevel == LogLevel.Error); + Assert.Equal("\"\" threw an unhandled exception.", log.Message); + var exception = Assert.IsType(log.Exception); + Assert.Equal(HandlerFailureMessage, exception.Message); + } + + [Fact] + public async Task ToolCallFailure_WithSummarizer_LogsSummaryAndNoException() + { + var options = CreateOptions(); + options.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + options.ExceptionSummarizer = ex => Summary; + + var logs = await RunFailingCallToolRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.LogLevel == LogLevel.Error); + Assert.Equal($"\"\" threw an unhandled exception: {Summary}.", log.Message); + Assert.Null(log.Exception); + } + + [Fact] + public async Task ToolCallFailure_WithThrowingSummarizer_FallsBackToRawException() + { + var options = CreateOptions(); + options.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + options.ExceptionSummarizer = ex => throw new NotSupportedException("summarizer is broken"); + + var logs = await RunFailingCallToolRequestAsync(options); + + var log = Assert.Single(logs.LogMessages, m => m.LogLevel == LogLevel.Error); + Assert.Equal("\"\" threw an unhandled exception.", log.Message); + Assert.IsType(log.Exception); + } + + #endregion + + #region The summarizer must not run when the event's level is disabled + + [Fact] + public async Task RequestHandlerFailure_WhenWarningDisabled_DoesNotInvokeSummarizer() + { + CountingSummarizer summarizer = new(); + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.ExceptionSummarizer = summarizer.Summarize; + + // LogRequestHandlerException is Warning; a Critical minimum filters it out, so nothing is emitted. + var logs = await RunFailingListToolsRequestAsync(options, LogLevel.Critical); + + Assert.Empty(logs.LogMessages); + Assert.Equal(0, summarizer.Invocations); + } + + [Fact] + public async Task RequestHandlerFailure_WhenWarningEnabled_InvokesSummarizer() + { + CountingSummarizer summarizer = new(); + var options = CreateOptions(); + options.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.ExceptionSummarizer = summarizer.Summarize; + + var logs = await RunFailingListToolsRequestAsync(options, LogLevel.Warning); + + Assert.Single(logs.LogMessages, m => m.Message.Contains("request handler failed") && m.Message.Contains(Summary)); + Assert.Equal(1, summarizer.Invocations); + } + + [Fact] + public async Task ToolCallFailure_WhenErrorDisabled_DoesNotInvokeSummarizer() + { + CountingSummarizer summarizer = new(); + var options = CreateOptions(); + options.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + options.ExceptionSummarizer = summarizer.Summarize; + + // ToolCallError is Error; a Critical minimum filters it out, so nothing is emitted. + var logs = await RunFailingCallToolRequestAsync(options, LogLevel.Critical); + + Assert.Empty(logs.LogMessages); + Assert.Equal(0, summarizer.Invocations); + } + + [Fact] + public async Task ToolCallFailure_WhenErrorEnabled_InvokesSummarizer() + { + CountingSummarizer summarizer = new(); + var options = CreateOptions(); + options.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + options.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + options.ExceptionSummarizer = summarizer.Summarize; + + var logs = await RunFailingCallToolRequestAsync(options, LogLevel.Error); + + Assert.Single(logs.LogMessages, m => m.Message == $"\"\" threw an unhandled exception: {Summary}."); + Assert.Equal(1, summarizer.Invocations); + } + + #endregion + + #region EventId identity is preserved across raw and summarized variants + + [Fact] + public async Task RequestHandlerFailure_EventIdIsIdenticalWithAndWithoutSummarizer() + { + var rawOptions = CreateOptions(); + rawOptions.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + var rawLogs = await RunFailingListToolsRequestAsync(rawOptions); + var raw = Assert.Single(rawLogs.LogMessages, m => m.Message.Contains("request handler failed")); + + var summarizedOptions = CreateOptions(); + summarizedOptions.Handlers.ListToolsHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + summarizedOptions.ExceptionSummarizer = ex => Summary; + var summarizedLogs = await RunFailingListToolsRequestAsync(summarizedOptions); + var summarized = Assert.Single(summarizedLogs.LogMessages, m => m.Message.Contains("request handler failed")); + + // Different rendering, same logical event, so EventId-based filtering keeps working. + Assert.NotEqual(raw.Message, summarized.Message); + Assert.Equal(raw.EventId, summarized.EventId); + Assert.Equal(raw.EventId.Id, summarized.EventId.Id); + Assert.Equal(raw.EventId.Name, summarized.EventId.Name); + } + + [Fact] + public async Task ToolCallFailure_EventIdIsIdenticalWithAndWithoutSummarizer() + { + var rawOptions = CreateOptions(); + rawOptions.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + rawOptions.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + var rawLogs = await RunFailingCallToolRequestAsync(rawOptions); + var raw = Assert.Single(rawLogs.LogMessages, m => m.LogLevel == LogLevel.Error); + + var summarizedOptions = CreateOptions(); + summarizedOptions.Handlers.CallToolHandler = (request, ct) => throw new InvalidOperationException(HandlerFailureMessage); + summarizedOptions.Handlers.ListToolsHandler = (request, ct) => throw new NotImplementedException(); + summarizedOptions.ExceptionSummarizer = ex => Summary; + var summarizedLogs = await RunFailingCallToolRequestAsync(summarizedOptions); + var summarized = Assert.Single(summarizedLogs.LogMessages, m => m.LogLevel == LogLevel.Error); + + Assert.NotEqual(raw.Message, summarized.Message); + Assert.Equal(raw.EventId, summarized.EventId); + Assert.Equal(raw.EventId.Id, summarized.EventId.Id); + Assert.Equal(raw.EventId.Name, summarized.EventId.Name); + } + + #endregion + + #region Dependency injection wiring + + [Fact] + public async Task OptionsSetup_WithoutRegisteredSummarizer_LeavesDelegateNull() + { + ServiceCollection services = new(); + services.AddMcpServer(); + + await using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + Assert.Null(options.ExceptionSummarizer); + } + + [Fact] + public async Task OptionsSetup_WithRegisteredSummarizer_PrependsExceptionTypeToDescription() + { + ServiceCollection services = new(); + services.AddSingleton(new FakeExceptionSummarizer(Summary)); + services.AddMcpServer(); + + await using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + Assert.NotNull(options.ExceptionSummarizer); + Assert.Equal( + $"InvalidOperationException: {Summary}", + options.ExceptionSummarizer(new InvalidOperationException(HandlerFailureMessage))); + } + + [Fact] + public async Task OptionsSetup_WithAddExceptionSummarizer_PreservesExceptionTypeAndOmitsMessage() + { + ServiceCollection services = new(); + services.AddExceptionSummarizer(); + services.AddMcpServer(); + + await using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + Assert.NotNull(options.ExceptionSummarizer); + + // The built-in summarizer has no provider for InvalidOperationException, so Description alone is + // "Unknown". The exception type must still survive, and the sensitive message must not appear. + string summary = options.ExceptionSummarizer(new InvalidOperationException(HandlerFailureMessage)); + Assert.StartsWith("InvalidOperationException:", summary); + Assert.DoesNotContain(HandlerFailureMessage, summary); + } + + [Fact] + public async Task OptionsSetup_WithExplicitlyConfiguredDelegate_DoesNotOverwriteIt() + { + ServiceCollection services = new(); + services.AddSingleton(new FakeExceptionSummarizer(Summary)); + services.AddMcpServer(options => options.ExceptionSummarizer = ex => "explicit"); + + await using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + Assert.NotNull(options.ExceptionSummarizer); + Assert.Equal("explicit", options.ExceptionSummarizer(new InvalidOperationException())); + } + + private sealed class FakeExceptionSummarizer(string description) : IExceptionSummarizer + { + public ExceptionSummary Summarize(Exception exception) => + new(exception.GetType().Name, description, string.Empty); + } + + #endregion + + private async Task RunFailingListToolsRequestAsync( + McpServerOptions options, LogLevel minimumLevel = LogLevel.Debug) + { + MockLoggerProvider logs = new(); + using ILoggerFactory loggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder => + { + builder.AddProvider(logs); + builder.SetMinimumLevel(minimumLevel); + }); + + await using var transport = new TestServerTransport(); + await using var server = McpServer.Create(transport, options, loggerFactory); + var runTask = server.RunAsync(TestContext.Current.CancellationToken); + + var receivedError = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + transport.OnMessageSent = message => + { + if (message is JsonRpcError error) + { + receivedError.TrySetResult(error); + } + }; + + await transport.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.ToolsList, Id = new RequestId(1) }, + TestContext.Current.CancellationToken); + + await receivedError.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + + await transport.DisposeAsync(); + await runTask; + + return logs; + } + + private async Task RunFailingCallToolRequestAsync( + McpServerOptions options, LogLevel minimumLevel = LogLevel.Debug) + { + MockLoggerProvider logs = new(); + using ILoggerFactory loggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder => + { + builder.AddProvider(logs); + builder.SetMinimumLevel(minimumLevel); + }); + + await using var transport = new TestServerTransport(); + await using var server = McpServer.Create(transport, options, loggerFactory); + var runTask = server.RunAsync(TestContext.Current.CancellationToken); + + var receivedResponse = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + transport.OnMessageSent = message => + { + if (message is JsonRpcResponse response) + { + receivedResponse.TrySetResult(response); + } + }; + + await transport.SendMessageAsync( + new JsonRpcRequest { Method = RequestMethods.ToolsCall, Id = new RequestId(1) }, + TestContext.Current.CancellationToken); + + await receivedResponse.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + + await transport.DisposeAsync(); + await runTask; + + return logs; + } +}