Exceptional rant

published: Mon, 28-Mar-2005   |   updated: Sat, 25-Aug-2018

I found an interesting story about a process-deficient software company this morning (via Larkware). The story itself was interesting for sure -- it's about the need for developers to fix their compiler warnings and keep them fixed, a proposition with which I heartily concur and follow -- but it was the comments that got my goat.

One of the first commenters stated that he didn't know how to get rid of the warning the C# compiler gives you when compiling this code (edited to follow my preferred coding standards):

try { 
  // something 
} 
catch (Exception ex) { 
  throw CustomException("Uhhh you need to specify a user name"); 
} 

The compiler flags the ex variable as being unused.

The answer is to just remove the ex altogether. It seems to be a little known fact that you can just catch on an exception type; you don't have to specify a variable. And various people have helped the original commenter with this response.

Unfortunately (at least at the time of writing) no one has pointed out that catching a bare Exception is a Bad Thing, period. I've talked about it in the past, see here and here for examples.

Now in the above snippet of code I don't know what the "// something" comment hides. It could make connections to a database. It could read a configuration file from disk. It could authenticate through a web service. It could do any number of weird system-like things that might fail. No matter what, though, all of these would be gathered together into a "user name required" error.

Also gathered into a "user name required" error would be:

• an out of memory exception (rare, but can happen on CE devices relatively easily)
• a thread abort exception (a thread was aborted, presumably because your code did it)
• an execution engine exception (bye bye, JITter)
• a stack overflow exception (for recursion see recursion)
• an invalid program exception (bad IL in program image)

Actually with some of these, you won't even be able to log the fact in code like this:

catch (Exception ex) {
  MyLogger.LogException(ex); // who knows what happens in here?
  throw;
}

This pattern of catching Exception bothers the heck out of me; does it you?. It's dangerous: one day you'll catch more than you bargained for. If you want to catch an exception do so, but just don't catch Exception.

And it's so friggin' prevalent: I've been writing a quick lightweight class to ping a SQL Server instance to see if it's alive (it's much more efficient than trying to open a connection). So I've been using sockets for the first time in C# (and it's been a long time since I did it in Delphi!) and relying on the MSDN documentation (more on that in a separate article).

So, here's some sample code from MSDN that goes along with the Socket.Bind() topic:

try {
  aSocket.Bind(anEndPoint);
}
catch (Exception e) {
  Console.WriteLine("Winsock error: " + e.ToString());
}

WTF?, I thought, because I'm apt to think in TLAs. Socket exceptions are thrown as SocketException instances. Yes, there are other exceptions that Bind() could throw (SecurityException being one) but if you need to know about those, catch them explicitly. Even the CLR UA team at Microsoft gets it wrong (and the proofreaders didn't catch it either).

I'd recommend using FxCop religiously: one of its rules warns you against catching instances of Exception. I'd ssay treat the warning as an error.