Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Dont close the reponse stream in debug#19114
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedJun 20, 2016
👍 |
1 similar comment
fabpot commentedJun 20, 2016
👍 |
fabpot commentedJun 20, 2016
Thank you@nicolas-grekas. |
…as-grekas)This PR was merged into the 2.7 branch.Discussion----------[HttpKernel] Dont close the reponse stream in debug| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19078| License | MIT| Doc PR | -Because it's `terminate`'s job to clean the state, not the `Response`'s,and because the current behavior prevents getting any output on trailing errors on FPM especially.Commits-------2fbc200 [HttpKernel] Dont close the output stream in debug
pyrech commentedJun 24, 2016
Isn't this a BC break for those using only the HTTP Foundation component? |
stof commentedJun 24, 2016
it is indeed a BC break |
nicolas-grekas commentedJun 24, 2016
@pyrech did you experience any issue or are you just wondering? |
pyrech commentedJun 24, 2016
I have a small application that uses the |
stof commentedJun 24, 2016
IMO, we should revert this in 2.7. Tiny BC break are not acceptable in patch releases IMO (except when necessary to fix security issues according to our guidelines, but this is not a security issue) |
stof commentedJun 24, 2016
btw, this change means that the behavior of your kernel.terminate event listener will be different in dev and prod, which might not be good. We should look into logging errors there, but still respecting the behavior of finishing the fastcgi request, to be closer to the prod |
nicolas-grekas commentedJun 24, 2016
Debug mode is already significantly different from prod, and it is far more important to give feedback to the dev in case of any errors. |
jakzal commentedJul 11, 2016
Looks like this actually broke something in PHPBB (see the referenced issue on their side). |
JEDIBC commentedJul 12, 2016
I agree with@stof . This behavior modification of kernel.terminate in dev is not a good thing. Furthermore the description in the changelog is absolutely not indicative of the repercussion on kernel.terminate. |
fabpot commentedJul 13, 2016
reverted |
* 2.7: [VarDumper] Fix dumping jsons casted as arrays PassConfig::getMergePass is not an array Revert "bug#19114 [HttpKernel] Dont close the reponse stream in debug (nicolas-grekas)" Fix the retrieval of the last username when using forwarding [Yaml] Fix PHPDoc of the Yaml class [HttpFoundation] Add OPTIONS and TRACE to the list of safe methods Update getAbsoluteUri() for query string uris
* 2.8: [VarDumper] Fix dumping jsons casted as arrays PassConfig::getMergePass is not an array Revert "bug#19114 [HttpKernel] Dont close the reponse stream in debug (nicolas-grekas)" Fix the retrieval of the last username when using forwarding [Yaml] Fix PHPDoc of the Yaml class [HttpFoundation] Add OPTIONS and TRACE to the list of safe methods Update getAbsoluteUri() for query string uris
* 3.0: [VarDumper] Fix dumping jsons casted as arrays PassConfig::getMergePass is not an array Revert "bug#19114 [HttpKernel] Dont close the reponse stream in debug (nicolas-grekas)" Fix the retrieval of the last username when using forwarding [Yaml] Fix PHPDoc of the Yaml class [HttpFoundation] Add OPTIONS and TRACE to the list of safe methods Update getAbsoluteUri() for query string urisConflicts:src/Symfony/Component/Yaml/Yaml.php
* 3.1: [VarDumper] Fix dumping jsons casted as arrays PassConfig::getMergePass is not an array Revert "bug#19114 [HttpKernel] Dont close the reponse stream in debug (nicolas-grekas)" [Serializer] Include the format in the cache key Fix the retrieval of the last username when using forwarding [Yaml] Fix PHPDoc of the Yaml class [HttpFoundation] Add OPTIONS and TRACE to the list of safe methods Update getAbsoluteUri() for query string urisConflicts:src/Symfony/Component/DependencyInjection/Compiler/PassConfig.phpsrc/Symfony/Component/HttpFoundation/Tests/RequestTest.php
lecajer commentedJul 19, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for reverting. In what versions of symfony/http-foundation and symfony/http-kernel is this fix available ? |
stof commentedJul 19, 2016
@jeremlicious the revert is currently only available in dev versions. It will be available in the next patch releases (i.e. 3.1.3, 3.0.9, 2.8.9 and 2.7.18) |
Uh oh!
There was an error while loading.Please reload this page.
Because it's
terminate's job to clean the state, not theResponse's,and because the current behavior prevents getting any output on trailing errors on FPM especially.