C#: Throwing Custom Exception Best Practices

Question

I have read a few of the other questions regarding C# Exception Handling Practices but none seem to ask what I am looking for.

If I implement my own custom Exception for a particular class or set of classes. Should all errors that relate to those classes be encapsulated into my exception using inner exception or should I let them fall through?

I was thinking it would be better to catch all exceptions so that the exception can be immediately recognized from my source. I am still passing the original exception as an inner exception. On the other hand, I was thinking it would be redundant to rethrow the exception.

Exception:

class FooException : Exception
{
    //...
}

Option 1: Foo encasulates all Exceptions:

class Foo
{
    DoSomething(int param)
    {
        try 
        {
             if (/*Something Bad*/)
             {  
                 //violates business logic etc... 
                 throw new FooException("Reason...");
             }
             //... 
             //something that might throw an exception
        }
        catch (FooException ex)
        {
             throw;
        }
        catch (Exception ex)
        {
             throw new FooException("Inner Exception", ex);
        }
    }
}

Option 2: Foo throws specific FooExceptions but allows other Exceptions to fall through:

class Foo
{
    DoSomething(int param)
    {
        if  (/*Something Bad*/)
        {
             //violates business logic etc... 
             throw new FooException("Reason...");
        }
        //... 
        //something that might throw an exception and not caught
    }
}

Answer 1

if you run the code snippet for 'Exception' in Visual Studio you have a template of a good practice exception writing.

Answer 2

The most important thing for code to know when catching an exception, which is unfortunately completely missing from the Exception object, is the state of the system relative to what it "should" be (presumably the exception was thrown because there was something wrong). If an error occurs in a LoadDocument method, presumably the document didn't load successfully, but there are at least two possible system states:

1. The system state may be as though the load were never attempted. In this case, it would be entirely proper for the application to continue if it can do so without the loaded document.
2. The system state may be sufficiently corrupted that the best course of action would be to save what can be saved to 'recovery' files (avoid replace the user's good files with possibly-corrupt data) and shut down.

Obviously there will often be other possible states between those extremes. I would suggest that one should endeavor to have a custom exception which explicitly indicates that state #1 exists, and possibly one for #2 if foreseeable but unavoidable circumstances may cause it. Any exceptions which occur and will result in state #1 should be wrapped in an exception object indicating state #1. If exceptions can occur in such a fashion that the system state might be compromised, they should either be wrapped as #2 or allowed to percolate.

Answer 3

Have a look at this MSDN-best-practises.

Consider to use throw instead of throw ex if you want to re-throw caught exceptions, because on this way the original stacktrace keeps preserved(line numbers etc.).

Answer 4

Based on my experience with libraries, you should wrap everything (that you can anticipate) in a FooException for a few reasons:

1. People know it came from your classes, or at least, their usage of them. If they see FileNotFoundException they may be looking all over for it. You're helping them narrow it down. (I realize now that the stack trace serves this purpose, so maybe you can ignore this point.)

2. You can provide more context. Wrapping an FNF with your own exception, you can say "I was trying to load this file for this purpose, and couldn't find it. This hints at possible correct solutions.

3. Your library can handle cleanup correctly. If you let the exception bubble, you're forcing the user to clean up. If you've correctly encapsulated what you were doing, then they have no clue how to handle the situation!

Remember to only wrap the exceptions you can anticipate, like FileNotFound. Don't just wrap Exception and hope for the best.

Answer 5

I always add a couple of properties when creating a custom exception. One is user name or ID. I add a DisplayMessage property to carry text to be displayed to the user. Then, I use the Message property to convey technical details to be recorded in the log.

I catch every error in the Data Access Layer at a level where I can still capture the name of the stored procedure and the values of the parameters passed. Or the inline SQL. Maybe the database name or partial connection string (no credentials, please). Those may go in Message or in their own new custom DatabaseInfo property.

For web pages, I use the same custom exception. I'll put in the Message property the form information -- what the user had entered into every data entry control on the web page, the ID of the item being edited (customer, product, employee, whatever), and the action the user was taking when the exception occurred.

So, my strategy as per your question is: only catch when I can do something about the exception. And quite often, all I can do is log the details. So, I only catch at the point where those details are available, and then rethrow to let the exception bubble up to the UI. And I retain the original exception in my custom exception.

Answer 6

Option 2 is best. I believe best practice is to only catch exceptions when you plan to do something with the exception.

In this case, Option 1 just is wrapping an exception with your own exception. It adds no value and users of your class can no longer just catch ArgumentException, for example, they also need to catch your FooException then do parsing on the inner exception. If the inner exception is not an exception they are able to do something useful with they will need to rethrow.