Exception Handling Blunders

When I do code review, one area I give at most care is the Exception Handling. There are two reasons for this.

1. Exception handling is the least tested area in the code.
2. Problems here can give you a few sleepless nights, after the application is moved to production.

If you Google, you will find thousands of links to good practices in exception handling. I am not going repeat all those. However, I will explain a few blunders found in exception handling.

One of the blunders done by the programmers is blank exception handler. See this

Whenever an exception is thrown from this code, it is simply suppressed. Remember, it is not far from now users come with a functionality is not working, and you are left clueless. No idea what happened. And if the users say, the problem happens randomly, you can be sure some thing wrong in exception handlers. What to do then? Simply replace the blank Exception handler with some code to catch and log the exception and move it to production. No other way. Then why don't you do it while you first code?

Here is the second blunder.

Well, here you are showing a user friendly message to the user. No need to worry about security aspects. Your password will not be shown to the user. Neither the database object names. But what happens when you need to fix this issue? What are you going to do with "Functionality not working"? Is it network issue or some one stopped the database? No way to find out.

Why programmers make such mistakes? After all, IT companies hire best people for their work. The answer is simple. The programmers are overconfident that their code will never break. So, they don't give much importance to the exception handlers. It is just done for the sake of coding standards.

Here is the third blunder.

(I have even seen Throw New Exception(ex.Message). But that is a different story). What's wrong here? You are just throwing the exception you got. The code will work the same way if the Exception handler were non existing. That's all? No.

Imagine, Inside SomeMethod another method is called (Say Method1). From this Method, another method, Method2, is called. Method1 and Method2 don't have exception handlers. Then one exception is thrown from Method2. It will be cascaded to the Exception handler in MyMethod (shown above). Then from catch block, a new exception is thrown. Remember the actual exception is happened in Method2, but in the logs you will find that it happened in MyMethod. Just remember this simple piece of misinformation can delay a problem fix for a few days, if it happened in an area you have least access, that is in production.

Here is the last one.

Wondering what's wrong here? I accept this the most intelligent one among the exception handlers listed so far. But here also we can improve.

1. The message is shown to the user. Are you expecting the users to call you and inform you each time such a message is seen? Or will they keep a notepad to write down every strange messages they see? Oh! they have more important things to do. So, add a logging to the Exception Handlers. Use log4net, Enterprise Library or whatever you like, but add logging.
2. What if you get a message "Object reference not set to an instance of an object."? No idea where it happened, among thousands of lines of code. You need to log the Exception.StackTrace as well. You need to log Exception.Message and Exception.StackTrace. There is an easy way. You can log Exception.ToString, which encompasses Message, StackTrace and even details of inner exceptions.
3. Showing the actual exception message to the user may be risky at times. Who knows the message contains your credit card numbers or passwords? There is a high possibility error message contains database object names, which can make a hackers job easy. So, my advice is log the entire exception details and show the user a friendly message.
   

So, how can be the right one here? See below.

That's all for time being. Hope this will help some one to fix the production issues faster.

Thanks.

---