EndOfStreamAwareHttpWorkerInterface

[Kenny1911]

Q	A
Bugfix?	✔️
Breaks BC?	❌
New feature?	❌

Alternative solution of #30.

Instead of rewriting the HttpWorkerInterface, I created EndOfStreamAwareHttpWorkerInterface, which overrides the respond method and adds an optional bool $endOfStream argument.

The HttpWorker class now implements EndOfStreamAwareHttpWorkerInterface.

This change doesn't break BC.

In the future, the $endOfStream argument can be added to the main HttpWorkerInterface, and EndOfStreamAwareHttpWorkerInterface can be removed.

Summary by CodeRabbit

• New Features
  • Implemented end-of-stream aware HTTP response handling to enable more reliable streaming content delivery with explicit response completion signaling and better termination management.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new interface EndOfStreamAwareHttpWorkerInterface is introduced to standardize end-of-stream handling for HTTP workers. The interface extends HttpWorkerInterface and defines a respond method that accepts status, body (string or generator), headers, and an end-of-stream flag. The HttpWorker class is updated to implement this new interface.

Changes

Cohort / File(s)	Summary
New End-of-Stream Interface src/EndOfStreamAwareHttpWorkerInterface.php	Introduces new interface extending HttpWorkerInterface with respond method supporting string and generator bodies, custom headers, and explicit end-of-stream control.
HttpWorker Interface Implementation src/HttpWorker.php	Updates class declaration to implement EndOfStreamAwareHttpWorkerInterface alongside existing HttpWorkerInterface.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A stream-aware interface springs to life,
Where responses flow without strife,
Generators yield their precious data,
As HttpWorker streams get greater!
End-of-stream now takes the stage, 🌊

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'EndOfStreamAwareHttpWorkerInterface' is a single word (the interface name) that directly refers to the main addition in the changeset, but lacks clarity about the actual change being introduced.	Consider using a more descriptive title that explains what was added, such as 'Add EndOfStreamAwareHttpWorkerInterface for end-of-stream handling' to clarify the purpose and nature of the change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

src/EndOfStreamAwareHttpWorkerInterface.php (1)

12-24: Missing @param tag for $endOfStream.

The docblock documents $status, $body, and $headers but omits the $endOfStream parameter, which is the primary reason this interface exists.

📝 Proposed fix

      *        that header.
+     * `@param` bool $endOfStream Whether this is the last response in the stream.
      */

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/EndOfStreamAwareHttpWorkerInterface.php` around lines 12 - 24, The
docblock for the respond method in EndOfStreamAwareHttpWorkerInterface is
missing a `@param` entry for the $endOfStream parameter; update the PHPDoc for the
respond method (respond(int $status, string|\Generator $body = '', array
$headers = [], bool $endOfStream = true): void) to add a `@param` bool
$endOfStream description explaining that it controls whether the last chunk ends
the stream (true by default) so the interface documentation matches the method
signature and purpose.

src/HttpWorker.php (1)

41-41: Redundant interface declaration, but acceptable given the migration plan.

EndOfStreamAwareHttpWorkerInterface already extends HttpWorkerInterface, so explicitly listing both is redundant. However, since the PR notes that EndOfStreamAwareHttpWorkerInterface may be removed in the future (folding $endOfStream into HttpWorkerInterface), keeping both explicit makes the future removal a smaller diff.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/HttpWorker.php` at line 41, The class declaration lists both
EndOfStreamAwareHttpWorkerInterface and HttpWorkerInterface, which is
technically redundant because EndOfStreamAwareHttpWorkerInterface already
extends HttpWorkerInterface; leave both interfaces on the HttpWorker class
declaration as-is to minimize future diffs during the planned migration, and add
a short inline comment near the class declaration (referencing class HttpWorker)
explaining that both are kept intentionally for a smoother future removal of
EndOfStreamAwareHttpWorkerInterface.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/EndOfStreamAwareHttpWorkerInterface.php`:
- Around line 12-24: The docblock for the respond method in
EndOfStreamAwareHttpWorkerInterface is missing a `@param` entry for the
$endOfStream parameter; update the PHPDoc for the respond method (respond(int
$status, string|\Generator $body = '', array $headers = [], bool $endOfStream =
true): void) to add a `@param` bool $endOfStream description explaining that it
controls whether the last chunk ends the stream (true by default) so the
interface documentation matches the method signature and purpose.

In `@src/HttpWorker.php`:
- Line 41: The class declaration lists both EndOfStreamAwareHttpWorkerInterface
and HttpWorkerInterface, which is technically redundant because
EndOfStreamAwareHttpWorkerInterface already extends HttpWorkerInterface; leave
both interfaces on the HttpWorker class declaration as-is to minimize future
diffs during the planned migration, and add a short inline comment near the
class declaration (referencing class HttpWorker) explaining that both are kept
intentionally for a smoother future removal of
EndOfStreamAwareHttpWorkerInterface.

[roxblnfk]

I would suggest making version 4 right away.

• Add a parameter to the interface
• Remove deprecated functions
• Raise the minimum version to PHP 8.2 (it would be good to use Rector)
  What do you think about this?

[Kenny1911]

I think, that releasing a major version just for this change is premature. Here are my thoughts:

The current change is too minor to warrant a major version bump. It doesn't break backward compatibility. The new interface simply extends the existing one, and everyone already using HttpWorkerInterface will continue working without any changes.

Additionally, this package has dependents (including third-party ones) that are unlikely to update quickly to v4.

In my opinion, we need a smoother transition. Here's how I see the update process:

Stage 1 (current, version 3.x):

• Add EndOfStreamAwareHttpWorkerInterface that extends HttpWorkerInterface.
• HttpWorker additionally implements this interface.
• The new interface is used only by those who actually need the endOfStream parameter. For everyone else, nothing changes.

Stage 2 (major version 4):

• Add the missing parameter directly to the main HttpWorkerInterface.
• The need for EndOfStreamAwareHttpWorkerInterface disappears, so we mark it as @deprecated.
• This release can also include other breaking changes (upgrade to PHP 8.2, removal of deprecated methods, etc.) so we release a proper major version, not a "major for a single argument".

Stage 3 (major version 5):

• Completely remove EndOfStreamAwareHttpWorkerInterface.

This approach gives the community time to migrate smoothly:

1. First, everyone can update to 3.x and get the new functionality without breakage.
2. Then, when v4 is ready (with a set of changes), users can plan their update.
3. And only in v5 do we finally remove deprecated interfaces.

[Kenny1911]

Example of smoother upgrading code, that used HttpWorkerInterface and needs endOfStream parameter:

Before

We can't use endOfStream parameter.

function action(HttpWorkerInterface $worker): void
{
    $worker->respond(200, 'OK');
}

Step 1

If we needs set parameter endOfStream, we can check, that worker implements EndOfStreamAwareHttpWorkerInterface.

function action(HttpWorkerInterface $worker): void
{
    if ($worker instanceof EndOfStreamAwareHttpWorkerInterface) {
        $worker->respond(200, 'OK', endOfStream: true);
    } else {
        $worker->respond(200, 'OK');
    }
}

Step 2

Upgrade to v4

EndOfStreamAwareHttpWorkerInterface is deprecated, HttpWorkerInterface supports endOfStream parameter.

We can refactor code and remove checks of worker implementation.

function action(HttpWorkerInterface $worker): void
{
    $worker->respond(200, 'OK', endOfStream: true);
}

Step 3

Upgrade to v5

EndOfStreamAwareHttpWorkerInterface removed.

We MUST refactor our code, if we haven't done it yet

[roxblnfk]

In my opinion, releasing version 4 will be better than adding an obviously outdated interface. Updating to v4 won't be difficult unless someone implements the HttpWorkerInterface directly (which is unlikely): just add v4 to composer.json.

[Kenny1911]

At this moment 52 public packages depends of spiral/roadrunner-http:^3. And all this packages must be upgrade to v4.

Also, the big part of these packages are depends of php >=8.1.

I'm more in favor of a smooth upgrade.

But if you think it's the right thing to do, then I've created a PR #33 to prepare v4 release.