Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Fixed missing {code} macro. (Cloudstack Hackathon 2013)

...

  1. If you are writing entry point code, you are responsible for catching all exceptions, both Checked and Unchecked, and properly logging the error message and exception stack trace. What is an entry point? That's the point where a thread enters into our code base. For example, all API commands are entry points. If you are spinning off threads to do processing, the run() method of that thread is an entry point. If you are scheduling tasks to be run in a thread pool, that particularly task is an entry point. All of those code should be wrapped as follows.
    Code Block
    try {
        code...;
    } catch (Exception specific to your code) {
        Specific exception handling and logging...
    } catch (Exception e) {
        s_logger.warn("Caught unexpected exception", e);
        exception handling code...
    }
    
  2. If you are not writing entry point code, then it's fine to expect that code above yours will catch and log the exception. However, it is your responsibility to make sure that the stack trace of the exception is captured in the log. Don't ever catch and throw a new exception without either logging the exception or including it as the cause of the new exception.
    Code Block
    try {
        code...;
    } catch (XenAPIException e) {
        // Do either this: s_logger.warn("Caught a xen api exception", e);
        // or throw new CloudRuntimeException("Caught a xen api exception", e);
        // Don't ever do JUST this.
        throw new CloudRuntimeException("Got a xen api exception");  
    }
    
  3. Don't ever declare a method to throw Exception. This may seem like a nice and quick easy way to handle exceptions but it forces the caller methods to catch Exception which then hides all other checked Exceptions in other parts of the code. Take for instance:
    Code Block
    public void irresponsibleMethod() throws Exception;
    public void responsibleMethod() throws XenAPIException;
    public void runtimeExceptMethod(); // throws CloudRuntimeException that's not suppose to be logged until entry point.
    public void innocentCaller() {
        try {
            irresponsibleMethod();
            responsibleMethod();
            runtimeExceptionMethod();
        } catch(Exception e) {
            s_logger.warn("Unable to execute", e);
            throw new CloudRuntimeException("Unable to execute", e);
            // What's wrong here?
            // 1. If the error was thrown from responsibleMethod, the caller now forgot to do special handling for XenAPIException.
            // 2. If the error was thrown from runtimeExceptionMethod, the caller now log it once here, and will log again at entry point.
        }
    }
    
  4. Don't ever throw Exception itself. If you need a checked Exception, either find one that fits your needs or create one yourself. If what you run into shouldn't be possible or is due to programmer mistake, then throw CloudRuntimeException. To decide if you need a CloudRuntimeException, ask yourself this, is this similar to hitting a null pointer? NullPointerException is a runtime exception because if the caller wanted to handle the pointer being null situation, they would have handled it before calling. Checked exceptions should be thrown if and only if the caller has a reasonable chance of handling the exception other than log and report error. Prefer CloudRuntimeException unless you have a good reason to throw a checked exception. Note the words "programmer mistake" here. User errors should be handled properly. 
  5. If you have to do some error handling for an exception, don't throw a new exception, rethrow the original one. Rethrowing the original one allows the correct stack trace to be logged.
    Code Block
    try {
        some code;
    } catch(XenAPIException e) {
        // catch generic error here.
        s_logger.debug("There's an exception.  Rolling back code: " + e.getMessage());
        ...rollback some code;
        throw e; // note there's no "new" here.
    }
    
  6. If you have a background thread processing a list of equal items, it is important that the processing of each item includes a try-catch loop. If you don't and if there is any exception in processing one of the items, the items that are not processed yet will stop processing. This can have disastrous consequences as the background thread can keep coming back to the same list of items but every item after the item with the exception will never get processed.
    Code Block
    for (Task task : taskList) {
        try {
            process task;
        } catch (Exception e) {
            ...handle exception and continue
        }
    }
    

...