Document stream ownership and v2 early-close behavior#57
Open
znull wants to merge 9 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR documents a behavioral difference between go-pipe v1 and v2 around early-consumer exit and producer-side pipe write errors, and adds/adjusts tests to validate the v2 behavior (including how IgnoreError(..., IsPipeError) can be used to safely ignore these errors for stateless producers).
Changes:
- Document v2’s more direct surfacing of producer-side pipe errors when downstream stages stop reading early, with guidance/patterns for stateful producers.
- Remove Windows-specific skips from several pipeline tests that shell out to Unix utilities, and drop the
runtimeimport. - Add new tests asserting (a) Go producers can observe pipe errors when a command exits without reading stdin and (b)
IgnoreError(IsPipeError)allows stateful producers to finish processing.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds v1→v2 migration note explaining early-close pipe errors and how to handle them. |
| pipe/pipeline_test.go | Updates pipeline tests (removes Windows skips) and adds new coverage for producer-side pipe errors + IgnoreError(IsPipeError) behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 9
Replace Stage.Start close flags with InputStream and OutputStream types. The endpoint constructors encode whether a stream is closing while preserving the underlying reader/writer type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add regression coverage for the go-pipe v2 behavior where a Go producer can see a pipe error directly when a downstream command exits without fully reading stdin. This documents the visible semantic change from v1 to v2. Also, we add an example that demonstrates correctly handling downstream early exit in a stateful producer.
znull
force-pushed
the
znull/v2-early-close-regression-tests
branch
from
8a9f705 to
0ca3ba9
Compare
znull
changed the title
znull
changed the title
znull
changed the title
Explain that go-pipe v1 could hide producer-side pipe write errors as an implementation detail of the command stdin-copy path, while v2 exposes those errors more directly. Document the migration patterns for stateless and stateful producers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
znull
force-pushed
the
znull/v2-early-close-regression-tests
branch
from
0ca3ba9 to
433e104
Compare
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.
This branch builds on
znull/stage-v2-cleanby introducing explicit endpoint stream ownership types and documenting the behavior changes that fall out of the v2 pipeline wiring.It replaces boolean stdin/stdout close flags in the
Stagestartup contract withInputStreamandOutputStreamtypes, so ownership is visible in the value passed to each stage. The stream ownership docs now spell out when stages must close closing streams, includingStart()error paths, and the startup cleanup code has been aligned so the pipeline only closes streams it still owns.WithStdoutCloseris still closed when startup fails before the final stage receives it.It also documents v2 early-close producer behavior. A downstream stage can stop reading before all input is consumed, causing upstream writes to surface
EPIPE,SIGPIPE, orio.ErrClosedPipe; README and public godoc now point callers atIgnoreError(stage, IsPipeError)for stateless producers and call out the extra consistency work required for stateful producers.