Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update HttpKernel.php#36885

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

Closed
thiagocordeiro wants to merge1 commit intosymfony:4.4fromthiagocordeiro:patch-1
Closed

Update HttpKernel.php#36885

thiagocordeiro wants to merge1 commit intosymfony:4.4fromthiagocordeiro:patch-1

Conversation

@thiagocordeiro
Copy link
Contributor

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
LicenseMIT

HTTP Kernel catches\Exception, but PHP 7.0 introduced\Throwable and a few errors are not handled by this try/catch.

Symfony registers a global handler\Symfony\Component\ErrorHandler\ErrorHandler and at some point\Symfony\Component\HttpKernel\Event\ExceptionEvent is triggered, so errors are handled twice

try {
return$this->handleRaw($request,$type);
}catch (\Exception$e) {
}catch (\Throwable$e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe:

catch(\Throwable | \Exception) {

?

Copy link
ContributorAuthor

@thiagocordeirothiagocordeiroMay 21, 2020
edited
Loading

Choose a reason for hiding this comment

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

Not actually@Guikingone, back in time PHP allowed us to catch\Exception only, from PHP7 on, errors are thrown and we can catch by\Error. Both implement\Throwable, which means any\Exception will be catch by\Throwable.

In other words,\Throwable is the same of\Exception | \Error.

@nicolas-grekasnicolas-grekas added this to the4.4 milestoneMay 22, 2020
@nicolas-grekas
Copy link
Member

This is done on purpose:\Error are later caught by the PHP exception handler, and reinjected into the kernel usingterminateWithException().

@thiagocordeiro
Copy link
ContributorAuthor

thiagocordeiro commentedMay 22, 2020
edited
Loading

This is not needed when you have an exception event listener right? any exception will be re-thrown insrc/Symfony/Component/HttpKernel/HttpKernel.php:214 then handled by the PHP exception handler if there is no response from the event listener

@derrabus
Copy link
Member

duplicate of#26514 😉

@fabpot
Copy link
Member

Closing for the same reasons as#26514

@fabpotfabpot closed thisJun 3, 2020
@thiagocordeirothiagocordeiro deleted the patch-1 branchJune 3, 2020 19:42
fabpot added a commit that referenced this pull requestJun 25, 2022
…to show `HttpKernel::handle()` will catch throwables (Nyholm)This PR was merged into the 6.2 branch.Discussion----------[FrameworkBundle][HttpKernel] Add deprecation warning to show `HttpKernel::handle()` will catch throwables| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Tickets       |Fix#16205| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->I suggest that starting from Symfony 7.0 the `HttpKernel` will start to catch `\Throwable` and convert them to a `Response`.This was first asked in#16205, I face a similar issue with Runtime component and Bref..----------The reason I push for this change is to embrace the request/response workflow of the Kernel without trusting the custom error handler. In an environment where you serve multiple requests with the same PHP process (read: RoadRunner, Swoole, Bref) you would write something like:```php$kernel = new Kernel('prod', false);while (true) {  $request = /* create sf request from custom environment */  try {    $response = $kernel->handle(request);    return ResponseConverter::convert($response);  } catch (\Throwable $e) {    exit(1);  }}```(pseudo code of course. Here is a [real example](https://github.com/php-runtime/bref/blob/0.3.1/src/BrefRunner.php#L30-L43))The `exit(1)` means a hard crash. For Bref Runtime it would result in a 500 error from API-gateway. Since the `\Throwable` is caught, the Symfony error handler is not used. If we would not to catch the `\Throwable`, then the Symfony error handler would be used, but it would print a Response instead of returning it. (Printing a response will just add HTML on the CLI...)-----------Other PRs and issues related to this:-#45301-#26514-#22128-#36885-#25467I'm happy to let the `HttpKernel` to catch the `\Throwable` exception right now, but I thought this very conservative PR would have a higher change to get merged.Also note that we do not specify any behaviour on our [HttpKernelInterface](https://github.com/symfony/symfony/blob/v6.0.7/src/Symfony/Component/HttpKernel/HttpKernelInterface.php#L43)-------To remove the deprecation message you need to add this to your config:```yamlframework:    catch_all_throwables: true```Commits-------7977a15 Add deprecation warning to show HttpKernel::handle() will catch throwables
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@GuikingoneGuikingoneGuikingone left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@thiagocordeiro@nicolas-grekas@derrabus@fabpot@Guikingone@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp