General Exception Handling Strategy for .NET

Question

I’m used to having try/catch blocks in every method. The reason for this is so that I can catch every exception at the point of infraction and log it. I understand, from my reading and conversations with others, that this isn’t a popular view. One should only catch what one is prepared to handle. However, if I don’t catch at the point of infraction, then it would be possible to never log that infraction and know about it. Note: When I do catch and don’t handle, I still throw. This allows me to let the exception propagate to something that will handle it, yet still let me log it at the point of infraction.

So... How does one avoid try/catch in every method, yet still log the error at the point at which it occurred?

Sallust

What language? Different languages have different ways of dealing with the traceback from point of catch to point where it was thrown. – Dominickdominie

WHat drives your need to log at the infraction point? if you log the call stack included in the exception you would still be able to se in the log what was going on and what route your code took to end up at that point – Morganne

There were two reasons for logging at the infraction point. One, the method could send objects and their values to be logged. Two, there could be code higher in the call stack that could end up ignoring or mishandling the exception and it wouldn't get logged. – Sallust

Answer 1

No, don't catch everything. Exceptions propagate higher up on the stack. All you have to do is make sure that the exception is caught before it gets to the top of the stack.

This means, for instance, that you should surround the code of an event handler with a try/catch block. An event handler may be the "top of the stack". Same for a ThreadStart handler or a callback from an asynchronous method.

You also want to catch exceptions on layer boundaries, though in that case, you might just want to wrap the exception in a layer-specific exception.

In the case of ASP.NET, you may decide to allow ASP.NET Health Monitoring to log the exception for you.

But you certainly don't ever need to catch exceptions in every method. That's a major anti-pattern. I would loudly object to you checking in code with that kind of exception handling.

Otalgia

+1 for "No, don't catch everything." good start there on this type question although I would have said "No, PLEASE don't catch everything." :) - I find that where code does this it is harder to follow. – Vey

@JohnSaunders - what if we want to log some of the arguments in the called function that caused the exception? That information would otherwise be lost – Crosscrosslet

That would be a reason to catch, log, rethrow – Otalgia

@JohnSaunders - and isn't edit-and-continue available to actually debug and fix the code before the exception starts bubbling up? That's why I was doing it -- productivity. – Acetous

@JohnSaunders - Here is where I wanted mixins or multiple inheritance or some kind of way of keeping all of that junk out of the way unless I needed it, and then temporarily putting the try-catch(all)-rethrow on everything (by a setting which kicked off some kind of code generation). That's what I imagined, anyway. I am currently benighted by trying to figure out Exception Handling for vb.net into which I have recently jumped. – Acetous

Answer 2

You can see everything at stack trace - no need to try/catch every method.

Stick to few rules:

1. Use try/catch only if you want to use a custom exception type
2. Define a new exception type only if upper levels needs to know that
3. Try/catch at top level instead of doing that for each method

Gerent

Answer 3

OK, having read all the answers saying you should have a single try/catch at the top level, I'm going to weigh in with an alternative view.

I wouldn't put a try/catch in every method, far from it. But I would use a try/catch around sections of code that I expected to fail (e.g. opening a file), and where I wanted to add additional information to the exception (to be logged higher up the chain).

A stack trace saying and a message saying "permission denied" might be enough to allow you as a programmer to figure out what went wrong, but my goal is to provide the user with meaningful information, such as "Could not open file 'C:\lockedfile.txt'. Permission denied.".

As in:

private void DoSomethingWithFile(string filename)
{
    // Note: try/catch doesn't need to surround the whole method...

    if (File.Exists(filename))
    {
        try
        {
            // do something involving the file
        }
        catch (Exception ex)
        {
            throw new ApplicationException(string.Format("Cannot do something with file '{0}'.", filename), ex);
        }
    }
}

I'd also like to mention that even the people saying "only have one try/catch" would presumably still use try/finally throughout their code, since that's the only way to guarantee proper cleanup, etc.

Griseldagriseldis

FYI, ApplicationException is deprecated. I also would not catch Exception, but rather only those exceptions related to what I was doing in the try block, like IOException. – Otalgia

In general, only catch exceptions when NOT catching them would be worse. – Otalgia

Answer 4

1. Catching and rethrowing an exception that you cannot handle is nothing more than a waste of processor time. If you can't do anything with or about the exception, ignore it and let the caller respond to it.
2. If you want to log every exception, a global exception handler will do just fine. In .NET, the stack trace is an object; its properties can be inspected like any other. You can write the properties of the stack trace (even in string form) to your log of choice.
3. If you want to ensure that every exception is caught, a global exception handler should do the trick. In point of fact, no application should be without one.
4. Your catch blocks should only catch exceptions that you know that you can gracefully recover from. That is, if you can do something about it, catch it. Otherwise, let the caller worry about it. If no callers can do anything about it, let the global exception handler catch it, and log it.

Corollary

Plus 1 for #3 the global exception handler. – Acetous

Answer 5

I definitely don't use a try catch wrapper around every method (oddly enough, I did when I first started but that was before I learned better ways).

1) To prevent the program from crashing and the users losing their info, I do this

                runProgram:
                try
                {
                    container.ShowDialog();
                }
                catch (Exception ex)
                {
                    ExceptionManager.Publish(ex);
                    if (MessageBox.Show("A fatal error has occurred.  Please save work and restart program.  Would you like to try to continue?", "Fatal Error", MessageBoxButtons.YesNo) == DialogResult.Yes)
                        goto runProgram;
                    container.Close();
                }

container is where my application starts so this basically puts a wrapper around my entire app so that nothing causes a crash that can't be recovered. This is one of those rare instances where I don't really mind using a goto (it is a small amount of code and still quite readable)

2) I only catch exceptions in methods where I expect something could go wrong (such as a timeout).

3) Just as a point of readibility, if you have a try catch block with a bunch of code in the try section and a bunch in the catch section, it is better to extract that code to a well named method.

   public void delete(Page page) 
   {
      try 
      {
         deletePageAndAllReferences(page)
      }
      catch (Exception e) 
      {
         logError(e);
      }
   }

Jigger

Answer 6

To do it at the point of occurrence, you would still need a try/catch. But you don't necessarily need to catch the exceptions everywhere. They propagate up the call stack, and when they are caught, you get a stack trace. So if a problem emerges, you can always add more try/catches as needed.

Consider checking out one of the many logging frameworks that are available.

Hertahertberg

The point of having the try/catch at the point of occurrence is so that it's assured of getting logged. If it propagates, other exceptions could occur and/or other code may not end up logging it like I'm hoping for. – Sallust

Consider checking out one of the many logging frameworks that are available – Hertahertberg

They can provide insight into the best ways of doing this. – Hertahertberg

Answer 7

I don't think you need to catch everything at the point of infraction. You can bubble up your exceptions, and then use the StackTrace to figure out where the point of infraction actually occurred.

Also, if you need to have a try catch block, the best approach I've heard is to isolate it in a method, as to not clutter the code with huge try catch blocks. Also, make as few statements as possible in the try statement.

Of course, just to reiterate, bubbling up your exceptions to the top and logging the stacktrace is a better approach than nesting try-catch-log-throw blocks all throughout your code.

Patton

Answer 8

I would look into using ELMAH for exception handling, which is pretty much a concept of "let exceptions happen". ELMAH will take care of logging them and you can even set it to email you when exceptions for a certain project reach or exceed a specific threshold. In my department, we stay as far away from try/catch blocks as possible. If something is wrong in the application, we want to know right away what the issue is so we can fix it, instead of repressing the exception and handling it in code.

If an exception happens, that means that something isn't right. The idea is to make your application do only what it should do. If it's doing something different and causing exceptions, your response should be to fix the reason it's happening, not let it happen and handle it in code. This is just my/our philosophy and it's not for everyone. But we've all been burned way too many times by an application "eating" an exception for some reason or another and no one knows that something is wrong.

And never, ever, EVER catch a general Exception. Always, always, always catch the most specific exception so that if an exception is thrown, but it's not the type you're expecting, again, you'll know because the app will crash. If you just catch (Exception e), then no matter what type of exception is thrown, your catch block will now be responsible for responding to every single type of exception that could possibly be thrown. And if it doesn't, then you run into the whole "eating" exceptions, where something is going wrong but you never know until it is likely too late.

Ethelynethene

Looks like ELMAH is an ASP.NET-only framework, as it uses HTTP Modules and Handlers to do what looks like a great tool. An article on MSDN covers it -- "Using HTTP Modules and Handlers to Create Pluggable ASP.NET Components" – Acetous

Answer 9

How does one avoid try/catch in every method, yet still log the error at the point at which it occurred?

It depends on the hosting env. Asp.Net, WinForms, and WPF all have different ways of capturing unhandled exceptions. But once the global handler is passes an exception instance you can determine the point of throw from the exception, as each exception includes a stacktrace.

Zincography

Based on the feedback, I think I’m comfortable now with this approach: My WCF service will include a try/catch at the boundary only (the service class itself, not at the BLL or DAL layers). At the boundary, I’ll log it and record the stack trace. Then I’ll throw ONE exception so the client can catch it and handle it. – Sallust

As to WCF, remember that web services use faults. If you're going to do this, pick a single FaultContract, and throw FaultException<thatContract>. Your top try/catch should allow all FaultException to pass, as another layer may have returned a specific fault. – Otalgia

Answer 10

Realistically, avoid granular try/catches. Allow the exception to traverse upwards in the stack and be caught in as high a level as possible. If you have a specific point of concern, then place logging in the immediate catch, if you are worried about the exception cascading - although you would still be able to resolve these by drilling into the inner exceptions.

Exception handling should not be an afterthought. Make sure that you do it consistently. I have seen a lot of people put a broad try/catch from the beginning to the end of every method and catch the general exception. People think that this helps them get more information, when in fact, it doesn't. More is less in some cases, as less is more. I never get tired of the axiom that "Exceptions should be used to note exceptional behavior." Recover if you can, and try to reduce the number of overall exceptions. Nothing is more frustrating when you are trying to troubleshoot an issue and seeing hundreds of the same NullReferenceException, or similar, when something goes wrong.

Tosch

Answer 11

Exceptions are implemented so that they have no cost unless thrown.

This means to me that performance ramifications is not a strong argument against. Exceptional conditions usually are ... exceptional.

Roos