3
\$\begingroup\$

I'm VERY new to error/exception handling, previously just echoing everything to screen, and am trying to really clean up my code. My goal was to log everything but only show the user a friendly "oops" page if something was seriously broken. How does what I have below look?

Any best practice advice is appreciated.

<?php
error_reporting(~0);
ini_set('display_errors', 0);
ini_set('log_errors', 1);


/* Set the error handler. */
set_error_handler(function ($errno, $errstr, $errfile, $errline) {
    /* Ignore @-suppressed errors */
    if (!($errno & error_reporting())) return;

    $e = array('type'=>$errno,
               'message'=>$errstr,
               'file'=>$errfile,
               'line'=>$errline);
    redirect($e);
});


/* Set the exception handler. */
set_exception_handler(function ($e) {
    $e = array('type'=>$e->getCode(),
               'message'=>$e->getMessage(),
               'file'=>$e->getFile(),
               'line'=>$e->getLine());
    redirect($e);
});


/* Check if there were any errors on shutdown. */
register_shutdown_function(function () {
    if (!is_null($e = error_get_last())) {
        redirect($e);
    }
});


function redirect($e) {
    $now = date('d-M-Y H:i:s');
    $type = format_error_type($e['type']);
    $message = "[$now] $type: {$e['message']} in {$e['file']} on line {$e['line']}\n";
    $error_log_name = ini_get('error_log');
    error_log($message, 3, $error_log_name);

    switch ($e['type']) {
        /* We'll ignore these errors.  They're only here for reference. */
        case E_WARNING:
        case E_NOTICE:
        case E_CORE_WARNING:
        case E_COMPILE_WARNING:
        case E_USER_WARNING:
        case E_USER_NOTICE:
        case E_STRICT:
        case E_RECOVERABLE_ERROR:
        case E_DEPRECATED:
        case E_USER_DEPRECATED:
        case E_ALL:
            break;
        /* Redirect to "oops" page on the following errors. */
        case 0: /* Exceptions return zero for type */
        case E_ERROR:
        case E_PARSE:
        case E_CORE_ERROR:
        case E_COMPILE_ERROR:
        case E_USER_ERROR:
            return header('Location: /oops_page.htm');
    }
}


function format_error_type($type) {
    switch($type) {
        case 0:
            return 'Uncaught exception';
        case E_ERROR: /* 1 */
            return 'E_ERROR';
        case E_WARNING: /* 2 */
            return 'E_WARNING';
        case E_PARSE: /* 4 */
            return 'E_PARSE';
        case E_NOTICE: /* 8 */
            return 'E_NOTICE';
        case E_CORE_ERROR: /* 16 */
            return 'E_CORE_ERROR';
        case E_CORE_WARNING: /* 32 */
            return 'E_CORE_WARNING';
        case E_CORE_ERROR: /* 64 */
            return 'E_COMPILE_ERROR';
        case E_CORE_WARNING: /* 128 */
            return 'E_COMPILE_WARNING';
        case E_USER_ERROR: /* 256 */
            return 'E_USER_ERROR';
        case E_USER_WARNING: /* 512 */
            return 'E_USER_WARNING';
        case E_USER_NOTICE: /* 1024 */
            return 'E_USER_NOTICE';
        case E_STRICT: /* 2048 */
            return 'E_STRICT';
        case E_RECOVERABLE_ERROR: /* 4096 */
            return 'E_RECOVERABLE_ERROR';
        case E_DEPRECATED: /* 8192 */
            return 'E_DEPRECATED';
        case E_USER_DEPRECATED: /* 16384 */
            return 'E_USER_DEPRECATED';
    }
    return $type;
}
?>
\$\endgroup\$
1
\$\begingroup\$

A few things jump out at me:

  • The redirect of $e['type'] == 0: For exceptions the default code is 0. Another code could be used and if the exception is not catched, it will throw a fatal error. Your handler does watch for fatal errors, so this may be a non-issue. The exception condition will just be logged twice.

  • For the actual header redirect: Some browsers are slow on the header redirect (I'm not sure why). If you allow the rest of your code to continue executing, it could cause an ugly screen flicker. Again, since you are only redirecting fatal errors, probably a non-issue.

  • The logging functionality: You may want to consider moving this out into another function. Some day you may decide to log the errors to a database, send emails, or even a text message. You could add these to that redirect method, but that would start to get pretty messy. If each piece were a separate method that was called from a generic log method, that would be easier to read and change as needed.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.