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.94k stars 1.95k forks source link

error handling of warnings and notices #2562

Closed mbohal closed 5 years ago

mbohal commented 5 years ago

Hi I'm running pretty standard Slim application. I'm trying to get it to treat Warning and Notice as Errors on my dev machine, but I do not know hot to do it.

I have added error_reporting(E_ALL); to my index.php.

It seems to me that the root of the problem is here where Warnings and Notices are not caught as Throwable. I can get them to show by putting ini_set('display_errors', 1); in my index.php, but that is not very elegant as I want it to be used only during development. I think this is something that should be handled by the framework.

What is the recommended way of handling this? Are there any plans to improve this?

I think using set_error_handler and set_exception_handler would be more robust. Would that be acceptable?

Thank you.

chriseskow commented 5 years ago

You can use set_error_handler() and throw an exception, such as PHP's built-in ErrorException which is meant precisely for this purpose:

error_reporting(E_ALL); // set to whatever types of errors you want to be fatal

set_error_handler(function ($severity, $message, $file, $line) {
    if (error_reporting() & $severity) {
        throw new \ErrorException($message, 0, $severity, $file, $line);
    }
});

The exception would then be caught by Slim's error handler and can be handled as you wish.

mbohal commented 5 years ago

I see.That would probably work. I will use it now.

IMHO it is boilerplate code though. If these lines must be part of each project that wants to follow best practices, then they might as well be part of the framework.

Or is there some reason why not having set_error_handler in the framework is the better option?

akrabat commented 5 years ago

With a micro framework there is always a balance between what goes into it as it's not full-stack.

In this case, some people prefer their notices to go to their error error logs and not their Exception handling, so it's a choice each app makes.

mbohal commented 5 years ago

@ akrabat Would you accept a PR to the documentation describing the solution suggested by @chriseskow?

I've spent quite a bit of time with this and I had to look through the source code to figure out whats up. I understand your decision, but it might make users life easier if it is documented.

What about the skeleton app?

akrabat commented 5 years ago

@mbohal Would definitely accept a documentation update! I'm mildly surprised it's not there already and certainly should be on the Error Handling page.

I'm on the fence about the skeleton. Maybe?

mbohal commented 5 years ago

@akrabat I would put it in the skeleton app, because I'm used to treating notices and warnings as errors during development and testing. I think it helps to write cleaner code.

But then again if it was up to me I would put it in the framework itself. So should I make a PR against the skeleton app?

akrabat commented 5 years ago

So should I make a PR against the skeleton app?

I don't know. I can't promise you it'll be accepted, but it might be.

I also can't imagine developing without a tail -f on my PHP error log in a terminal window, so clearly we're all different!

mandienk commented 2 years ago

You can use set_error_handler() and throw an exception, such as PHP's built-in ErrorException which is meant precisely for this purpose:

error_reporting(E_ALL); // set to whatever types of errors you want to be fatal

set_error_handler(function ($severity, $message, $file, $line) {
    if (error_reporting() & $severity) {
        throw new \ErrorException($message, 0, $severity, $file, $line);
    }
});

The exception would then be caught by Slim's error handler and can be handled as you wish.

Great, thx :-)