I received a question in an e-mail today and it was asking about custom exceptions. There was a few things that the person was curious about, and it mainly revolved around a few topics:
- How do I design exceptions in my applications?
- What are my best practices for exception handling?
- When should objects throw exceptions?
- Do I put exception classes in separate assemblies?
I decided that I would write a blog post about my response to these questions, since my ideas about exceptions definitely differ from what I have read over and over in different recommendations on .NET development. I’m not asking that you follow my practices, I am letting you know what my practices are, so that you can think about them and hopefully provide feedback. The answers to these questions may vary depending on the requirements of the individual application, but on the average case, these would be my answers.
1) When it comes to designing exceptions in my application, I take an extremely pragmatic approach. I learn towards almost always using Framework exceptions before creating custom exceptions. Exceptions like ArgumentNullException, ArgumentOutOfRangeException, InvalidOperationException, etc… should all be used before creating custom exceptions for your own case. If I do create custom exceptions, I try to keep them extremely generic, since I believe that the important info in an exception is usually in the Message and not the type. The overhead of creating tons of custom exceptions is rarely worth it, when a human is usually the one that ends up looking at an exception. Spend your time creating good meaningful error messages in exceptions, rather than focusing on creating tons of exception types.
2) I don’t care for the term "best practices", but I understand what they mean. In my case, they aren’t "best practices" though, just merely suggestions. 🙂 I usually take the approach of letting exceptions go. I never catch an exception unless I have something specific to do with it. I’ve seen many applications where they catch exceptions all over the place and log the exception and then rethrow it. No no no. In most cases there should be some sort of global error handling/logging. Maybe not global, but at least at a much higher level than at the class level.
When it comes to catching exceptions, I think that there are usually only three reasons to catch an exception:
- If you need to add info to the exception. This is generally done by wrapping the exception.
- You need to perform some cleanup or logic after an exception is thrown. Even then, the exception is usually rethrown.
- You need to recover from the exception, and you are absolutely sure that you know how to do this.
I find that 99% of all cases are number 1 and number 2. Generally speaking an exception is because something bad and unexpected happened, and so you should let it rise up, notify the user, do some logging, and then move on. In general, it should terminate the application (or request). Once an exception is thrown, there are very few cases where you can be sure that you are able to recover. I’ve seen many pieces of code where people put exception handlers in to try and recover from exceptions only to have them cause errors further down the line due to side effects of the exception that the developer could not have anticipated. This can make thing exponentially harder to debug.
3) Only throw exceptions in exceptional cases. That is why they are called exceptions. Reserve exceptions for things that keep the application from performing its duties properly. The one place I see this violated a lot is with validation. People will validate some piece of data coming from the user, and when it is invalid they will throw an exception and then usually catch it only a few lines later in order to do something with it. Most people will cite this as bad practice because throwing exceptions is expensive. If you follow my blog then you’ll know that I hate micro-optimizations in most cases, so to me the real travesty is in disrupting the control flow of your application for very little reason. Throwing an exception is like a goto with no target. Structured exception handling is great, but don’t abuse it in order to circumvent the normal flow of your application.
4) I don’t. I usually keep them close to the code using them. I might do this if I had a really large project and I needed to share my exceptions across several assemblies.
And one last thing, when you do rethrow an exception after some cleanup, make sure that you throw it like this:
try
{
//do some work
}
catch (MyException exc)
{
//cleanup
throw;
}
And not like this:
try
{
//do some work
}
catch (MyException exc)
{
//cleanup
throw exc;
}
The reason for this is simple, one maintains the original exception information (including the proper stack trace), the other does not. Calling "throw exc" now makes it look as if the "throw exc" line was where the exception was raised, making it potentially much harder to debug. Oh, and you save yourself some typing.
So how do you handle your exceptions?
Loved the article? Hated it? Didn’t even read it?
We’d love to hear from you.
I have a bit of an issue with recommending that any custom exceptions be on the "fairly generic".
This seems to be at odds with the recommendations in the .NET Framework Design guidelines. Exceptions should be specific. The point of throwing an exception is so that it can be handled. How can you handle generic exceptions? Are you proposing parsing the messages to handle the exception? Certainly, the message is useful for investigation after the fact but the exception type is useful during the handling.
In a theoretical example consider throwing FileException. Suppose you catch FileException. What can you do?
Now, if you throw FileInUseException or FileDoesNotExistException the handling code becomes very clear.
Krystof Cwalina says that "well-designed exception hierarchies are wide, not very deep and contain only those exceptions for which there is a programmatic scenario for catching."
http://blogs.msdn.com/brada/archive/2004/03/25/96251.aspx
Does that argument change your recommendation at all? I agree wholeheartedly with the rest of your points. I’ve seen some awful rethrowing in recent code reviews at my company.
I think the following is more helpful that just re throwing:
throw new Excetption("mymessage", exc);
@Eddie Yup, I do disagree with the .NET Framework design guidelines. I think that in many cases there is absolutely no need to catch and programmatically do anything with an exception. In fact, I think that most applications try to do things with exceptions when they should just let them bubble up.
If there is a specific kind of error that needs to be handled in a specific way, then sure, make an exception for this and catch it appropriately. Maybe it is my dynamic language leanings, but when I think about how often I need to check custom exception types in order to deal with them, it isn’t all that often.
Most of the things that I need to handle programmatically are *expected* events, and in those cases, I don’t throw exceptions. Maybe it is my style of coding that leads me in this direction.
@t_moriarty That is exactly what I don’t think is necessary very often. I see apps that create tons of custom exception types, catch errors everywhere and then wrap the meaningful framework exceptions in custom exceptions that usually add no value. To me this is the definition of waste. In most cases, letting the exceptions bubble up is IMO the correct course of action.
@Justin So how do you programmatically handle those *expected* events (like file not found) when they happen somewhere in the lower layers of your application. Do you let your methods return error codes, that then travel up the call stack through intermediate layers to the highest layer, finally to be displayed in a messagebox or something?
The design guideline quoted by Eddie is consistent with your thoughts. That is why Krystof says you should only create exception types where "there is a programmatic scenario for catching." i.e. if you do not have a scenario for catching, don’t create it.
I also think that the phrase "only throw exceptions in exceptional cases" is a little overused. IMO people read that as "my code should not have that many throws in it". I prefer to say that exceptions should be thrown whenever a member cannot do what it is designed to do (paraphrased from Krystof http://bit.ly/7N8MF). This is easier to apply to a given case. For example, a validator is designed to validate. If it does that, regardless of the result, there is no need to throw. If it can’t do it for some reason (e.g. it can’t read the validator config) then it should throw.
Nit picking out of the way 🙂 I agree with everything you say and the sooner we rid the worlds code bases of awful looking exception code the better!
@Peter If your application cannot function with the file missing, and therefore should always be there, then it is an exceptional event. In that case, why not raise a FileNotFoundException?
What I see people doing, and what is often advocated, is to create a SomethingConfigFileNotFoundException and SomethingElseConfigFileNotFoundException and then raise them accordingly. To me, this is wasteful since these two exceptions would almost never need to be handled differently outside of the tiny scope in which they are being raised. Generally logging them out is enough. But that all depends on your application.
I absolutely [b]hate[/b] the advice "Only throw exceptions in cases that are exceptional." It is too vague and ambiguous to be useful. It’s like saying "Only eat food that is edible."
A far better guideline is one that I came across via Scott Hanselman: "Throw an exception if your method can’t do what its name says it does." This dovetails very nicely with concepts such as the Single Responsibility Principle.
@Ben Thank you for the correction! I’m glad that I am in line with guidelines.
As far as exception cases, that is why I followed it up with "Reserve exceptions for things that keep the application from performing its duties properly."
@James As my response to Ben said, I did follow it up with "Reserve exceptions for things that keep the application from performing its duties properly." Maybe that still isn’t clear enough? I agree though, saying "exceptional" isn’t enough of a description.
There are quite a few good points up there that I agree with. A few notes:
1) throw new Exception("mymessage", exc); – I prefer this syntax to the ones offered. Basically, this allows me to have very very generic exceptions w/ very specific examples. In most of the examples given, you are stating scenarios in which execptions are rethrown. Most of my throw’s are dedicated to instances in which I know there is a data / processing issue (often that can *never* exist, but somehow does) and the compiler might not catch it due to its nature. Using the message attribute is extraordinarily handy for this.
Also, when you do rethrow exceptions and you are using an global exception handler, this throw approach is very important as it allows you to capture many more details than you exception logic. Especially for scenarios in which your exception is not ‘looked at’ until several days later. Once again, it keeps you from rolling custom exceptions in order to pass one attribute for one instance of the exception.
Finally, a question:
You mention exception handling higher up. I would be interested in knowing what everyone uses. Do you use the health monitoring provider for .net, a third party tool, or something else?
@Nathan Excellent point, wrapping an exception to add additional info is often necessary.
As far as 3rd party tools, ELMAH has been getting a lot of looks recently.
Justin, now we’re finding common ground. I agree that the Catch and handle code is rare and exceptional (no pun intended). I recently worked on a very large Windows Forms application and we had about a dozen justified Catch statements. And, in my opinion, the only one that really mattered was the one in Sub Main() that logged the issue.
But, once you commit to generic exceptions, you’re fairly tied to it. You need to change the caller and callee to handle anything appropriately.
Your example about ConfigFileException versus SomethingConfigFileException and SomethingElseConfigFileException adds clarity. That’s exactly what I would do. It sounds like your concept of a generic exception is my idea of specific exception. I think we are saying the same thing — it is just a frames of reference issue.
From a blog post of mine a while back:
"Exceptions definitely should not be used for first-chance validation purposes; a validation failure (or collection thereof) should be returned to consuming code. The client code can then choose to present these violation in some meaningful manner. Of course, it is okay to throw an exception on second-chance validation prior to performing an operation; but client code would interpret this as a failure. Throwing and catching of exceptions do come at a cost."
My two cents.
Thank you for adding that last bit. I think this is something that you can’t stress enough. I can’t count how many times I’ve gone through error logs and gone to the line of code that was the origin of the exception just to find the following line:
throw exc;
One quote I really like concerning exceptions is, "Don’t use exceptions to control the flow of your application or to apply business logic." (I can’t remember who said that, but it was good.) I love the analogy of the goto without a target.
Keep up the good work.
"You need to perform some cleanup or logic after an exception is thrown. Even then, the exception is usually rethrown."
I think I get what you’re saying here, but it’s worth noting that most of the time, the right way to do this is:
try
{
…
}
finally
{
//clean up
}
OR for IDisposables
using(…)
{
}
Just 2 notes.
A situation I use exceptions a couple of times is when I know things can go wrong, but don’t mind if they do. An empty catch, will keep the cycle going. For instance if I need to process and exctact text from different files, and one is protected, an Exception may be thrown. I this case I’ll just anotate/log the error (if needed or usefull) and skip to the next iteration.
second note is about your 4th point (throw instead of throw exec) Which I didn’t know about and will help alot in resolving a couple of situations. Thanks! 😀
@Miguel – "An empty catch, will keep the cycle going".
Phew – this is the sort of thing that I berate my developers for doing! It is an anathema, the root of all evil, the devil incarnate … no!!!!
If you actually want bugs that are difficult to find, then by all means, pepper your code with empty catch blocks.
I completely agree with Justin in this post – exceptions are exceptional, and they’re a means for improving our lives, not making them more difficult, so leave ’em be *if at all possible*. Are you one of those people who likes to do GC.Collect() as well?
That being said, you should still have a system-wide instrumentation framework for logging exceptions nicely once they’ve trickled up to the top. Arguably, you also need a selective translation layer that will present an exceptional condition in a user-friendly manner when it does take place. Smells of "cross-cutting concern" to me.
In applications that are to be localized, and the error condition is something that will be presented to the end user as "Something like X went wrong, perhaps you should do Y to your system", generic exceptions with messages are not a good way to go.
It’s a lot simpler to create lots of trivial Exception types, with no message (the message implicit in the type) and those items which are peculiar to this instance of the exception as custom properties. That way all the static string-valued information is confined to the UI layer where it belongs, and the exception type can be directly mapped to an appropriate string resource.
I also see a lot of "throwing and catching exceptions is slow" kind of arguments. That depends on what you mean by "slow"..
http://nomorehacks.wordpress.com/2008/06/13/exceptionally-fast/
Of course, if your exception is slupring a lot of data into itself, your mileage may vary.
Dude, sometimes recommending exception to be generic blows up the legs of the programmer. For instance once i had to disambiguate on the IOException message when two threads were using PipedInputStream and PipedOutputStream. Not fun to discover, and brittle. When is a jdk developer going to change the message for giggles.
I can only speak for myself. I have developed a medium sized project, and I wrap most exceptions if the methods are to be used by other classes. This way, I am free to change the internal workings of the class, without having to add "catch" statements to other classes.
Well, you know how Exceptions have catch blocks for different types? I have tried on several occasions to wrap all error messages that I am throwing into custom Exception classes in order for other people to be able to selectively catch them.
Is it a good practice? I don’t know. I have never found a use for this pattern, nor anyone working with my stuff, albeit, they rarely use my stuff anyway and make me do all the work. 🙂
Great article, learned some things I didn’t know.
Hi Justin, nice article!
I have two relevant articles about exception handling practices, which could be useful for readers:
http://www.coderecycling.net/2009/03/code-quality-by-exception-handling.html
The other one is about centralized exception logging, without unnecessary try/catch blocks: http://www.coderecycling.net/2009/04/centralized-exception-handling-logging.html