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

Flight is removing my custom error_handler #275

Closed
rkyoku opened this issue Jul 12, 2016 · 12 comments
Closed

Flight is removing my custom error_handler #275

rkyoku opened this issue Jul 12, 2016 · 12 comments

Comments

@rkyoku
Copy link

rkyoku commented Jul 12, 2016

Because of this code below (in addition to using \Flight::set('flight.handle_errors', false);):

public function handleErrors($enabled)
    {
        if ($enabled) {
            set_error_handler(array($this, 'handleError'));
            set_exception_handler(array($this, 'handleException'));
        }
        else {
            restore_error_handler();
            restore_exception_handler();
        }
    }

Flight is removing my custom error_handler that I defined before calling Flight::start() (because it would have no sense to define it after this call)

I could not override the Engine (at least I tried using Flight::register but it did not work).
And I could not override Flight itself so that I could manually override the $engine property (which is set using self::$engine = new new \flight\Engine();), because the propery is private.
Extending the whole \Flight class obviously causes disastrous damage.

Also, using an "after start" callback is of no use because the error arises during start(), not after.

I don't know what else to do, but I cannot have uncaught errors in my app :(

The only option for me is to define my error_handler during the construct of my controller (called by the router), but ideally I'd rather define my error_handler at the very beginning of my application code.

Thanks for any tips, or for replacing the above code with something like:

public function handleErrors($enabled)
    {
        if ($enabled) {
            set_error_handler(array($this, 'handleError'));
            set_exception_handler(array($this, 'handleException'));
        }
        else {
            // do nothing
        }
    }

(I don't know if it will cause other issues though, but at least it will not remove any previously set error_handler)

Sincerely,

@tamtamchik
Copy link
Contributor

tamtamchik commented Jul 12, 2016

Can you show how do you exactly define your error handler?

I personally use https://github.com/filp/whoops to handle errors (so I turn flight error handling off), and my code that actually works looks like this (it's very simple and no extra):

use Whoops\Run;
use Whoops\Handler\PrettyPageHandler;

$whoops = new Run;
$whoops->pushHandler(new PrettyPageHandler);
$whoops->register();

Flight::set('flight.handle_errors', false);

@rkyoku
Copy link
Author

rkyoku commented Jul 12, 2016

Sure, here is my code:

// This code below is in my main App object, but I put it here for sake of simplicity
\Flight::set('app', $this);
\Flight::set('flight.handle_errors', false);

// ... other init like DB and other stuff

function allThrowErrorHandler($errNo, $errStr, $errFile, $errLine)
{
    die('test'); // This code is never called
}
set_error_handler('allThrowErrorHandler');

$this->_initRouter(); // All my routes add associated callbacks
\Flight::start();

If I initialize (again!) my handler in my controller ctor, it works fine and handles errors (because my controller is called near the end of the Flight _start() method, thanks to Flight's router). Otherwise, since Flight calls restore_error_handler();, it obviously unsets my first error handler.

@tamtamchik
Copy link
Contributor

tamtamchik commented Jul 12, 2016

I see. Try remove \Flight::set('flight.handle_errors', false); and change this:

function allThrowErrorHandler($errNo, $errStr, $errFile, $errLine)
{
    die('test'); // This code is never called
}
set_error_handler('allThrowErrorHandler');

to this:

Flight::map('error', function(Exception $e){
    die('test'); // This code is never called
});

This should work.

@rkyoku
Copy link
Author

rkyoku commented Jul 12, 2016

You mean even with \Flight::set('flight.handle_errors', false); ?

@tamtamchik
Copy link
Contributor

tamtamchik commented Jul 12, 2016

@RenaudParis no, just updated my answer :)
You'll have to delete \Flight::set('flight.handle_errors', false);

@tamtamchik
Copy link
Contributor

@RenaudParis were you able to make it work?

@rkyoku
Copy link
Author

rkyoku commented Jul 13, 2016

@tamtamchik Thank you for the code snippet.
I tried it, but it did not seem to work. It seems the standard error handling (which is XDebug btw) is used.

Here is my code:

// in "app->initFlightErrorHandler()":
\Flight::map('error', array($this, 'onFlightError'));
$toto = $tata; // cause a notice, that should be caught and converted into an exception (at least that's what I would like to achieve with my own error handler)

// "app->onFlightError':
public function onFlightError(\Exception $e)
    {
        var_dump($e);
        die('error ' . $e->getMessage());
    }

@mikecao
Copy link
Collaborator

mikecao commented Jul 15, 2016

It sounds like I need to check if the currently set error handlers are Flight's own handlers before running restore handler functions. I'll do some testing.

@CodeBrauer
Copy link

Any updates? I'm running into the same issue.

$whoops = new \Whoops\Run;
$whoops->pushHandler(new \Whoops\Handler\PrettyPageHandler);
$whoops->register();

Flight::set('flight.handle_errors', false);

Error handler is now default php handler and not whoops. (PHP 7.1)

@CodeBrauer
Copy link

CodeBrauer commented Oct 16, 2017

Maybe I found the problem...

So restore_error_handler and restore_exception_handler gets called here. But Flight didn't register it's own error/exception handler before.

From php.net/restore_error_handler:

Used after changing the error handler function using set_error_handler(), to revert to the previous error handler (which could be the built-in or a user defined function).

So it actually reverts my own registered error handler - and I am back to PHPs default error handler... I think the two function calls aren't necessary. After commenting out line 133&134 my error handler is working fine.

@mikecao
Copy link
Collaborator

mikecao commented Oct 16, 2017

@CodeBrauer I think you're right. I'll provide a fix.

@mikecao
Copy link
Collaborator

mikecao commented Oct 17, 2017

Fixed in 5b4916d.

@mikecao mikecao closed this as completed Oct 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants