Skip to content

WIP: [Feature] Stream responses without putting whole to memory#180

Closed
dmitryuk wants to merge 2 commits into
Baldinof:3.xfrom
dmitryuk:stream-generator
Closed

WIP: [Feature] Stream responses without putting whole to memory#180
dmitryuk wants to merge 2 commits into
Baldinof:3.xfrom
dmitryuk:stream-generator

Conversation

@dmitryuk
Copy link
Copy Markdown
Contributor

@dmitryuk dmitryuk commented Nov 22, 2025

Closes #179 #130
Impossible to adapt Symfony's StreamedResponse class, so I created the custom one

Comment thread src/RoadRunnerBridge/HttpFoundationWorker.php
Copy link
Copy Markdown
Contributor

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Great. Thank you

@dmitryuk
Copy link
Copy Markdown
Contributor Author

dmitryuk commented Dec 2, 2025

@Baldinof can you look and merge?

Comment thread src/Http/StreamedGeneratorResponse.php Outdated

use Symfony\Component\HttpFoundation\Response;

class StreamedGeneratorResponse extends Response
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

2 things:

  • ideally it would extend Symfony StreamedResponse so in case the app runs without RR, it would still work and stream the content.
  • maybe the constructor could accept Generator<string>|callable() => Generator<string>, so it could be used like this
return new StreamedGeneratorResponse(function () {
    yield "hello";
    yield "world";
});

Instead of

return new StreamedGeneratorResponse((function () {
    yield "hello";
    yield "world";
})());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. I really tried to convert ob_start/on_flush to \Generator, but it looks impossible

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I mean do something like this:

class StreamedGeneratorResponse extends StreamedResponse
    public function __construct(private readonly \Generator $generator, int $status = 200, array $headers = [])
    {
        parent::__construct(function () use ($generator) {
            foreach ($generator as $chunk) {
                echo $chunk;
            }
        }, $status, $headers);
    }

    public function getGenerator(): \Generator
    {
        return $this->generator;
    }

So when not using RR, the response will still be streamed the normal way

Copy link
Copy Markdown
Contributor Author

@dmitryuk dmitryuk Dec 3, 2025

Choose a reason for hiding this comment

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

Thanks for review, I finally reworked the PR as you suggest, please take a look
UPD: I need a time to look in failing tests

@dmitryuk dmitryuk changed the title [Feature] Stream responses without putting whole to memory WIP: [Feature] Stream responses without putting whole to memory Dec 3, 2025
Comment thread src/Http/StreamedGeneratorResponse.php
Comment on lines +47 to +50
$callback = $response->getCallback();
if (!$callback || (!($content = $callback()) instanceof \Generator)) {
throw new \RuntimeException('StreamedGeneratorResponse callback must return a Generator');
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
$callback = $response->getCallback();
if (!$callback || (!($content = $callback()) instanceof \Generator)) {
throw new \RuntimeException('StreamedGeneratorResponse callback must return a Generator');
}
$content = $response->getGenerator();

@dmitryuk
Copy link
Copy Markdown
Contributor Author

dmitryuk commented Dec 5, 2025

PR frozen. I looking for a way toimplement a feature out-of-box here roadrunner-php/issues#43

@dmitryuk dmitryuk closed this Jan 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect usage of streamed response

3 participants