Handling exceptions, is this a good way?

Question

We're struggling with a policy to correctly handle exceptions in our application. Here's our goals for it (summarized):

• Handle only specific exceptions.
• Handle only exceptions that you can correct
• Log only once.

We've come out with a solution that involves a generic Application Specific Exception and works like this in a piece of code:

try {
  // Do whatever
}
catch(ArgumentNullException ane)
{
  // Handle, optinally log and continue
}
catch(AppSpecificException)
{
  // Rethrow, don't log, don't do anything else
  throw;
}
catch(Exception e)
{
  // Log, encapsulate (so that it won't be logged again) and throw
  Logger.Log("Really bad thing", e.Message, e);
  throw new AppSpecificException(e)
}

All exception is logged and then turned to an AppSpecificException so that it won't be logged again. Eventually it will reach the last resort event handler that will deal with it if it has to.

I don't have so much experience with exception handling patterns... Is this a good way to solve our goals? Has it any major drawbacks or big red warnings?

Note: One of the drawbacks of this is that after the first catch you lose the ability to handle an specific exception (if you call a method that calls another method and the second one throws an exception you're not able to handle it) but I've found I've never done this any way ... I only handle exceptions with one level of depth ...

Answer 1

This is a quite common approach to solve the exception handling problem (from more specific to less specific).

Just bear in mind that having one generic ApplicationSpecific exception to catch everything that happens in that application/method is not a great idea if you want to catch specific problems. Eventually try extending it with more specific exceptions.

Rethrowing exceptions is good, better is declaring the method to throw certain exceptions and let callers handle them. This way you'll have to create less code and you could centralize some controls.

Answer 2

If you log the exception too near the time it is first thrown, you won't be logging the full stack trace.

Handle exceptions (that is, fix them), as close as possible to when they were thrown. Gather information about the context as soon as possible to when they were thrown. But allow exceptions to propagate up to where they can actually be handled. Logging is a last-resort sort of handling, so it should occur in the outer layers of application subsystems.

This should eliminate the need for an application-specific exception used as a marker to not log an exception which shouldn't have been caught to begin with.

Answer 3

Don't log an exception and then re-throw it - it is the callers responsibility to handle / log any exceptions that you generate.

Only catch an exception to handle it (for example to log it), or add context specific information.

Answer 4

A good solution about exception handling is using Interception. However you must validate if this pattern can be applied to your application depending on the architecture : Interception requires a container.

The principle is to factorise your exception handling outside the methods by using attributes (custom) on them and then use the container to initialize your instances. The container will proxy theses instances by reflection : its called instances interceptors. You'll just have to call your methods as usual via theses instances and let interception mechanism do the job you coded before or after the method.

Note that you can add try catch in the methods to manage specific exception that are unmanaged in your interceptors.

Unity interception : http://msdn.microsoft.com/en-us/library/dd140045.aspx

Answer 5

First option to solve the stack trace problem:

class AppSpecificException : ApplicationException
{
    public string SpecificTrace { get; private set; }
    public string SpecificMessage { get; private set; }

    public AppSpecificException(string message, Exception innerException)
    {
        SpecificMessage = message;
        SpecificTrace = innerException.StackTrace;
    }

}

I had to write an example to understand the question and check the stacktrace problem, this is the code to me, put attention to the button2_click method, finally my textbox show the crash string and the stacktrace:

    private String internalValue;

    private void Operation1(String pField)
    {
        if (pField == null) throw new ArgumentNullException("pField");
        internalValue = pField;
    }

    private void Operation2(Object pField)
    {
        if (pField == null) throw new ArgumentNullException("pField");
        internalValue = Convert.ToInt32(pField).ToString();
    }

    private void Operation3(String pField)
    {
        if (pField == null) throw new ArgumentNullException("pField");
        internalValue = pField;
        Operation2(-1);
    }


    /// <exception cref="AppSpecificException"><c>AppSpecificException</c>.</exception>
    private void button1_Click(object sender, EventArgs e)
    {
        try
        {
            Operation1("One");
            Operation2("Two");
            Operation3("Three");
            MessageBox.Show(internalValue);
        }
        catch (ArgumentNullException ex)
        {
            textBoxException.Text = ex.Message + (char) 13 + (char) 10 + ex.StackTrace;
        }
        catch (AppSpecificException ex)
        {
            //textBoxException.Text = ex.Message + (char)13 + (char)10 + ex.StackTrace;
            throw;
        }
        catch (Exception ex)
        {
            textBoxException.Text = ex.Message + (char)13 + (char)10 + ex.StackTrace;                    
            throw new AppSpecificException("crash", ex);
        }

    }

    private void button2_Click(object sender, EventArgs e)
    {
        try
        {
            button1_Click(sender, e);
        }
        catch (AppSpecificException ex)
        {
            textBoxException.Text = ex.SpecificMessage + (char) 13 + (char) 10 + ex.SpecificTrace;
        }
    }

Answer 6

Try to avoid creating a new exception and and rethrowing, since throwing an exception sets the stack trace to where the exception was thrown. Just do a plain throw. See Too Much Reuse on Eric Lippert's Blog.

Answer 7

I'd recommend putting more thought into what patterns you want to use for "handing"

If your handling patterns come down to log or rethrow, then the rethrown error will eventually be logged. So in the end, it's just error logging. If you're using ASP.NET use elmah so at least your code isn't covered with try/catch-and-log boiler plate.

There are only a few ways to "handle" errors that don't end in mere logging.

Re-try. (Watch out for infinite loops)

Wait and re-try.

Try different but equivalent technique (Can't connect on http? try connecting on https).

Establish the missing conditions (create the folder that threw the FolderNotFoundException)

Ignore the error-- think twice about this, it makes sense only when the error isn't really a problem, like if a 3rd party library is warning you about a condition that doesn't apply.

Answer 8

See the comments to this blog post:

http://blogs.msdn.com/ericlippert/archive/2010/03/04/too-much-reuse.aspx

There is some excellent discussion in there of some of the pitfalls of handling exceptions.