Baldinof / roadrunner-bundle

A RoadRunner worker integrated in your Symfony app
MIT License
266 stars 47 forks source link

Problem with StreamedResponse #9

Closed vsychov closed 4 years ago

vsychov commented 4 years ago

Hi,

I found that StreamedResponse not work now, example code for reproduce problem:

<?php

namespace App\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\StreamedResponse;
use Symfony\Component\Routing\Annotation\Route;

class TestController extends AbstractController
{
    /**
     * @Route("/test")
     */
    public function test()
    {
        $response = new StreamedResponse(null, 200, ['Content-Type' => 'text/plain']);
        $response->setCallback(function () {
            var_dump('Hello World');
            flush();

            var_dump('Hello World');
            flush();
        });

        return $response;
    }
}
vsychov commented 4 years ago

This issue may be fixed by #10, but this fix looks not well.

vsychov commented 4 years ago

Another way - disallow usage of StreamedResponse, and create some alternative in this bundle.

Baldinof commented 4 years ago

Hi!

Thank you for the report and sorry for the late response :)

symfony/psr-http-message-bridge should handle streamed response : https://github.com/symfony/psr-http-message-bridge/blob/master/Factory/PsrHttpFactory.php#L133

After investigations, it turns out that Symfony has a StreamedResponseListener that sends the response before the Kernel::handle() is done...

Then when symfony/psr-http-message-bridge tries to convert it into a PSR response, the callback is not called again.

I wrote another PR to fix this, if you want to review it : https://github.com/Baldinof/roadrunner-bundle/pull/11

10 should have fixed this too but I prefer to limit code in KernelHandler.

vsychov commented 4 years ago

Thanks, @Baldinof , you PR looks much better, can you merge it please?

Baldinof commented 4 years ago

Done, and released with 1.2.2 :)