Repository navigation
Treat a read whose handler threw OperationCanceledException as failed [patch] - #146
Merged
matt-edmondson merged 1 commit intoNov 10, 2026
Merged
Conversation
… [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 #144 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW45a4XVwC8wG7P95kY
|
matt-edmondson
deleted the
fix/handler-cancellation-is-a-failed-read-144
branch
November 10, 2026 23:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #144
What was wrong
ReadChunksisasync. When anOutputHandlerorLineOutputHandlerdelegate throwsOperationCanceledExceptionorTaskCanceledException, the read task ends Canceled, not Faulted.DrainOrAbandononly checkedIsFaulted, so it treated that read as a clean finish:ExecuteAsyncreturned the exit code (0) and dropped every chunk after the one that threw.write, andExecuteAsyncnever returned. This is the ExecuteAsync hangs forever (and leaves the child blocked) when the output handler throws or a strict encoding hits a decode error #89 hang, reached through a different exception type.What changed
DrainOrAbandonnow treatsIsFaulted || IsCanceledas a failed read. It abandons the other read and awaits the failed one, so the handler's own exception reaches the caller andRunAsync's existing catch kills the tree. Cancellation from the caller's token still goes through its owncancelledbranch and is unaffected. I usedIsCanceledrather thanIsCompletedSuccessfullybecause the latter doesn't exist on the netstandard2.0 target. CLAUDE.md's reader section now notes the case.Tests
ExecuteAsyncShouldThrowAndKillTheCommandWhenTheOutputHandlerThrowsOperationCanceledand...WhenALineOutputHandlerThrowsTaskCanceledreuseAssertAFaultedReadEndsTheCall, writing about 1 MB after the throwing chunk. They require the call to end within 10 s, finish Canceled with the handler's own exception (same message), and leave the command's pid gone.ExecuteAsyncShouldRethrowAnOperationCanceledExceptionFromTheOutputHandlerWhenTheOutputIsSmallcovers the small-output case: the exception is thrown instead of the exit code being returned, and delivery stops at the chunk that threw.Before the fix, all three new tests failed: the two large-output cases hit the 10 s bound with the child still blocked, and the small-output case got "no exception was thrown". After the fix the full suite passes on net10.0 / Linux: 93 passed, 0 failed, and the 2 elevation tests skipped as usual.
Generated by Claude Code