5
\$\begingroup\$

I have created this code for user error logging, and I am wondering if there is anything that can be improved. The point is that this error handler would ONLY catch user errors created in-code by trigger_error(), and would display, log, and/or email the error, depending on the config settings. The error logging class is loaded by an autoloader function. For production, all error levels would be set to 0, and the user error handler would never be set, and the class would never be loaded.

Config file:

 // user error display level (change for production)
 define('LEV_USER_ERROR_DISPLAY_LEVEL', E_USER_ERROR);

 // user error logging level (change for production)
 define('LEV_USER_ERROR_LOG_LEVEL', E_USER_ERROR | E_USER_WARNING | E_USER_NOTICE);

 // user error email alert level
 define('LEV_USER_ERROR_EMAIL_LEVEL', 0);

 // user error email address list (e.g. 'someone1@somewhere.com, someone2@somewhere.com')
 define('LEV_USER_ERROR_EMAIL_ADDRESSES', '');

Init file:

   // set user error handler
   if (LEV_USER_ERROR_LOG_LEVEL | LEV_USER_ERROR_DISPLAY_LEVEL | LEV_USER_ERROR_EMAIL_LEVEL) {
    set_error_handler('lev_user_error_handler::user_error_handler', LEV_USER_ERROR_LOG_LEVEL | LEV_USER_ERROR_DISPLAY_LEVEL | LEV_USER_ERROR_EMAIL_LEVEL);
   }

Error logging class file:

<?php

 // user error handler
 class lev_user_error_handler {

  // user error handler
  public static function user_error_handler($error_level, $message, $file_name, $line_number) {
   if ((LEV_USER_ERROR_LOG_LEVEL | LEV_USER_ERROR_DISPLAY_LEVEL) == 0) return true;
   switch ($error_level) {
    case E_USER_ERROR:
     if (LEV_USER_ERROR_DISPLAY_LEVEL & E_USER_ERROR) {
      echo '[' . date('Y-m-d h:i:s') . '] User Level Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.'<br />';
     }
     if (LEV_USER_ERROR_LOG_LEVEL & E_USER_ERROR) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . "\"\n", 3, 'application/logs/user_error_log.txt');
     }
     if (LEV_USER_ERROR_EMAIL_LEVEL & E_USER_ERROR) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . '"', 1, LEV_USER_ERROR_EMAIL_ADDRESSES, 'From: no-reply@' . preg_replace('/^.+?\./i', '', $_SERVER['SERVER_NAME']));
     }
     die;
     break;
    case E_USER_WARNING:
     if (LEV_USER_ERROR_DISPLAY_LEVEL & E_USER_WARNING) {
      echo '[' . date('Y-m-d h:i:s') . '] User Level Warning: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.'<br />';
     }
     if (LEV_USER_ERROR_LOG_LEVEL & E_USER_WARNING) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Warning: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . "\"\n", 3, 'application/logs/user_error_log.txt');
     }
     if (LEV_USER_ERROR_EMAIL_LEVEL & E_USER_WARNING) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . '"', 1, LEV_USER_ERROR_EMAIL_ADDRESSES, 'From: no-reply@' . preg_replace('/^.+?\./i', '', $_SERVER['SERVER_NAME']));
     }
     break;
    case E_USER_NOTICE:
     if (LEV_USER_ERROR_DISPLAY_LEVEL & E_USER_NOTICE) {
      echo '[' . date('Y-m-d h:i:s') . '] User Level Notice: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.'<br />';
     }
     if (LEV_USER_ERROR_LOG_LEVEL & E_USER_NOTICE) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Notice: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . "\"\n", 3, 'application/logs/user_error_log.txt');
     }
     if (LEV_USER_ERROR_EMAIL_LEVEL & E_USER_NOTICE) {
      error_log('[' . date('Y-m-d h:i:s') . '] User Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.', Request: "' . $_SERVER['ORIG_PATH_INFO'] . '"', 1, LEV_USER_ERROR_EMAIL_ADDRESSES, 'From: no-reply@' . preg_replace('/^.+?\./i', '', $_SERVER['SERVER_NAME']));
     }
     break;
    default:
     // call PHP internal error handler
     return false;
   }
   // do not call PHP internal error handler
   return true;
  }
 }
?>
\$\endgroup\$
3
\$\begingroup\$

Personally I think that working with 50% native and 50% your code does not work very well, because of things such as trigger_error does not allow custom bits to be sent.

That being said if you named your class Error and created it to be abstract that implements a logger interface (optional) you would be able to do more with it.

Creating custom constants such as LOG,SHOW_ERROR,SEND_MAIL combined with a custom static function would be a better option, as doing things such as:

Error::Trigger("Cannot divide by 0", Error::LOG | Error::SHOW_ERROR);

makes more sense to have specific control over errors.

Here's a small example how I would improve the above

abstract class Error
{
    public const LOG = 0;
    public const SEND_MAIL = 1;
    public const SHOW_ERROR = 2;
    /*...*/

    public static function Monitor(){}; /*Used for set_error_handler*/

    public static function Trigger($Message,$bits = Error::LOG | Error::SEND_MAIL,$Context = false)
    {
        if($bits & Error:LOG)
        {
             //Log it
        }

        if($bits & Error:SEND_MAIL)
        {
             //Send it
        }

        /*Lastly*/
        if($bits & Error:LOG)
        {
             //Show it
        }
    }
}

you would then bind the Monitor to the error_reportng and call the Error::Trigger depending on what type of error has been triggered, or you could extend the class and run the parent static method

class ErrorHandler extends Error
{
    public fucntion Monitor(/*...*/)
    {
        parent::Trigger(/*...*/);
    }
}

And allow your class to handle all errors as you then have more control over the decision on what to do.

you can then set the default error handling to a combination of options such as:

 define("DEFAULT_ERROR_HANDLING",Error:log | Error::SEND_MAIL);

and change $bits in the parameter section of trigger to $bits = DEFAULT_ERROR_HANDLING

\$\endgroup\$
  • \$\begingroup\$ I definetly like the idea of a custom trigger method to allow for more control over each trigger. My concern is that if error handling is turned off in the config, I would like to avoid loading the class at all, but without using native functionality, I am not sure how I would accomplish this. Any advice? \$\endgroup\$ – dqhendricks Jan 27 '11 at 20:51
  • \$\begingroup\$ this can be accomplished by creating a function in the root scope, so that instead of Error::Trigger you would have TriggerError function and within that function check if errors are turned on, as well as check if the class is loaded, then you can pass the params to the actual object \$\endgroup\$ – RobertPitt Jan 27 '11 at 21:01
1
\$\begingroup\$

Preface:

I'm not going to talk about alternatives to trigger_error for now.

Edit: Someone else did, perfect ! :)

About the config:

I assume those settings are what you are using in development ?

If so: I'd like to advice against that. You should always produce at least E_NOTICE free code and only logging those errors to a file isn't going to help you achieve that.

Why ?

The E_NOTICE error you are going to see mostly is $asd is an undefined variable. That will point you to errors in your code very quickly and you don't have to think about ("why doesn't it work, i put the value into the function") or similar problems coming from typos. Maybe your IDE warns you about those but let php do it too.

Init:

Usually an error handler isn't a static function. I'm not sure what php version will throw an E_STRICT warning.

Also "class::function" is not the best way to pass a callback. (See here for php callbacks)

You might want to use set_error_handler("class", "function");. That help if you decide you don't want to use a static function but an object because it works the same way set_error_handler($obj, "function");

The handler:

You are repeating

'[' . date('Y-m-d h:i:s') . '] User Error: "' . $message . '", File: "'.$file_name.'", Line: '.$line_number.'

many times there. Put that into an extra method. It will help you if you want to change the logfile name or the date or something like that.

Also it helps with the distinction you are already makeing betten the 2 log files.

Displaying errors ?

You should never display errors in production. I'd use ini_get("display_errors") to figure out how the server is configure and honor that at all costs.

If you don't want the errors displayed in development (maybe many ajax calls ?)

Look into firebug. It lets you see the errors if you got the firefox plugin installed without cluttering the pages output.

\$\endgroup\$
  • \$\begingroup\$ You are definitely right about the needless repeating, I will fix that asap. One note, E_NOTICE errors would not be handled by the custom error handler, only user level errors (E_USER_...) would be handled by this class. All other errors would be handled by the native error handler and use its settings. \$\endgroup\$ – dqhendricks Jan 27 '11 at 20:56
  • \$\begingroup\$ Also, you are right, I should definetly look in to honoring the ini setting. \$\endgroup\$ – dqhendricks Jan 27 '11 at 20:59

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.