Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Converting errors to exceptions: design flaw?

I came across some code recently that used a custom error handler to turn any PHP errors into an generalized application exception. A custom exception handler was also defined that would log the exception if it was within a particular error code range. Example:

class AppException extends Exception
{

}

function error_handler($errno, $errstr, $errfile, $errline)
{
    throw new AppException($errstr, $errno);
}

function exception_handler($exception)
{
    $min = ...;
    $max = ...;

    if ($exception->getCode() >= $min && $exception->getCode() <= $max)
    {
        // log exception
    }
}

set_error_handler('error_handler');
set_exception_handler('exception_handler');

$a[1]; // throws exception

The problem is that I saw things like:

try
{
    do_something();
}
catch (AppException $exception)
{
}

Which implies that there is no distinction between actual programming errors and "Exceptional" behavior. Upon further digging, I came across parts of the code that were designed around the idea that PHP errors represented "Exceptional" behavior such as:

...

function my_function($param1, $param2)
{
    // do something great
}

try
{
    my_function('only_one_param');
}
catch (AppException $exception)
{
}

Which ends up obfuscating errors and the design of the application's interface.

What is your opinion on handling errors this way? Is it worth turning PHP's native errors into exceptions? What do you do in situations like the above where a codebase is designed around this idea?

like image 706
rr. Avatar asked Aug 06 '10 16:08

rr.


3 Answers

I convert everything, including notices to exceptions. There's no reason to allow exceptions to continue normally as it does. Undefined variable or offset? That's a problem.

https://github.com/KyleWolfe/PHPErrorNet

like image 100
kwolfe Avatar answered Sep 20 '22 03:09

kwolfe


There has been some debate about this in the PHP community. I believe the general thinking is that errors are things thrown by internal PHP functions, and are coding problems you really need to fix; whereas exceptions are things thrown by your application code that you may just need to 'handle'.

However some of the newer SPL extensions (which tend to be more object oriented) do use exceptions for things previously might have thrown errors, so there is some grey area.

There's also a PHP core class ErrorException - http://www.php.net/manual/en/class.errorexception.php which looks a little easier to use than your code example, if this is a route you want to go down.

like image 34
Tim Fountain Avatar answered Sep 23 '22 03:09

Tim Fountain


Personally, I do this all the time. The only difference is that in my error_handler function, I check to see if the error is an E_NOTICE first, and only throw if it is not (I log the notice anyway)...

I would change the AppException to something that extends ErrorException... Something like: PhpRuntimeErrorException extends ErrorException which you ONLY use for PHP errors... The reason is so that it's more readable (it's easier to tell what a PhpRuntimeErrorException is without needing to figure out where it's thrown). The other reason, is that the ErrorException will store the generating line/file/etc information, where it would not be stored elsewhere (since the backtrace starts from the throw line)...

So, then you can "try" code like this:

try {
    $f = fopen('foo.bar', 'r');
    $ret = '';
    while ($data = fread($f)) {
        $ret .= process($data);
    }
    fclose($f);
    return '';
} catch (PHPRuntimeErrorException $e) {
    throw new RuntimeException('Could not open file');
} catch (ProcessException $e) {
    fclose($f);
    throw new RuntimeException('Could not process data');
}
return $ret;

I also make my default exception handler generate a 500 server error page. That's because any exceptions should be caught, and if they were not, it really is a server error...

Just my experience and opinion...

like image 31
ircmaxell Avatar answered Sep 20 '22 03:09

ircmaxell