Skip to content

[ErrorCatcher][ExceptionHandler] Handle \Throwable #31694

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

Closed

Conversation

fancyweb
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Follow up of #28954

I also removed an unused test file + added a test for the new behavior + added 3 tests when you use custom handlers.

@@ -85,7 +85,7 @@ public function testHeaders()

ob_start();
$handler->sendPhpResponse(new MethodNotAllowedHttpException(['POST']));
$response = ob_get_clean();
ob_get_clean();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused var

});

$handler->handle($exception);
$handler->handle(new \Exception());
ob_end_flush(); // Necessary because of this PHP bug : https://bugs.php.net/bug.php?id=76563
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lost more than 4 hours on this. If someone wants to look at it, just put this in a controller :

$handler = ExceptionHandler::register();
$handler->setHandler(function ($e) {
    echo 'foo';
});

$handler->handle(new \RuntimeException('bar'));

dump(ob_get_clean()); die();

It will dump the full content (default error handler content + "foo"). If you comment the dump, only "foo" is actually rendered.

@nicolas-grekas nicolas-grekas added this to the 5.0 milestone May 30, 2019
@fancyweb fancyweb force-pushed the final-error-and-exception-hanlers branch from bf577d0 to 2653752 Compare June 28, 2019 13:41
@fancyweb fancyweb changed the title [Debug] Make ExceptionHandler and ErrorHandler final [Debug][ErrorCatcher] Make ExceptionHandler and ErrorHandler final Jun 28, 2019
@fancyweb
Copy link
Contributor Author

@nicolas-grekas @yceruto Just rebased this on master to apply the changes on the new ErrorCatcher component.

Since it touches 2 components, should I split this ?

@fancyweb fancyweb force-pushed the final-error-and-exception-hanlers branch from 2653752 to 64d02cb Compare June 28, 2019 14:08
@fancyweb fancyweb changed the title [Debug][ErrorCatcher] Make ExceptionHandler and ErrorHandler final [ErrorCatcher][ExceptionHandler] Handle \Throwable Jun 28, 2019
Tobion added a commit that referenced this pull request Jun 28, 2019
… the ErrorCatcher component (fancyweb)

This PR was merged into the 5.0-dev branch.

Discussion
----------

[Debug] Remove all deprecated classes that were moved to the ErrorCatcher component

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | -

#31694 (comment)

Commits
-------

8ce3a89 [Debug] Remove all deprecated classes that were moved to the ErrorCatcher component
@nicolas-grekas
Copy link
Member

The plan is to remove ExceptionHandler from ErrorHandler if you want to have a look.

@fancyweb fancyweb deleted the final-error-and-exception-hanlers branch July 18, 2019 20:52
nicolas-grekas added a commit that referenced this pull request Jul 23, 2019
…(fancyweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[Debug][ExceptionHandler] Add tests for custom handlers

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

In #31694 I mixed many things but the whole PR was closed. I wrote some tests for custom handlers + the handle tests don't use mock anymore

I think they are useful even if the `ExceptionHandler` will disappear in the new component because it will still exists in 4.4 for the next 3 years.

Commits
-------

c53e253 [Debug][ExceptionHandler] Add tests for custom handlers
symfony-splitter pushed a commit to symfony/debug that referenced this pull request Jul 23, 2019
…(fancyweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[Debug][ExceptionHandler] Add tests for custom handlers

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

In symfony/symfony#31694 I mixed many things but the whole PR was closed. I wrote some tests for custom handlers + the handle tests don't use mock anymore

I think they are useful even if the `ExceptionHandler` will disappear in the new component because it will still exists in 4.4 for the next 3 years.

Commits
-------

c53e25332a [Debug][ExceptionHandler] Add tests for custom handlers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants