Repository navigation
Update to stream responses without dependency on react/promise-stream - #107
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Pending resume requests can leave inputs stalled, and buffering cancellation retains accumulated output.
2 open issues
What changed in this PR
Replaces react/promise-stream with internal helpers for Docker response streaming and buffering.
Changes:
- Adds an internal promise-backed readable stream.
- Implements buffering and cancellation without the removed dependency.
- Ports stream tests and updates functional tests.
| File | Description |
|---|---|
| tests/Io/StreamingParserTest.php | Tests buffering, errors, and cancellation. |
| tests/Io/ReadableUnwrapStreamTest.php | Tests the new stream wrapper. |
| tests/FunctionalClientTest.php | Uses internal buffering in integration tests. |
| src/Io/StreamingParser.php | Replaces external streaming helpers. |
| src/Io/ReadableUnwrapStream.php | Adds the internal readable-stream wrapper. |
| src/Client.php | Uses the wrapper for logs and attachment. |
| composer.json | Removes react/promise-stream. |
Review effort: Balanced
clue
force-pushed
the
remove-promise-stream
branch
from
October 18, 2026 21:40
fc81479 to
6fb02d2
Compare
This was referenced Oct 18, 2026
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 changeset updates the client to stream responses without the dependency on
react/promise-stream. It now uses a small internalReadableUnwrapStreamin place ofunwrapReadable()and buffers streams with a few lines of its own, so consumers of this package no longer install the extra package. The dependency was added with #27 back in 2016, and reactphp/http#482 removed it from the HTTP layer the same way.All APIs and streaming behavior stay the same, so this should be safe to apply. Stream events now only pass the arguments documented for
ReadableStreamInterface, without the extra stream argument the old package appended. The tests are ported fromreact/promise-streamv1.7.0 and the test suite confirms this keeps 100% code coverage. This should also make it easier to fix #96 in a follow-up PR.Builds on top of #27 and reactphp/http#482
Refs #106 and #96