Fix process crash when testing Stderr

Author: ericstj

src/ModelContextProtocol.Core/Client/StdioClientTransport.cs

@@ -151,7 +151,17 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
                         stderrRollingLog.Enqueue(data);
                     }
 
-                    _options.StandardErrorLines?.Invoke(data);
+                    try
+                    {
+                        _options.StandardErrorLines?.Invoke(data);
+                    }
+                    catch (Exception ex)
+                    {
+                        // Prevent exceptions in the user callback from propagating
+                        // to the background thread that dispatches ErrorDataReceived,
+                        // which would crash the process.
+                        LogStderrCallbackFailed(logger, endpointName, ex);
+                    }
 
                     LogReadStderr(logger, endpointName, data);
                 }
@@ -230,12 +240,10 @@ internal static void DisposeProcess(
 
                 // Ensure all redirected stderr/stdout events have been dispatched
                 // before disposing. Only the no-arg WaitForExit() guarantees this;
-                // WaitForExit(int) (as used by KillTree) does not.
-                // This should not hang: either the process already exited on its own
-                // (no child processes holding handles), or KillTree killed the entire
-                // process tree. If it does take too long, the test infrastructure's
-                // own timeout will catch it.
-                if (!processRunning && HasExited(process))
+                // WaitForExit(int) (as used by KillTree) does not. We must call
+                // this whether the process exited on its own or was killed above,
+                // otherwise ErrorDataReceived handlers may fire after Dispose().
+                if (HasExited(process))
                 {
                     process.WaitForExit();
                 }
@@ -299,6 +307,9 @@ private static string EscapeArgumentString(string argument) =>
     [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} received stderr log: '{Data}'.")]
     private static partial void LogReadStderr(ILogger logger, string endpointName, string data);
 
+    [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} StandardErrorLines callback failed.")]
+    private static partial void LogStderrCallbackFailed(ILogger logger, string endpointName, Exception exception);
+
     [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} started server process with PID {ProcessId}.")]
     private static partial void LogTransportProcessStarted(ILogger logger, string endpointName, int processId);
 

tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs

@@ -59,6 +59,17 @@ public async Task CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked()
         Assert.Contains(id, sb.ToString());
     }
 
+    [Fact(Skip = "Platform not supported by this test.", SkipUnless = nameof(IsStdErrCallbackSupported))]
+    public async Task CreateAsync_StdErrCallbackThrows_DoesNotCrashProcess()
+    {
+        StdioClientTransport transport = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
+            new(new() { Command = "cmd", Arguments = ["/c", "echo fail >&2 & exit /b 1"], StandardErrorLines = _ => throw new InvalidOperationException("boom") }, LoggerFactory) :
+            new(new() { Command = "sh", Arguments = ["-c", "echo fail >&2; exit 1"], StandardErrorLines = _ => throw new InvalidOperationException("boom") }, LoggerFactory);
+
+        // Should throw IOException for the failed server, not crash the host process.
+        await Assert.ThrowsAnyAsync<IOException>(() => McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
+    }
+
     [Theory]
     [InlineData(null)]
     [InlineData("argument with spaces")]