From f77f40cdcbaff6d4994711cc071bbc3a03d50cb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Nov 2026 05:27:49 +0000 Subject: [PATCH] Treat a read whose handler threw OperationCanceledException as failed [patch] A handler that throws OperationCanceledException or TaskCanceledException ends its async read Canceled, not Faulted. DrainOrAbandon checked only IsFaulted, so it took that read for a clean finish: with small output ExecuteAsync returned the exit code as if the run had succeeded, and with more output than the pipe holds it hung on a command blocked writing to the pipe nobody drained any more (the #89 hang through another exception type). A Canceled read now abandons the other read and is awaited like a Faulted one, so the handler's own exception reaches the caller and RunAsync kills the tree. Cancellation from the caller's token still has its own branch and never arrives through a read. Fixes ktsu-dev/RunCommand#144 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW45a4XVwC8wG7P95kY --- CLAUDE.md | 3 ++ RunCommand.Test/RunCommandTests.cs | 57 ++++++++++++++++++++++++-- RunCommand/AsyncProcessStreamReader.cs | 8 +++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ca78b82..c54c72f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,6 +156,9 @@ A read that fails (the handler throws, or a strict `Encoding` rejects the bytes) pipe, so the command blocks as soon as that pipe fills and never exits. The reader therefore throws the first failure without waiting for the other stream, and `RunAsync` kills the process tree before rethrowing it rather than waiting for an exit that will not come. +A handler that throws `OperationCanceledException` ends its read Canceled rather than Faulted, and +counts as a failed read all the same. Cancellation from the caller's token never arrives through a +read; it has its own branch in `DrainOrAbandon`. ### Process configuration diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 3a202a5..7222491 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -1471,6 +1471,40 @@ await AssertAFaultedReadEndsTheCall( @"printf '\377\n'", new OutputHandler(encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true))).ConfigureAwait(false); + [TestMethod] + public async Task ExecuteAsyncShouldThrowAndKillTheCommandWhenTheOutputHandlerThrowsOperationCanceled() => + await AssertAFaultedReadEndsTheCall( + "echo first", + new OutputHandler(_ => throw new OperationCanceledException(HandlerCancellationMessage)), + expectHandlerCancellation: true).ConfigureAwait(false); + + [TestMethod] + public async Task ExecuteAsyncShouldThrowAndKillTheCommandWhenALineOutputHandlerThrowsTaskCanceled() => + await AssertAFaultedReadEndsTheCall( + "echo first", + new LineOutputHandler(_ => throw new TaskCanceledException(HandlerCancellationMessage)), + expectHandlerCancellation: true).ConfigureAwait(false); + + [TestMethod] + public async Task ExecuteAsyncShouldRethrowAnOperationCanceledExceptionFromTheOutputHandlerWhenTheOutputIsSmall() + { + List delivered = []; + OperationCanceledException thrown = await Assert.ThrowsAsync( + () => RunCommand.ExecuteAsync( + "sh", + ["-c", "echo one; sleep 0.2; echo two"], + new OutputHandler(chunk => + { + delivered.Add(chunk); + throw new OperationCanceledException(HandlerCancellationMessage); + }))).ConfigureAwait(false); + + Assert.AreEqual(HandlerCancellationMessage, thrown.Message, "Expected the handler's own exception, not one standing in for it."); + Assert.HasCount(1, delivered, "Expected delivery to stop at the chunk whose handler threw."); + } + + private const string HandlerCancellationMessage = "thrown by the output handler"; + /// /// Runs a command that makes the first read fault and then writes far more than a pipe holds, /// and requires the call to rethrow that fault promptly with the command no longer running. @@ -1479,7 +1513,15 @@ await AssertAFaultedReadEndsTheCall( /// A faulted read stops draining its pipe, so the command blocks writing to it and never exits. /// Waiting for that exit is what hung the call. /// - private static async Task AssertAFaultedReadEndsTheCall(string faultingOutput, OutputHandler handler, [CallerMemberName] string testName = "") + /// Shell that produces the output whose read fails. + /// The handler the command's output goes to. + /// + /// Whether the handler fails by throwing with + /// . That ends its read Canceled rather than Faulted, and + /// the call has to rethrow it as itself. + /// + /// Names the pid file. + private static async Task AssertAFaultedReadEndsTheCall(string faultingOutput, OutputHandler handler, bool expectHandlerCancellation = false, [CallerMemberName] string testName = "") { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { @@ -1502,8 +1544,17 @@ private static async Task AssertAFaultedReadEndsTheCall(string faultingOutput, O Task finished = await Task.WhenAny(execution, Task.Delay(TimeSpan.FromSeconds(10))).ConfigureAwait(false); Assert.AreSame(execution, finished, "Expected a faulted read to end the call rather than wait on a command blocked writing to it."); - Assert.IsTrue(execution.IsFaulted, "Expected the fault to reach the caller."); - Assert.IsNotInstanceOfType(execution.Exception!.InnerException); + if (expectHandlerCancellation) + { + Assert.IsTrue(execution.IsCanceled, $"Expected the handler's cancellation to reach the caller, but the call {execution.Status}."); + OperationCanceledException thrown = await Assert.ThrowsAsync(() => execution).ConfigureAwait(false); + Assert.AreEqual(HandlerCancellationMessage, thrown.Message, "Expected the handler's own exception, not one standing in for it."); + } + else + { + Assert.IsTrue(execution.IsFaulted, "Expected the fault to reach the caller."); + Assert.IsNotInstanceOfType(execution.Exception!.InnerException); + } string pid = (await File.ReadAllTextAsync(pidFile).ConfigureAwait(false)).Trim(); Assert.IsFalse(IsRunning(pid), $"Expected the command (pid {pid}) to have been killed."); diff --git a/RunCommand/AsyncProcessStreamReader.cs b/RunCommand/AsyncProcessStreamReader.cs index a529429..36ed9a1 100644 --- a/RunCommand/AsyncProcessStreamReader.cs +++ b/RunCommand/AsyncProcessStreamReader.cs @@ -105,7 +105,13 @@ private static async Task DrainOrAbandon(Task outputTask, Task errorTask, return false; } - if (finished.IsFaulted) + // Canceled counts as a failure too. A handler that throws OperationCanceledException (its + // own token's ThrowIfCancellationRequested, or a call it awaited timing out) ends its read + // Canceled rather than Faulted. Taken for a clean finish, that read stopped draining its + // pipe while the loop went on waiting for the other one, so the run either reported + // success or hung on a command blocked writing. Cancellation from the caller's own token + // arrives through `cancelled` above, never through a read. + if (finished.IsFaulted || finished.IsCanceled) { Abandon(outputTask, errorTask);