slimphp / Slim

Slim is a PHP micro framework that helps you quickly write simple yet powerful web applications and APIs.
http://slimframework.com
MIT License
11.98k stars 1.95k forks source link

Trigger E_WARNING halt execution #1454

Closed petitchevalroux closed 9 years ago

petitchevalroux commented 9 years ago

According to http://php.net/manual/en/errorfunc.constants.php E_WARNING should not halt script execution

To reproduce : $app->get('/hello/:name', function ($name) { trigger_error('E_USER_WARNING', E_USER_WARNING); echo "Hello, $name"; });

=> I get a 500 error

Removing E_WARNING from error_reporting disable this behavior but for me reporting setting's should not interfere with script execution.

JoeBengalen commented 9 years ago

Dont think this is Slim related. Try if you have the same behaviour outside of Slim. Also take a look at error_reporting

petitchevalroux commented 9 years ago

@JoeBengalen Thank you for your quick answer. Outside of Slim, updating error_reporting does not modify script behaviour E_ERROR => halt script E_WARNING => does not halt the script and that's it.

I think this may be due to Slim::handleErrors

JoeBengalen commented 9 years ago

Ohw you using slim v2. I've never used that version myself so I cannot really help you with that. But looking at the code Slim should respect the error reporting level: https://github.com/slimphp/Slim/blob/2.x/Slim/Slim.php#L1408

petitchevalroux commented 9 years ago

Totally agree with you, but error_reporting should just interfere with error displaying and logging, and imho not changing script behavior.

akrabat commented 9 years ago

Replace Slim's handleErrors with your own one.

petitchevalroux commented 9 years ago

@akrabat can you explain me how to do that ? btw the problem occured with E_USER_NOTICE too if it's is in error_reporting.

akrabat commented 9 years ago

http://docs.slimframework.com/errors/500/

$app->error(function (\Exception $e) use ($app) {
    // do something here
});
petitchevalroux commented 9 years ago

Overloading errorHandler does not change anything. handleErrors trigger ErrorException stopping the call process

JoeBengalen commented 9 years ago

handlerErrors() checks the error reporting level before throwing the exception. Try setting the error reporting level to 0 and try again. The exception should not be thrown.

error_reporting(0);

Note. You should not leave error reporting at 0, but just to test it for now.

akrabat commented 9 years ago

Ah yeah. Sorry.

What you need to do is write your own middleware that resets the error handler:

<?php
require '../vendor/autoload.php';

class ErrorHandlerMiddleware extends \Slim\Middleware
{
    public function call()
    {
        set_error_handler(null);
        $this->next->call();
    }
}

$app = new \Slim\Slim();
$app->add(new ErrorHandlerMiddleware);

$app->get('/', function () use ($app) {
    trigger_error('E_USER_WARNING', E_USER_WARNING);
    echo "<p>Here</p>";
});

$app->run();
akrabat commented 9 years ago

Or, extend the Slim class and override handleErrors :)

petitchevalroux commented 9 years ago

@JoeBengalen this what I said before, removing E_WARNING from error_reporting does the job ;) @akrabat I was reading the PrettyExceptions middleware in order to write my own error middleware ;)

Despite i fixed this for my usage, don't you think it's a major issue ?

akrabat commented 9 years ago

Slim 2 is no longer under active development and is only accepting security fixes.

Slim 3 doesn't have this "feature" :)

petitchevalroux commented 9 years ago

ok and is slim 3 ready for "end user" ?

akrabat commented 9 years ago

We're at beta 2. Expecting RC1 towards the end of next week.

petitchevalroux commented 9 years ago

Nice, i will give it a try.

BTW, thank you for your help ;)

akrabat commented 9 years ago

You're welcome :)