Question

Wondering how much effort I should go to forcing useful debugging information when creating exception messages, or should I just trust the user to supply the right info, or defer the information gathering to an exception handler?

I see a lot of people people doing their exceptions like:

throw new RuntimeException('MyObject is not an array')

or extending the default exceptions with custom exceptions that don't do much but change the name of the exception:

throw new WrongTypeException('MyObject is not an array')

But this doesn't supply much debugging info... and doesn't enforce any kind of formatting with the error message. So you could end up with exactly the same error producing two different error messages... eg "Database connection failed" vs "Could not connect to db"

Sure, if it bubbles to the top, it'll print the stack trace, which is useful, but it doesn't always tell me everything I need to know and usually I end up having to start shooting off var_dump() statements to discover what went wrong and where... though this could be somewhat offset with a decent exception handler.

I'm starting to think about something like the code below, where I require the thrower of the exception to supply necessary args to produce the correct error message. I'm thinking this might be the way to go in that:

  • Minimum level of useful information must be supplied
  • Produces somewhat consistent error messages
  • Templates for exception messages all in the one location (exception classes), so easier to update the messages...

But I see the downside being that they are harder to use (requires you look up exception definition), and thus might discourage other programmers from using supplied exceptions...

I'd like some comment on this idea, & best practices for a consistent, flexible exception message framework.

/**
* @package MyExceptions
* MyWrongTypeException occurs when an object or 
* datastructure is of the incorrect datatype.
* Program defensively!
* @param $objectName string name of object, eg "\$myObject"
* @param $object object object of the wrong type
* @param $expect string expected type of object eg 'integer'
* @param $message any additional human readable info.
* @param $code error code.
* @return Informative exception error message.
* @author secoif
*/
class MyWrongTypeException extends RuntimeException {
    public function __construct($objectName, $object, $expected, $message = '', $code = 0) {
        $receivedType = gettype($object) 
        $message = "Wrong Type: $objectName. Expected $expected, received $receivedType";
        debug_dump($message, $object);
        return parent::__construct($message, $code);
    }
}

....

/**
 * If we are in debug mode, append the var_dump of $object to $message
 */
function debug_dump(&$message, &$object) {
     if (App::get_mode() == 'debug') {
         ob_start();
         var_dump($object);
         $message = $message . "Debug Info: " . ob_get_clean();
    }
}

Then used like:

// Hypothetical, supposed to return an array of user objects
$users = get_users(); // but instead returns the string 'bad'
// Ideally the $users model object would provide a validate() but for the sake
// of the example
if (is_array($users)) {
  throw new MyWrongTypeException('$users', $users, 'array')
  // returns 
  //"Wrong Type: $users. Expected array, received string
}

and we might do something like a nl2br in a custom exception handler to make things nice for html output.

Been reading: http://msdn.microsoft.com/en-us/library/cc511859.aspx#

And there is no mention of anything like this, so maybe it's a bad idea...

Was it helpful?

Solution

I strongly recommend the advice on Krzysztof's blog and would note that in your case you seem to be trying to deal with what he calls Usage Errors.

In this case what is required is not a new type to indicate it but a better error message about what caused it. As such a helper function to either:

  1. generate the textual string to place into the exception
  2. generate the whole exception and message

Is what is required.

Approach 1 is clearer, but may lead to a little more verbose usage, 2 is the opposite, trading a terser syntax for less clarity.

Note that the functions must be extremely safe (they should never, ever cause an unrelated exception themselves) and not force the provision of data that is optional in certain reasonable uses.

By using either of these approaches you make it easier to internationalise the error message later if required.

A stack trace at a minimum gives you the function, and possibly the line number, thus you should focus on supplying information that is not easy to work out from that.

OTHER TIPS

I won't detract from the advise regarding Krzysztof's blog, but here is a dead-easy way to create custom exceptions.

Example:

<?php
   require_once "CustomException.php";
   class SqlProxyException extends CustomException {}

   throw new SqlProxyException($errorMsg, mysql_errno());     
?>

The code behind that (which I borrowed somewhere, apologies to whomever that was)

<?php

interface IException
{
    /* Protected methods inherited from Exception class */
    public function getMessage();                 // Exception message
    public function getCode();                    // User-defined Exception code
    public function getFile();                    // Source filename
    public function getLine();                    // Source line
    public function getTrace();                   // An array of the backtrace()
    public function getTraceAsString();           // Formated string of trace

    /* Overrideable methods inherited from Exception class */
    public function __toString();                 // formated string for display
    public function __construct($message = null, $code = 0);
}

abstract class CustomException extends Exception implements IException
{
    protected $message = 'Unknown exception';     // Exception message
    private   $string;                            // Unknown
    protected $code    = 0;                       // User-defined exception code
    protected $file;                              // Source filename of exception
    protected $line;                              // Source line of exception
    private   $trace;                             // Unknown

    public function __construct($message = null, $code = 0)
    {
        if (!$message) {
            throw new $this('Unknown '. get_class($this));
        }
        parent::__construct($message, $code);
    }

    public function __toString()
    {
        return get_class($this) . " '{$this->message}' in {$this->file}({$this->line})\n"
                                . "{$this->getTraceAsString()}";
    }
}

See How to Design Exception Hierarchies on the blog of Krzysztof Cwalina, a coauthor of "Framework Design Guidelines".

Never, ever trust a user to 'do the right thing', and include information for debugging. If you want information, you need to gather it yourself and store it somewhere where its accessible.

Also as stated, if it's hard(er) to do something, the users will avoid doing it, so again, don't depend on their goodwill and their knowledge of what they need to send.

This thinking implies a method by which you collect the information and log it, which implies using var_dump() somewhere.

Also, as said by Mark Harrison, a button which makes it easy to send an error message somewhere is fantastic for you and for the users. It makes it easy for them to report an error. You (as the recipient) get a lot of duplicates, but duplicate information is better than no information.

However much detail you add, be sure and either

  • make it easy to cut and paste the whole thing, or
  • have a button that will report the error for them
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top