27.7.10

Why you shouldn't handle exceptions

Every programmer understands exception handling. You put your code in a try block. If it pukes, do something with in a catch block. Finally, if you need to clean up anything regardless of failure, put it in the finally block. What I would like to focus on today is the middle step, catching exceptions.


try {
externalResource.DoSomething();
} catch { } // we don't care if this call fails


I know some of you are cringing at this point. However lots of programmers do this. On the surface it seems logical. This
may be a call to some external resource that may or may not be there, and the application doesn't care about it. Did you know that?
  • This code can leave the system in a exploitable state.
  • You may have deprived a hosting application (e.g. web server) from logging its own error.
  • The operating system itself may now be in an unrecoverable state. Causing several applications on the machine to act up. With no record of what happened.


These are all very real and serious issues. Let's see if we can fix this:

try {
externalResource.DoSomething();
} catch (Exception fail) {
Logger.Log(fail); // we don't care if this call fails
}


There that looks better. Or is it? You still have the same issues as above, but now at
least you have something to help track them down. This doesn't leave me feeling warm and fuzzy. I wouldn't want to be
known as the guy who let some hacker bring down our systems, or worse. Let's see if we can do this:


if (externalResouce.CanConnect())
externalResource.DoSomething();


Much better! Unfortunately our third party code doesn't have this method. So we are still stuck with our exception
handler. Woe is me what ever shall we do? I know, let's see what exception is actually being thrown and catch that.


try {
externalResource.DoSomething();
} catch (CannotConnectException fail) {
Logger.Log(fail);
}


Perfect. Now we aren't catching any of the exceptions that we don't know what to do with. We
are also logging the one that occurred so at least someone can be made aware of it.



Six months goes by, everyone is happy. Then one day:

Dept B: We saw the application you built for Dept. A and we saw it sends stuff to ExternalResource.
We want a new application too. We currently enter this information into ExternalResource by hand, but now
that we know you can do it automatically. We want it in our new application.

You: no problem. I thought someone else might want to use that functionality. I put it in a separate library.

Dept B: ???

You: Nevermind. Yes we can do that for you.



Now the application for Dept. B has been in production for a week, and everyone is excited
that you were able to write it so quickly. Things are looking good!


A bug report pops up in your queue: "Dept. B application is not writing to ExternalResouce." What? This can't be my code.
I saw it write to ExternalResouce every time when we wrote and tested the application.



Later that day...

You: boss, um .. we ... um .. have a problem

Boss: what is it?

You: I figured out what the ExternalResouce issue is for Dept. B application

Boss: how is that a problem?

You: well the code that writes to ExternalResouce doesn't fail if ExternalResource is unavailable. If I remove this check
then the application for Dept. A will start breaking. They don't care about ExternalResouce. So now we need to modify Dept. A application
to ignore ExternalResouce, modify the shared library to propagate the exception, and modify Dept. B application to notify the user
that ExternalResouce is unavailable.



WOW!



// in shared library
externalResource.DoSomething();


That is what the code in your library looks like now. Moral of the story? Don't handle exceptions
unless that piece of code knows what to do with it. In our example scenario, if the programmer had
not handled the exception in his library, and instead let the application decide what to do with it, the
problem at the end wouldn't have existed.

If your code is going to be used by some other code, or to say it does not stand on its own.
Then you shouldn't be catching an exception. If your code does know what to do with that exception
and how to recover from it without involving its user, then first check that there is a non-exception way to make the call:
if (File.Exists(myFile))
File.Open(myFile);
Get widget