docs: describe every source of MonitoredPipe#exception - #201
Merged
Merged
Conversation
The docs for #exception said it holds "the exception raised by a destination", but the attribute records any failure while collecting output: a destination #write raising, the monitor loop raising, or pipe cleanup raising. A user inspecting the cause of a ProcessIOError was told a narrower story than the truth. Broaden the attribute docs and the matching class-level paragraph, and state two behaviors that were previously unwritten: the first recorded exception wins, and in the double-failure corner (monitor loop raises, then cleanup also raises) the cleanup error is the one recorded because the monitor-loop exception reaches its recording site last. The #close docs were verified against the issue's criteria and need no change; their wording is already source-neutral. No behavior change.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only, and the updated wording matches the verified exception-recording behavior in MonitoredPipe without introducing any code or test risk.
Pull request overview
This PR updates YARD documentation in ProcessExecuter::MonitoredPipe to accurately describe all sources and precedence behavior of MonitoredPipe#exception, aligning user-facing docs with the actual failure-recording behavior used by Commands::Run when setting ProcessIOError#cause.
Changes:
- Broadens the class-level documentation to state that
{#exception}may come from destination writes, the monitor loop, or pipe cleanup. - Expands the
#exceptionattribute docs to document “first recorded wins” and the specific double-failure corner where cleanup can supersede a monitor-loop exception. - Updates the
@returndescription to match the broader meaning (“first recorded exception”).
File summaries
| File | Description |
|---|---|
| lib/process_executer/monitored_pipe.rb | Expands YARD docs for MonitoredPipe#exception and related class-level wording to reflect all recording sites and precedence behavior. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Implements #193 (docs-only scope).
The docs for
MonitoredPipe#exceptionsaid it holds "the exception raised by a destination", but the attribute records any failure while collecting output: a destination#writeraising, the monitor loop raising, or pipe cleanup raising.Commands::Runreports the recorded exception as thecauseof aProcessIOError, so the narrow wording reached users.This PR broadens the attribute docs and the matching class-level paragraph, and writes down two behaviors that had no documentation: the first recorded exception wins, and in the double-failure corner (monitor loop raises, then cleanup also raises) the cleanup error is the one recorded. The
#closedocs were checked per the issue's acceptance criteria and need no change. No code or spec changes.Validation
bundle exec rakepasses (100% line coverage, no RuboCop or yardstick offenses, 100% YARD coverage)bundle exec rakepassesbundle exec rakepassesCloses #193