views:

42

answers:

2

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?

+1  A: 

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...

ircmaxell
Yea that all sounds good, changing the AppException to a PHP error specific exception is what came to mind first. Though I wonder what the difference is between this solution and just generating the 500 server error in the custom error handler without using exceptions for actual errors. Also, is there any overhead for doing it this way?
rr
Well, it's not about the overhead. I only generate 500 error pages for unhandled exceptions. There are quite often times when I catch PHP errors and continue processing. For example, DomDocument will raise a warning if it can't parse an XML string. So I capture that error, and use it to trigger a cleanup and re-parse of the file `try { $dom->loadXml($xml); } catch (PhpRuntimeErrorException $e) { $xml = XmlCleaner::clean($xml); $dom->loadXml($xml); }` I don't want to run the `XmlCleaner`, since it's really quite expensive. So I let the PHP error tell me when to do it...
ircmaxell
+1  A: 

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 extensions 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.

Tim Fountain
That was my thought as well...I think the confusion comes from the fact that there are multiple error handling mechanisms and no real standards. I did take a look at the ErrorException class and that looks like a good first step.
rr
Well, for some, yes. Like if you call `fopen($foo)`, you should get an error since you're missing the second param. But if you call `parse_url($url)`, and `$url` is a really malformed url, you'll get a warning there as well. So in some cases, yes they are coding problems. But there are a large number of "runtime" problems that have nothing to do with coding... So it's really just a matter of how you want to handle your errors (I like my exceptions. If you don't, that's quite fine too. As long as you're consistent and methodical, that's all that really matters in the end)...
ircmaxell
Sure, I think that is a pretty good case, I was thinking strictly programming errors, but that makes more sense.
rr