Developer Drain Brain

November 27, 2011

Is Exception Handling Broken on .Net?

Filed under: Development — rcomian @ 10:47 pm

Here’s a challenge for you. Take a .Net project of yours (a large one, preferably in c#) and look at how much exception handling is done. Now, for every try/catch statement, prove that you’re catching only the exceptions that you expect to be thrown.

This is the challenge facing anyone who wants to correctly handle their error conditions. It’s also a challenge that you should be willing to step up to if you want to get your code windows logo certified.

There’s a problem if you miss exceptions that should be caught. If you do that, your application will simply crash, unceremoniously losing anything your customer hasn’t saved and possibly earning you an angry phone call. In terms of customer satisfaction, it’s clearly the #1 thing to avoid doing.

There’s also a problem if you catch exceptions that you shouldn’t. The worst case scenario there is that you’re corrupting user data. More likely, you’re hiding the place where an original problem occurred and saving the actual crash for later. If you’re really good at catching everything you shouldn’t, you might be limping along in a bad enough state where the user has to just stop and restart the application before things start working properly again. I know I’ve been there as a user many times.

If you’re signed up for Windows Error Reporting, catching too many exceptions means that you don’t get notified of real problems with your application, meaning there are a whole set of problems you don’t see, or end up manifesting in some unexpected and un-debuggable place.

So exception handling appears to be this wonderful balancing act, it’s essential that you get just enough and not too much. I assume you’d rather be pro-active and double check that you’ve got all the error cases correctly covered rather than let your customers find them with a crash.

Good luck.

Seriously, I find it hard to believe I’m writing this, but you can’t do it.

Sure, you can read the documentation for every method you call, check what you need to do for every documented exception and handle it. That’s actually quite a lot of work, especially if you’re doing it for an existing project. If you’re lucky all the exceptions will be documented, they’ll even be documented correctly and fully, including all the 3rd party libraries you use … and all the internal ones.

And once you’ve done all that, the only way you can verify that it’s correct in your next release is to do it again. Now what happens when you go to the next version of a 3rd party library or .Net. You need to examine every method you use to see if any new exceptions have been added, old ones removed, or the handling requirements of the others changed. Then check that your code handles them all correctly, in every location.

You’d think there would be a tool to help you with this. But there’s nothing in .Net or visual studio that offers any clue. One company, RedGate, did do this at one point. However, reading their website, they were having trouble keeping the list meaningful, and with .Net 4, they gave up.

So even if you catch only the exceptions that are actually thrown, you’re safe, right?
Of course not. Because someone, somewhere will be throwing a raw “System.Exception”, and the only way to catch that is to catch “System.Exception” and that catches everything. So now you need to catch everything, then check to see if it’s what you really thought it was and re-throw everything else.

Of course, you’re re-throwing with just the “throw” statement, right? Not the “throw obj” version of the statement. Everywhere. Sure?

Lets assume you can change this code to throw something sensible instead of a raw System.Exception. How about a System.ApplicationException, that’s bound to be just the exception we were expecting to catch, right? If you answered “No”, you’re getting the hang of this. The .Net framework will happily throw its own exceptions which are derived from ApplicationException, even if you haven’t derived anything from it yourself. So you still need to check the actual exception type after you catch it just to make sure it’s something you expected. No, you need to ignore what Microsoft have given you and make your own exception hierarchy derived directly from System.Exception.

Ok, so we’ve done all that. We’ve manually gone through the latest documentation for every method we ever call and checked that we’re handling all the exceptions correctly. We’re not throwing any base-class exceptions that aren’t our own and we’re re-throwing everything correctly. We’re clear now, right?

Ah, you’re catching on, not yet. Let’s look at that web service you use, you’re catching a WebException, right? You’ve checked the inner exception as well, haven’t you? Why? Because although Microsoft will deny you a windows logo certification if you catch too many exceptions, they don’t follow their own advice. If something like AccessViolationException occurs whilst calling a web service, it will get wrapped up very nicely in a WebException and presented to you just like a timeout, or a connection failure, or a 500 error. So you might need to leak a WebException.

So what could InnerException be in these cases then? Surely that’s documented? Yeah, right.

Why is this such a hard problem?

It strikes me after much pain working through all this, that .Net’s exception handling is fundamentally flawed.

Firstly, we rely on documentation to work out what exceptions can be called. We don’t rely on documentation to work out what parameters to pass, or what return values we get, yet handling the error conditions is left to our discretion.

Secondly, there are some exceptions which could get thrown at any point and would probably never be thrown by your own code. The clearest cases are things like StackOverflowException and AccessViolationException. Perhaps NullReferenceException is in this list too. We would almost never want to catch these. These are genuine coding errors and never something to do with the state of our user’s workflow. Yet we have no way to determine what exceptions fall into this category.

The exception hierarchy is a joke. The knee-jerk reaction for how to categorize AccessViolationException is to say “It derives from System.SystemException, exceptions derived from here must be serious!”. And then you find yourself leaking when you parse a user’s badly formatted xml snippet, because XmlException is also derived from there. These aren’t isolated incidents, you can’t tell anything meaningful from the .Net exception hierarchy. The namespace usage is slightly better, but still useless.

If an exception is going to be a base class, it must never be thrown. There’s a built in mechanism for this, it’s called the abstract class. An abstract base class will never be thrown directly preventing a whole category of errors. And once an exception class is made concrete, it must never be subsequently derived from. We have a mechanism for this too – sealed classes can’t be derived from. Yet these mechanisms are not made use of in the built in exception hierarchy.

So what can we do to mitigate these problems?

Well, at the end of the day, your exception handling is your best guess. You can’t make it any better than that, so 90% is the way to go. Sign up to Windows Error Reporting and find what you’re leaking in the field. It’s a shame that as a top tier development company, that’s the best you can do, but it’s the best that anyone can do, so don’t feel bad.

In those cases where a leak would be a truly terrible thing, do what you did anyway.
Let’s be honest, your windows form event handlers are all wrapped with “try {} catch (Exception e) {}” aren’t they. Because after years of pissed off customers that’s what you’ve resorted to. Yes, your deeper code all does proper exception handling as you want, but the lure of the safety net was too much for your bottom line to resist. Well, in those critical, high level places, keep doing that. But try to categorize the list of exceptions you don’t want to catch and make sure you leak them. Unless you’re doing something really funky, you don’t want to hide an AccessViolationException.

And of course, unit testing can help. It’s not 100%, firstly because your coverage won’t be 100%, also because you’re still relying on documentation to work out what exceptions you’re supposed to be emulating, and also because a lot of exceptions that happen in practice are environmental problems, like dropped connections.

But the best way to handle your exceptions?

Don’t.

At least, don’t if you can get away with it. Just crash. In many cases that’s the best thing to do. If you’re a service, it’s a no-brainer, assuming your service will be auto-restarted. A web page returning an error is a good thing for visibility of errors.
They get logged, and you can find out about every single one of them as they happen.

Avoid getting in to situations where exceptions are possible. Always use “TryParse” instead of “Parse”, for example. Always check that the file exists before opening it. That sort of thing. If you’re thinking of throwing an exception, offer a way for people to find out what the situation is before you need to throw it.

Going 100% exception free isn’t possible. Some databases throw exceptions that demand the transaction be retried. That’s aggravating, but handle it and leak everything else.
A web service call should be retried a couple of times if certain error codes get returned. But anything else, let it go.
Keep it minimal.

The cost of crashing a windows application is higher. But why are you writing thick client applications in this day and age? Perhaps you can treat it as a container for applets that crash with impunity – I’m thinking of Chrome’s “aw snap!” errors here.

Crash, seriously. It’s the future.

Advertisements

Blog at WordPress.com.