ProcessMessagesAsync's internal fire-and-forget task duplicates Completion's failure signal via TaskScheduler.UnobservedTaskException

[aymericbouzy]

Description

McpClientImpl.ConnectAsync starts the client's message-processing loop as a discarded task:

// src/ModelContextProtocol.Core/Client/McpClientImpl.cs:285
_ = _sessionHandler.ProcessMessagesAsync(CancellationToken.None);

That task runs McpSessionHandler.ProcessMessagesCoreAsync, which reads from _transport.MessageReader.ReadAllAsync(...) and only catches OperationCanceledException:

// src/ModelContextProtocol.Core/McpSessionHandler.cs:352
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
    // Normal shutdown
    LogEndpointMessageProcessingCanceled(EndpointName);
}

When the transport disconnects unexpectedly (e.g. the server 401s or otherwise closes the message channel mid-session), the transport already records this correctly — SetDisconnected(exception) completes the channel, and that same exception is exposed to callers through the documented, opt-in public API:

// McpClient.cs:64-75
/// For unexpected closure (e.g., process crash, network failure), it may contain
/// an exception that caused or that represents the failure.
public abstract Task<ClientCompletionDetails> Completion { get; }

ReadAllAsync then rethrows that same exception when it observes the completed-with-fault channel, and because ProcessMessagesCoreAsync's only catch clause is for cancellation, it propagates out of the discarded task. Since that task is never awaited or observed (_ =, CancellationToken.None), it becomes eligible for TaskScheduler.UnobservedTaskException, firing later on the GC finalizer thread with no request/caller context.

Why this is a duplication, not a missing feature

Completion already gives callers full control over how to react to this exact failure — await it, log it, retry, ignore it. That's the right design for something as opinionated as "what should happen when the transport dies." The problem is that the internal read loop independently re-surfaces the same failure through an unrelated, unobservable channel (TaskScheduler.UnobservedTaskException), which no caller asked for and can't configure. It's not that the SDK is missing a way to hand this back to the caller — it already has one — it's that this internal implementation detail leaks the same information a second time, through the worst possible channel (fires late, on the finalizer thread, disconnected from ConnectAsync's call site).

Impact

Consumers who wire up a global TaskScheduler.UnobservedTaskException handler (a common .NET practice for exactly this class of leak) get a contextless, delayed error report for a condition their code may already be aware of and had a designed way to observe via Completion. It reads as an SDK-internal unhandled bug rather than the already-surfaced session-completion condition it actually is.

Suggested fix

ProcessMessagesCoreAsync's read loop doesn't need to re-throw on transport disconnect — the transport already recorded why it disconnected, and that's retrievable via Completion. The loop exiting for that reason isn't a new failure to report, just a symptom of one already exposed elsewhere. A minimal fix: catch and swallow non-cancellation exceptions from the await foreach at the point the loop exits, since Completion (backed by the same SetDisconnected/channel-fault state) remains the sanctioned way for a caller to learn about and react to the disconnect:

catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
    // Normal shutdown
}
catch (Exception)
{
    // Already recorded via SetDisconnected and exposed through Completion;
    // the loop exiting is a symptom, not a new failure to surface here.
}

Happy to send a PR if this framing looks right — wanted to open the issue first per CONTRIBUTING.md and get agreement on the fix's direction before writing code, since it touches how the SDK's failure-reporting is supposed to work.

Version

ModelContextProtocol.Core 2.0.0

[jstar0]

I would like to work on this issue if it is still available. The intended scope would be to preserve the existing public Completion signal while ensuring the internal ProcessMessagesAsync task observes or routes transport failures without surfacing the same exception through TaskScheduler.UnobservedTaskException, with deterministic coverage for unexpected transport failure and normal cancellation. GitHub does not allow my account to self-assign this issue, so could a maintainer assign it if this scope is welcome?

[aymericbouzy]

I'm ready to do this as a first contribution as well 👍