-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] "Deprecation" of StreamedResponseListener causes RequestStack to be empty within StreamedResponse callback #46743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
semi related #46445 |
@DaDeather interesting does this also mean that return new StreamedResponse(function () use ($twig) {
$twig->display('template.html.twig');
}); ? |
Yes, exactly. |
Maybe something like the following in the HttpKernel would be required to fix this issue and not lose compatibility to roadrunner or swoole streamed response: if ($response instance of StreamedResponse) {
$callback = $response->getCallback();
$response->setCallback(function() use ($callback) {
try {
$callback();
} finally {
$this->requestStack->pop();
}
});
} else {
$this->requestStack->pop();
} /cc @nicolas-grekas what do you think? The #46445 would require another fix in the browserkit to convert StreamedResponse into browserkit response correctly. |
I was also affected by this in our application. We have a lot of legacy code that we simply wrapped in StreamedResponse-callbacks (similar to what's described in https://symfony.com/doc/current/migration.html). This broke in 6.1 because the RequestStack is empty inside the callback, which means the callback can't access the Session or make Sub-Requests. I decided to simply re-add a clone of the StreamedResponseListener for the time being: <?php
declare(strict_types=1);
namespace App\EventSubscriber;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\StreamedResponse;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
class MyStreamedResponseListener implements EventSubscriberInterface
{
public function onKernelResponse(ResponseEvent $event)
{
if (!$event->isMainRequest()) {
return;
}
$response = $event->getResponse();
if ($response instanceof StreamedResponse) {
$response->send();
}
}
public static function getSubscribedEvents(): array
{
return [
KernelEvents::RESPONSE => ['onKernelResponse', -1024],
];
}
} This seems to work fine for us. I just rolled out the fix today. |
Hey, thanks for your report! |
@carsonbot yes it is still relevant and also present in Symfony 6.2.7. My workaround above works, but I still consider this a bug in the framework and a BC break in Symfony 6.1. |
We are also running into this. We use streamed responses for processes that take a while, where we show a progress bar in the UI. However, we then can't add any flashes anymore. I don't really like the old workaround, since clearly there is a reason Symfony doesn't do that anymore. I hope we can find a decent solution. |
still relevant in 6.2.9 |
@alexander-schranz up to give a PR a try? |
I have opened up a PR for this issue based on @alexander-schranz code. |
I think we completely missed this consequence. It'd be fine reverting #45476, and adding also a test case that covers this situation so that we won't forget about the use case again. |
…medResponse (Ismail Turan) This PR was merged into the 6.3 branch. Discussion ---------- [HttpKernel] Fix missing Request in RequestStack for StreamedResponse | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46743 | License | MIT | Doc PR | - Similar to #51139 by `@DaDeather`, see discussion there. Commits ------- ca1c40e [HttpKernel] Fix missing Request in RequestStack for StreamedResponse
Symfony version(s) affected
6.1.*
Description
The change made within #45476 causes the
RequestStack
to be empty when theStreamedResponse
Callback is being called.In detail:
When using an
StreamedResponse
, the Callback passed toStreamedResponse
is executed AFTER the\Symfony\Component\HttpKernel\Kernel::handle
method (which is doing apop
on theRequestStack
in\Symfony\Component\HttpKernel\HttpKernel::finishRequest
).Instead it is executed within the
\Symfony\Component\HttpFoundation\Response::send
method causing the\Symfony\Component\HttpFoundation\RequestStack
to be empty and not having anyRequest
object inside it.We're upgrading our legacy application which is already wrapped with symfony.
This is why we're sadly have to use a construct where we're required to inject the
\Symfony\Component\HttpFoundation\RequestStack
to get theRequest
which is not available anymore at that state within aStreamedResponse
.Is this really the expected / wanted behaviour for this change? That when using a
StreamedResponse
theRequest
object becomes unavailable or is required to be reinitialized manually? If this is desired it should be announced as a possible BC break.How to reproduce
./src/Service/TestService.php
./src/Controller/TestController.php
When calling the
/test
url an exception is thrown:Possible Solution
No response
Additional Context
Behaviour was changed in pull request: #45476
The text was updated successfully, but these errors were encountered: