Skip to content

Fix custom $stderrEvent for container attach and logs streams - #108

Merged
clue merged 1 commit into
clue:1.xfrom
clue-labs:stderr-event
Oct 18, 2026
Merged

clue merged 1 commit into
clue:1.xfrom
clue-labs:stderr-event

Conversation

@clue

@clue clue commented Oct 18, 2026

Copy link
Copy Markdown
Owner

This changeset fixes the custom $stderrEvent for containerAttachStream() and containerLogsStream(). Both methods first inspect the container to check its TTY setting and previously returned a stream that only forwarded data, end, error and close events, so the custom STDERR event never reached the caller. The stream now forwards this event as well. This has been broken since both methods were added in #53 (2019) and #61 (2020), and the functional tests now cover both against a real Docker daemon.

Builds on top of #107, #61 and #53
Resolves / closes #96

@clue clue added the bug Something isn't working label Oct 18, 2026
@clue clue added this to the v1.7.0 milestone Oct 18, 2026
@clue
clue requested a balanced review from Copilot October 18, 2026 22:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Custom event names matching lifecycle events can cause argument errors or duplicate error notifications.

1 open issue
What changed in this PR

Fixes custom STDERR event forwarding for container attach and log streams, addressing #96.

Changes:

  • Forwards custom events through the internal stream wrapper.
  • Passes the event name from both client methods.
  • Adds unit and Docker-backed functional regression tests.
File Description
tests/​Io/​ReadableUnwrapStreamTest.php Tests custom forwarding and avoids duplicate data events.
tests/​FunctionalClientTest.php Checks STDERR delivery against Docker.
tests/​ClientTest.php Tests custom events from both client methods.
src/​Io/​ReadableUnwrapStream.php Adds optional custom event forwarding.
src/​Client.php Passes STDERR event names to the wrapper.

Review effort: Balanced

Comment thread src/Io/ReadableUnwrapStream.php Outdated
@clue
clue merged commit 8762efd into clue:1.x Oct 18, 2026
15 checks passed
@clue
clue deleted the stderr-event branch October 18, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

containerAttachStream's stderrEvent is emitted on wrong stream

2 participants