Improve exception diagnostics for stdio client

src/ModelContextProtocol/Protocol/Transport/SseClientSessionTransport.cs

@@ -137,7 +137,7 @@ private async Task CloseAsync()
         }
         finally
         {
-            SetConnected(false);
+            SetDisconnected();
         }
     }
 
@@ -203,7 +203,7 @@ private async Task ReceiveMessagesAsync(CancellationToken cancellationToken)
         }
         finally
         {
-            SetConnected(false);
+            SetDisconnected();
         }
     }
 
@@ -251,7 +251,7 @@ private void HandleEndpointEvent(string data)
         _messageEndpoint = new Uri(_sseEndpoint, data);
 
         // Set connected state
-        SetConnected(true);
+        SetConnected();
         _connectionEstablished.TrySetResult(true);
     }
 

src/ModelContextProtocol/Protocol/Transport/StdioClientSessionTransport.cs

@@ -1,6 +1,5 @@
 using Microsoft.Extensions.Logging;
 using ModelContextProtocol.Protocol.Messages;
-using System;
 using System.Diagnostics;
 
 namespace ModelContextProtocol.Protocol.Transport;
@@ -10,54 +9,51 @@ internal sealed class StdioClientSessionTransport : StreamClientSessionTransport
 {
     private readonly StdioClientTransportOptions _options;
     private readonly Process _process;
+    private readonly Queue<string> _stderrRollingLog;
+    private int _cleanedUp = 0;
 
-    public StdioClientSessionTransport(StdioClientTransportOptions options, Process process, string endpointName, ILoggerFactory? loggerFactory)
+    public StdioClientSessionTransport(StdioClientTransportOptions options, Process process, string endpointName, Queue<string> stderrRollingLog, ILoggerFactory? loggerFactory)
         : base(process.StandardInput, process.StandardOutput, endpointName, loggerFactory)
     {
         _process = process;
         _options = options;
+        _stderrRollingLog = stderrRollingLog;
     }
 
     /// <inheritdoc/>
-    /// <remarks>
-    /// <para>
-    /// For stdio-based transports, this implementation first verifies that the underlying process 
-    /// is still running before attempting to send the message. If the process has exited or cannot
-    /// be accessed, a <see cref="InvalidOperationException"/> is thrown with details about the failure.
-    /// </para>
-    /// <para>
-    /// After verifying the process state, this method delegates to the base class implementation
-    /// to handle the actual message serialization and transmission to the process's standard input stream.
-    /// </para>
-    /// </remarks>
-    /// <exception cref="InvalidOperationException">
-    /// Thrown when the underlying process has exited or cannot be accessed.
-    /// </exception>
     public override async Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
     {
-        Exception? processException = null;
-        bool hasExited = false;
         try
         {
-            hasExited = _process.HasExited;
+            await base.SendMessageAsync(message, cancellationToken);
         }
-        catch (Exception e)
+        catch (IOException)
         {
-            processException = e;
-            hasExited = true;
-        }
+            // We failed to send due to an I/O error. If the server process has exited, which is then very likely the cause
+            // for the I/O error, we should throw an exception for that instead.
+            if (await GetUnexpectedExitExceptionAsync(cancellationToken).ConfigureAwait(false) is Exception processExitException)
+            {
+                throw processExitException;
+            }
 
-        if (hasExited)
-        {
-            throw new InvalidOperationException("Transport is not connected", processException);
+            throw;
         }
-
-        await base.SendMessageAsync(message, cancellationToken).ConfigureAwait(false);
     }
 
     /// <inheritdoc/>
-    protected override ValueTask CleanupAsync(CancellationToken cancellationToken)
+    protected override async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default)
     {
+        // Only clean up once.
+        if (Interlocked.Exchange(ref _cleanedUp, 1) != 0)
+        {
+            return;
+        }
+
+        // We've not yet forcefully terminated the server. If it's already shut down, something went wrong,
+        // so create an exception with details about that.
+        error ??= await GetUnexpectedExitExceptionAsync(cancellationToken).ConfigureAwait(false);
+
+        // Now terminate the server process.
         try
         {
             StdioClientTransport.DisposeProcess(_process, processRunning: true, _options.ShutdownTimeout, Name);
@@ -67,6 +63,49 @@ protected override ValueTask CleanupAsync(CancellationToken cancellationToken)
             LogTransportShutdownFailed(Name, ex);
         }
 
-        return base.CleanupAsync(cancellationToken);
+        // And handle cleanup in the base type.
+        await base.CleanupAsync(error, cancellationToken);
+    }
+
+    private async ValueTask<Exception?> GetUnexpectedExitExceptionAsync(CancellationToken cancellationToken)
+    {
+        if (!StdioClientTransport.HasExited(_process))
+        {
+            return null;
+        }
+
+        Debug.Assert(StdioClientTransport.HasExited(_process));
+        try
+        {
+            // The process has exited, but we still need to ensure stderr has been flushed.
+#if NET
+            await _process.WaitForExitAsync(cancellationToken).ConfigureAwait(false);
+#else
+            _process.WaitForExit();
+#endif
+        }
+        catch { }
+
+        string errorMessage = "MCP server process exited unexpectedly";
+
+        string? exitCode = null;
+        try
+        {
+            exitCode = $" (exit code: {(uint)_process.ExitCode})";
+        }
+        catch { }
+
+        lock (_stderrRollingLog)
+        {
+            if (_stderrRollingLog.Count > 0)
+            {
+                errorMessage =
+                    $"{errorMessage}{exitCode}{Environment.NewLine}" +
+                    $"Server's stderr tail:{Environment.NewLine}" +
+                    $"{string.Join(Environment.NewLine, _stderrRollingLog)}";
+            }
+        }
+
+        return new IOException(errorMessage);
     }
 }

src/ModelContextProtocol/Protocol/Transport/StdioClientTransport.cs

@@ -127,8 +127,28 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
 
             process = new() { StartInfo = startInfo };
 
-            // Set up error logging
-            process.ErrorDataReceived += (sender, args) => LogReadStderr(logger, endpointName, args.Data ?? "(no data)");
+            // Set up stderr handling. Log all stderr output, and keep the last
+            // few lines in a rolling log for use in exceptions.
+            const int MaxStderrLength = 10; // keep the last 10 lines of stderr
+            Queue<string> stderrRollingLog = new(MaxStderrLength);
+            process.ErrorDataReceived += (sender, args) =>
+            {
+                string? data = args.Data;
+                if (data is not null)
+                {
+                    lock (stderrRollingLog)
+                    {
+                        if (stderrRollingLog.Count >= MaxStderrLength)
+                        {
+                            stderrRollingLog.Dequeue();
+                        }
+
+                        stderrRollingLog.Enqueue(data);
+                    }
+
+                    LogReadStderr(logger, endpointName, data);
+                }
+            };
 
             // We need both stdin and stdout to use a no-BOM UTF-8 encoding. On .NET Core,
             // we can use ProcessStartInfo.StandardOutputEncoding/StandardInputEncoding, but
@@ -154,14 +174,14 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
             if (!processStarted)
             {
                 LogTransportProcessStartFailed(logger, endpointName);
-                throw new InvalidOperationException("Failed to start MCP server process");
+                throw new IOException("Failed to start MCP server process.");
             }
 
             LogTransportProcessStarted(logger, endpointName, process.Id);
 
             process.BeginErrorReadLine();
 
-            return new StdioClientSessionTransport(_options, process, endpointName, _loggerFactory);
+            return new StdioClientSessionTransport(_options, process, endpointName, stderrRollingLog, _loggerFactory);
         }
         catch (Exception ex)
         {
@@ -176,7 +196,7 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
                 LogTransportShutdownFailed(logger, endpointName, ex2);
             }
 
-            throw new InvalidOperationException("Failed to connect transport", ex);
+            throw new IOException("Failed to connect transport.", ex);
         }
     }
 
@@ -185,20 +205,9 @@ internal static void DisposeProcess(
     {
         if (process is not null)
         {
-            if (processRunning)
-            {
-                try
-                {
-                    processRunning = !process.HasExited;
-                }
-                catch
-                {
-                    processRunning = false;
-                }
-            }
-
             try
             {
+                processRunning = processRunning && !HasExited(process);
                 if (processRunning)
                 {
                     // Wait for the process to exit.
@@ -214,6 +223,19 @@ internal static void DisposeProcess(
         }
     }
 
+    /// <summary>Gets whether <paramref name="process"/> has exited.</summary>
+    internal static bool HasExited(Process process)
+    {
+        try
+        {
+            return process.HasExited;
+        }
+        catch
+        {
+            return true;
+        }
+    }
+
     [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} connecting.")]
     private static partial void LogTransportConnecting(ILogger logger, string endpointName);
 

src/ModelContextProtocol/Protocol/Transport/StreamClientSessionTransport.cs

@@ -53,17 +53,12 @@ public StreamClientSessionTransport(
         _readTask = readTask.Unwrap();
         readTask.Start();
 
-        SetConnected(true);
+        SetConnected();
     }
 
     /// <inheritdoc/>
     public override async Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
     {
-        if (!IsConnected)
-        {
-            throw new InvalidOperationException("Transport is not connected");
-        }
-
         string id = "(no id)";
         if (message is JsonRpcMessageWithId messageWithId)
         {
@@ -82,31 +77,22 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation
         catch (Exception ex)
         {
             LogTransportSendFailed(Name, id, ex);
-            throw new InvalidOperationException("Failed to send message", ex);
+            throw new IOException("Failed to send message.", ex);
         }
     }
 
     /// <inheritdoc/>
-    /// <summary>
-    /// Asynchronously releases all resources used by the stream client session transport.
-    /// </summary>
-    /// <returns>A task that represents the asynchronous dispose operation.</returns>
-    /// <remarks>
-    /// This method cancels ongoing operations and waits for the read task to complete
-    /// before marking the transport as disconnected. It calls <see cref="CleanupAsync"/> 
-    /// to perform the actual cleanup work.
-    /// After disposal, the transport can no longer be used to send or receive messages.
-    /// </remarks>
-    public override ValueTask DisposeAsync() =>
-        CleanupAsync(CancellationToken.None);
+    public override ValueTask DisposeAsync() => 
+        CleanupAsync(cancellationToken: CancellationToken.None);
 
     private async Task ReadMessagesAsync(CancellationToken cancellationToken)
     {
+        Exception? error = null;
         try
         {
             LogTransportEnteringReadMessagesLoop(Name);
 
-            while (!cancellationToken.IsCancellationRequested)
+            while (true)
             {
                 if (await _serverOutput.ReadLineAsync(cancellationToken).ConfigureAwait(false) is not string line)
                 {
@@ -130,12 +116,13 @@ private async Task ReadMessagesAsync(CancellationToken cancellationToken)
         }
         catch (Exception ex)
         {
+            error = ex;
             LogTransportReadMessagesFailed(Name, ex);
         }
         finally
         {
             _readTask = null;
-            await CleanupAsync(cancellationToken).ConfigureAwait(false);
+            await CleanupAsync(error, cancellationToken).ConfigureAwait(false);
         }
     }
 
@@ -166,7 +153,7 @@ private async Task ProcessMessageAsync(string line, CancellationToken cancellati
         }
     }
 
-    protected virtual async ValueTask CleanupAsync(CancellationToken cancellationToken)
+    protected virtual async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default)
     {
         LogTransportShuttingDown(Name);
 
@@ -191,7 +178,7 @@ protected virtual async ValueTask CleanupAsync(CancellationToken cancellationTok
             }
         }
 
-        SetConnected(false);
+        SetDisconnected(error);
         LogTransportShutDown(Name);
     }
 }