Pages

Tuesday, January 31, 2012

15 Best Practices for Exception Handling






  1. Don't manage business logic with exceptions. Use conditional statements instead. If a control can be done with if-else statement clearly, don't use exceptions because it reduces readability and performance  (e.g. null control, divide by zero control). .
  2. Exception names must be clear and meaningful, stating the causes of exception.
  3. Throw exceptions for error conditions while implementing a method. E.g. if you return -1, -2, -3 etc. values instead of FileNotFoundException, that method can not be understand.
  4. Catch specific exceptions instead of the top Exception class. This will bring additional performance, readability and more specific exception handling.
  5. Null control with conditionals is not an alternative to catching NullPointerException. If a method may return null, control it with if-else statement. If a return may throw NullPointerException, catch it.
  6. Try not to re-throw the exception because of the price. Bu if re-throwing had been a must, re-throw the same exception instead of creating a new exception. This will bring additional performance. You may add additional info in each layer to that exception.
  7. Define your own exception hierarchy by extending current Exception class (e.g. UserException, SystemException and their sub types) and use them. By doing this you can specialize your exceptions and define a reusable module/layer of exceptions.
  8. Use types of exceptions clearly. Fatal: System crash states. Error: Lack of requirement. Warn: Not an error but error probability. Info: Info for user. Debug: Info for developer. 
  9. Don't absorb exceptions with no logging and operation. Ignoring exceptions will save that moment but will create a chaos for maintainability later.
  10. Don't log the same exception more than once. This will provide clearness of the exception location.
  11. Always clean up resources (opened files etc.) and perform this in "finally" blocks.
  12. Exception handling inside a loop is not recommended for most cases. Surround the loop with exception block instead.
  13. Granularity is very important. One try block must exist for one basic operation. So don't put hundreds of lines in a try-catch statement.
  14. Produce enough documentation for your exception definitions (at least JavaDoc).
  15. Giving a number/code for each different exception message is a good practice for documentation and faster communication.

32 comments:

  1. Thanks for this post! I totally agree with most of the points, but creating a JavaDoc for exceptions is in my opinion an overkill. Maybe you meant to give proper comments in the code?

    ReplyDelete
  2. Thanks for the comment. I meant to give proper comments and/or JavaDoc for our specific exception class definitions, not on each try-catch.

    ReplyDelete
  3. I don't agree with point 6 at all. If you have so many exceptions
    that wrapping them will hurt performance, then you have much bigger problems.

    ReplyDelete
  4. Point 6 is not about wrapping exceptions. It recommends throwing same exception instead of creating new one for re-throw states. Some info may be added into message part.

    ReplyDelete
    Replies
    1. If I need to add more information to an exception, I prefer fields (getters/setters), so that testing is easier - exception.getMessage().contains('foo') vs. assertEquals('foo', exception.getAadditionalInfo()). Also helps with localization.

      Delete
    2. This is true. "Adding to message part" is a general statement.

      Delete
  5. In my experience, developers often make the mistake of trying to catch and handle (and throw!) far too many exceptions.

    You should only catch exceptions you can and must logically handle in a particular unit of code. Exceptions that don't directly pertain to that unit are allowed to propagate.

    (#13 in particular reminds me of nightmare code I've seen where nearly every single statement is wrapped with its own try/catch/finally block.)

    Similarly, you should only throw exceptions in *exceptional* conditions, hence the name. Use them sparingly and only when necessary.

    ReplyDelete
  6. Generally I agree with you.
    I think there is a misunderstanding about #13. I don't suggest try-catch for each statement. I suggest using blocks for granular operations, not for long lines of code. But try-cath block must be used as least as you can of course.

    ReplyDelete
  7. 1. Handle exceptions at the boundary between layers of your code. For example, if you have DAOs for database access, feel free to throw exceptions within them, but handle the exception only at the service class that begins the transaction and calls the DAOs:
    //Payment Service
    Connection conn;
    try {
    conn = connectionFactory.getConnection();// assume autocommit turned off.
    accountDAO.pay(fromAccount, amount);
    accountDAO.credit(toAccount, amount);
    conn.commit();
    } catch(InsufficientBalanceException e) {
    conn.rollback();
    throw new TransactionRejectedException(user, fromAccount, toAccount, amount, e); //Handled by UI.
    }

    2. Do not put passwords or other secure data into exceptions, whether as part of the message, or as part of data in the exception's constructor. You don't want a log file to have:

    31-Jan-2012 17:03 Incorrect login: User aneuman entered 'whatmewory'.

    ReplyDelete
  8. I'm sorry, but the second part of rule 6 is just horrible advice. By re-throwing the same exception, you're losing the stack trace for the method where you're catching the exception and everything above it.

    In other words, never do this:

    catch (Exception ex)
    {
    // Do something...
    throw ex;
    }

    Always do this:

    catch (Exception ex)
    {
    // Do something...
    throw;
    }

    ReplyDelete
    Replies
    1. In c# there is no difference between those two blocks. Is it different somehow in Java? Or did you mean to say...

      catch (Exception x) {
      throw new Exception(x);
      }

      THAT is accomplishing nothing, but hurting performance, and as you say, you lose the stack trace unless the constructor for the new exception saves it, and then you're copying data, another unnecessary operation.

      Delete
    2. I agree with this and practice catch/throw; often (specifically to keep the call stack intact). But I wonder about the "price" of it. Does bubbling the same exception up the chain without ever rethrowing it really incur a sigificant cost?
      More importantly, ultimately at the end of all the try/catch/throw; blocks, not matter how nested they are, somebody eventually has to handle it (or an application crash will occur). When somebody DOES handle it, it most likely is going to result in some form of IO. Either writing a log to disk, or even halting any execution on the UI with a visible error message to alert the user.
      Won't the price in time bubbling up the exception pale in comparison to the price of handling the exception with IO/UI? Is it even worth worrying about the minuscule amount of time taken to bubble up an exception when you know you will have to wait for the much longer IO operation, or worse yet a user error prompt?

      Delete
  9. You are completely right, I meant using "throw;" instead of "throw new Exception()" here. There are no codes in this post, so re-throw statement caused a misunderstanding I think.

    ReplyDelete
  10. The use of the phrase "re-throw" is incorrect regarding the first example above. This is actually throwing a new and different exception. Therefore the use of re-throw should have been within quotes.

    The second example is an example of a re-throw.

    ReplyDelete
  11. Note that the above comments about "throw" versus "throw ex" only applies to .NET. In Java, "throw ex" rethrows the exception without modification.

    http://www.tkachenko.com/blog/archives/000352.html

    ReplyDelete
  12. This list is generally good advice, but it's a cookie-cutter list that you could find pretty much everywhere. Like other commenters, I disagree with some of the points and feel other important points have been left out. With #6, once you're into an exception handling situation performance generally takes a back seat to logging for research and mitigation (bug fixes, etc). If you can provide more information by creating a new exception with discrete properties and wrapping the original as an internal, then do it.

    The best practice where I work is that we only catch exceptions if we can: (a) do something about it (correct/recover/log); (b) need to add information; or, (c) need to swallow it (rare, but it happens).

    This brings me to the next problem with the list, #4. This gem gets trotted out far too often. If I follow the general rules I mentioned previously, most of my catch statements will be at the top of the call stack where all I want to do is log the exception and inform the user something went wrong. There's nothing wrong with catch( Exception ex ) in this situation.

    ReplyDelete
    Replies
    1. I agree for the most part...but please refer to Bill wagner's excellent article "Working Effectively with Exceptions" for an explanation of what's wrong with catch(Exception) in the .NET environment:
      http://visualstudiomagazine.com/Articles/2009/08/01/Working-Effectively-with-Exceptions.aspx?Page=3

      Delete
    2. I agree. I discourage my .NET students from trying to catch specific exceptions since it is often hard to predict what exceptions may arise. I only advocate having specific exception handlers when there needs to be specific actions taken for a specific exception type.

      I encourage them to just use catch (Exception ex) and then they can log the type of the exception, the exception message and indicate what operation failed so that the user has context.

      In most cases logging/displaying what happened (operation x failed) and why it happened (exception type and message) is the best and most practical strategy.

      Delete
    3. +1 for Craig's advice about when one should catch Exceptions

      The logging/displaying of the generic exception could make sense, but only at the outermost place (the application unhandled exception event handlerj), but one has to be careful where is it logged/displayed - you don't want to reveal too much information to the user when your production website crashes...

      Delete
  13. Thanks Alan for the important recall. These practices are written as language independent.

    ReplyDelete
  14. I disagree with using exceptions for anything "less or equal to" warning (point 8). Warnings, infos and debugs are cumulative (you can land up with multiple warnings per operation); Exceptions are not.

    Furthermore, an exception precludes a return value - and by definition (well, at least by mine) a warning should should return a value and indicate something may be crazy with it; even more-so with info and debug. You should be logging warnings, infos and debugs directly to your logging system and not relying on exceptions - and even in some cases rely on the return type to convey them (e.g. CompileResults from CodeDom).

    This mean that exceptions can be "Fatal Error" (which by definition should be left to bubble to your 'Main' and logged from there - in other words a fatal error is akin to an OS kernel panic) or simply "Error" (which would inform the user of the problem - or dealt with in some other fashion if possible).

    When in doubt figure out where the word Exception comes from: something exceptional (out of the ordinary) has happened. Info and debug are by a very large margin not exceptional - warning may be exceptional depending on the scenario at-hand.

    ReplyDelete
    Replies
    1. This is another viewpoint, which may be true for most scenarios. But anyway I can't generalize that "exceptions can not include warning, debug & info level logging".

      Delete
  15. I'm surprised that #7 has garnered no negativity. The major problem (in Java code) are beginner programmers that create extensive Exception class hierarchies for their code that make maintenance and integration a nightmare. ex, FileException, FileOpenException, FileOpenNonExclusiveWriteException instead of using the built in IOException with a meaningful message since it matters little whether how the exception occurred for most other than an IOException occurred. Think about how the error conditions will be handled in the program, not how a programmer will debug it.

    Another is the eternal (in Java anyways) RuntimeException vs Exception hierarchy, and having to declare thrown exceptions. I personally feel that the designers of Java made a mistake here and should only have instituted RuntimeExceptions. Throw clauses on the methods would be indications of potential exceptional cases, which is all they are now really, except those that do not descend from RuntimeException must be declared in every method up the stack or caught, whether desired or not.

    ReplyDelete
    Replies
    1. Having come to python from a .NET/Java background, I was rather frustrated with the lack of specific exceptions and the language's reliance on the error message.

      First, I prefer to put concrete details into exception messages (e.g.: instead of "Invalid Number" I prefer to say "XII could not be parsed to a number"), which then requires regexp pattern matching or contains, which is a great tool, but in case of exceptions I'd rather be sure. Also, I've seen a number of cases of unit test suites that relied on exception messages failing when run on non-US locales.

      While in certain situations, you might only care about IOExceptions, in a CMS document editing scenario you might provide different error messages to your users depending on *why* you couldn't open the file - non-existent files and not being able to obtain exclusively for writing are two different things.

      But I do agree that exception hierarchies just for the sake of it is a bad idea - only do these a) if you are writing a framework b) when you actually need it.

      Delete
  16. Great little piece! I'm currently doing a personal project that will be done by 2016. Gonna make me Millions lol! I'll have to do Exception handling totally properly for once! Bookmarked this! It has everything I want in Exception Handling that I've a history of forgetting!

    ReplyDelete
  17. My experience is somewhat different.

    It is good practice to manually check input for each method, and not to proceed with invalid data. Valid data should never generate exceptions - exceptions are a lot more expensive than normal returns. As a consequence, exceptions should almost exclusively be used to signal irrecoverable problems in application code.

    The situation is slightly different for library code. A library designer has no idea of how and if the library's clients will want to proceed when a specific abnormal situation occurs. It's also difficult for him to predict all usage patterns, and it is almost never feasible to restrict the library use to allow only valid usage. This leaves a lot of space for incorrect calls, so the library code must signal conditions which are abnormal to the library but may be recoverable for users.

    For instance send a select statement to a database in order to test for the database's existence is an example of incorrect usage, but the situation, although abnormal for the database, is perfectly recoverable for the caller, and the best way for the database driver to signal the abnormality is an exception. If, instead of reading from a database, you try to read from a hash table, not checking for a key's presence or for null on return is plain stupid, and a very bad way to use exceptions. If you are in library code yourself, and you were asked by client code to look up the value a key which doesn't exist, it is again an exception which is the most adequate way of signaling the abnormality to the caller.

    This means that exceptions received from libraries are either recoverable, in which case you simply handle the exception and continue, or they are irrecoverable, in which case there's not much use in interpreting them.

    So what do I usually do? I have a try/catch around the entire method body for all methods, and smaller try/catch blocks around specific statements or statement groups from which I expect exceptions which are recoverable. The outer try/catch just throws the exception up the call stack - if any exception reaches its catch, it is definitely one from which I can't recover locally, so it makes no sense to parse it. This also allows me to easily transform Exception into RuntimeException in Java, which helps with the checked exceptions problem mentioned above. At the top of every call stack there's a central dispatch point in the application where all exceptions from which there was no recovery land. That place simply logs them, and exits or restarts, depending on what's adequate for the given type of application.

    What are the benefits? Exceptions are handled in a very uniform way, but with minimal overhead, both in terms of runtime effort and programming effort.

    ReplyDelete
  18. I once worked on a project where the manager had coded an exception-catcher which completely obscured where the exception had occurred.
    Don't do that ;-)

    (This is the same guy who coded error message "Parameter error", took me 2 hours to find when he was away one day..)

    I would add to (2) "and unique" (!)
    Further than that, I'd give each a unique code, recorded on a separate database, pointing to further useful information.

    If you catch and exception with a state-word (of bit-indicators), it may be useful to interpret states in a way that can be passed back to the caller. Dropping the state-word & obscuring it is not good practice.

    If you have exceptions which are common but harmless, it's as well to divert the (after reporting!) to a "harmless" bucket.
    A example of this is collecting unfulfilled links in a mapping or linkage. The advantage of this is that if a code change produces a new linking exception, it's easily spotted.

    ReplyDelete
  19. Another guideline that I recommend is to avoid writing algorithms based on exception handling. For example in this thread on stackoverflow forum, the algorithm in the answer proposed by ukhardy uses the IOException in the while loop.

    ReplyDelete
  20. Best practice is read Framework Design Guidelines.

    I don't agree with first practice. In general it will much easier and understandable code with exceptions in comparison with return codes (witch is C-style). Return codes can be used, for example on critical way of high load app.

    ReplyDelete
    Replies
    1. Great point, reading up always is useful. However, not all frameworks exhibit "best practices". For instance, some Microsoft tools and libraries (e.g.: MSTest, Entity Framework) tend to be quite behind community accepted best practices and thus their guidance can be sub-optimal.

      Delete
  21. I just stumbled over this 1-year-old post, still a google top result when searching for exception best practices.

    For me, there are two main guidelines for exceptions:

    A) I throw an exception if my method did not fulfill its contract. By returning normally without exception, I tell my method's caller that the job has been completely done.

    B) Catching an exception means that I warrant the program can reasonably continue from this point, whatever situation caused the exception. Typically, the next statements depend on the previous ones, so how can you continue if they failed somehow. Typically, I only catch exceptions around top-level actions.

    From these guidelines, I disagree with some of your best practices (or the reasons you give for them).

    I support #4, but for different reasons. If you catch just a given subclass of Exception, it's easier to analyze its possible causes in the try-catch surrounded block and to warrant that continuing the program makes sense. But I doubt that there is a measurable performance difference between more or less specific exception classes in catch clauses.

    #5 depends on what the "contract" defines. If e.g., not finding an element in a list is considered a normal situation and thus documented in the Javadocs, then returning null is perfectly valid and throwing an exception is not. If, on the other hand, the contract assumes that a matching object will be present, then you must throw an exception, and that can be a NullPointerException (not the best choice, but still legitimate).

    #6 recommends against rethrowing exceptions, and this because of performance reasons. Never let (often poorly understood) performance concerns guide your software style. When exceptions cause significant performance issues, you probably missed my guideline B). Rethrowing or wrapping and rethrowing exceptions can be

    #8 defines Warn, Info and Debug types of exceptions. That's logging and not exceptions, because in these cases the methods probably fulfilled their contract asking for a normal return without exception.

    #13 strongly contradicts my guideline B). I can't believe that every 5 or 10 lines of code there's a point where the program can continue useful work, even if the previous steps failed. The very concept of exceptions aims to make code more readable by having less error handling overhead.

    Best regards
    -- Ralf

    ReplyDelete
  22. #12, in a lot of cases the loop is used by batch jobs. You will like to have try/catch inside the loop in order to maximize the processing. Lots of the system will skip the failed one and continue on.

    ReplyDelete