Skip to content
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

try catch doesn't work cause of the ErrorHandler.php and HandledErrorException.php #10787

Closed
crashbtz opened this issue Apr 25, 2014 · 6 comments
Labels

Comments

@crashbtz
Copy link

I have a question about the method handleWith() define in HandledErrorException.php and call in ErrorHandler.php line 176.

Here is my test code in my Action:

try {
    $value = 5 / 0;
} catch (\Exception $e) {
    echo $e->getMessage();
//    throw $e;
}
return new Response('test');

When I do that, I want to catch the Exception (write a message or not) and render a response.
So why does the HandledErrorException generate a template of the Exception anyway?

cf. HandledErrorException.php line 45

echo $this->handlerOutput;

We solved "this problem" (I mean for us) temporary by adding our own

set_error_handler()

which override yours after the line

Debug::enable();

in app_dev.php

I don't know if it's a good pratice or not.

Thank you

@nicolas-grekas
Copy link
Member

HandledException are special because they have to work around bugs in PHP and around fatal error situation. The way we do this is to pre-fill a buffer that should be manually canceled when no special situation occurs. If for any reason the buffer is not cleared (especially a non catchable situation like said bug or fatal error), the buffer is outputted and at least you have some debug message to deal with.
So, I would just suggest you to add this code in your catch block (with proper use statement):

if ($e instanceof HandledErrorException) {
    $e->cleanOutput();
}

@stof
Copy link
Member

stof commented Apr 25, 2014

@nicolas-grekas the issue with this is that the Debug component now requires each library catching exceptions for recovery to be aware of the component to avoid breaking things. It is not a good behavior IMO

@nicolas-grekas
Copy link
Member

I agree, I'm working on it

@crashbtz
Copy link
Author

thank both of you for your responses, I'm gonna do like you said @nicolas-grekas until this gonna be fix

@nicolas-grekas
Copy link
Member

Wait a minute, I just realized that this is not new behavior: to work around https://bugs.php.net/54275 the error handler has always echoed some output. What is new with handled exceptions is that it is now possible to cancel this output. No regression at all. Am I wrong?
Still, I'll continue trying other solutions and see whats possible.

@crashbtz
Copy link
Author

You're right, there is no regression, the new version is better thanks to the cleanOutput() method

@crashbtz crashbtz added the Debug label Apr 28, 2014
fabpot added a commit that referenced this issue Apr 28, 2014
…t/54275 (nicolas-grekas)

This PR was merged into the 2.5-dev branch.

Discussion
----------

[Debug] less intrusive work around for https://bugs.php.net/54275

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #10787, #10292
| License       | MIT
| Doc PR        | none

- This PR supersedes the behavior introduced in #10725 :
  Instead of building some complicated code to work around https://bugs.php.net/54275, the code is now as
  straightforward as possible, with a conditional fallback work around.
- The handling of fatal errors is also made more robust/fail-safe.
- Last but not least, ErrorsLoggerListener and FatalErrorExceptionsListener are now registered earlier and
  are now cleaning up their handler/logger once set to prevent setting again and again for sub-requests
  (+remove one refcount for these handler and logger).

Commits
-------

d7a186f [Debug] less intrusive work around for https://bugs.php.net/54275
@fabpot fabpot closed this as completed Apr 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants