Exception class, return codes, and messages


I’ve had a couple of questions recently about how to architecture libraries with respect
to exception handling, specifically related to exception granualarity. Questions like:

  • Is is okay to define an exception with a return code inside of it?
  • How many exception classes should I have?

The first place to start is the Error
Raising and Handling Guidelines
from the Design
Guidelines for Class Libraries
. There is lots of useful information in the design
guidelines even if you aren’t writing libraries. The guidelines do mention something
like “don’t use return codes”, but that’s about it.

One of the high-value design points when designing exceptions is robustness. The robustness
of the customer’s code depends directly on the way you structure your exception hierarchy.
Here’s some code that I’ve seen. Assume that ApplicationException is the actual type
I’m catching, not the base type of a hierarchy:

try
{
    // code here...
}
catch (ApplicationException e)
{
    if (e.Message.IndexOf("Database Connection Error"))
    {
         // recovery code here
    }
}

There are at least two problems with the code, one major, and one a bit more subtle.
Do you see them?

I’ll start with the more subtle one first. If your exception messages are localized,
it’s likely that the string matching will fail. This is a bad bug, because it only
shows up in the localized version in an error case, and your tests aren’t very
likely to catch it.

The second bug is a whopper. If the exception that comes in isn’t a
database connection error, it is swallowed by the catch block, never to be heard
from again. That’s pretty much the worst thing that exception handling code
can do.

The problem is that the exception design has forced the user to look inside the
exception to decide what to do. This is bad, whether it be the message text,
a return code, or the type of an inner exception that the user is looking for. 

To prevent this, I’d recommend the following guideline:

  • Make sure that your user doesn’t have to grovel inside of your exception classes
    to find out whether to handle the exception or not.

One way to determine this is to ask yourself, “Self, would a user ever care about
the different cases where ApplicationException is thrown?” In some cases this
is obvious, but in other cases, it’s not quite so clear. My recommendation is
that you err on the side of more granularity rather than less granularity.


Comments (12)

  1. MartinJ says:

    I wonder if this will get passed on to the SqlClient people. There, you have to look at the exception to find out what kind of problem happened.

    I can understand a generic exception with the OleDb provider. It might just be too hard to anticipate every error that every data provider out there could generate. But, you’d think the SQL Server people would be aware of what could happen.

    As it is, almost always I end up dumping the original SQL statement along with every piece of information in the SqlException. That way, I can at least tell at a glance if it happened to be something I mistyped by copying it into Query Analyzer.

  2. Tristan says:

    The first problem is avoidable by using a GUID for a discriminator instead of the text. With all due respect, I disagree in general with the term "grovel inside the exception class to decide what to do” as I’m not sure, given that multiple exceptions share the same type only with different data, how you’d know if you were capable of handling a particular exception. For instance, say I have a class that has a load method. The load method tries to load up two files. If either of the files isn’t there, a FileNotFoundException is thrown. Now let’s say that my app knows how to fix the first file not existing (maybe it creates it and calls Load again), but the second one is always supposed to be there, so it should not be missing and I can’t handle it and we kick off WER. Because both exceptions throw essentially identical exceptions (differing only by message text and FileName), it isn’t possible to catch just the FileNotFoundException and assume it’s the exception you know how to deal with. However, if the first file being missing generates a FileNotFoundException whose guid differed from the second FileNotFoundException, then you’d have a cleaner way to determine if you can handle a particular exception.

    The second problem (accidentally squashing the exception) is a limitation of C#. VB.Net doesn’t have this issue, because you can use the “when” keyword:

    Catch ex As MyException When ex.ID = 2

    The issue I have with the recommended approach is that that you lose a certain amount of fidelity with exceptions. If you want to determine if an exact exception is being generated in the system, the only way to do this would be to compare both the type and the message and bits of data from the exception. Using a guid system, you can, if you wished, tag every exception that gets thrown.

  3. Tristan, that Load situation is precisely the kind of situation where you’d want to use separate exception types, so you could write something like:

    public void Load() {
    try {
    open(recoverableFile);
    } catch (IOException e) {
    throw new RecoverableLoadException(e);
    }
    try {
    open(unrecoverableFile);
    } catch (IOException e) {
    throw new UnrecoverableLoadException(e);
    }
    }

    Now the consumer of your Load method can distinguish between the two situations and make sure they only catch exceptions they care about.

  4. Tristan says:

    Mike,

    That works, assuming the Load method knows if the caller can recover from the error or not, which wont be the case if you’re building a library and have no clue who will be using your software. Besides, what happens if there are 3 files? 10? Some recoverable in situation A, but not in situation B?

    Here’s the difference between the two approaches, as I see it:

    try{
    Foo.Load();
    }
    catch(FileNotFoundException notFound){
    if(notFound.FileName == "userdef.xml"){
    //create the file and try again
    }
    else{
    throw; //better not forget this!
    }
    }

    If you used a guid to distinguish the different exceptions, plus you had VB’s when keyword, you’d get something like this:

    try

    Foo.Load()

    ‘I can deal with the user def file not found, but that’s all!
    catch notFound as FileNotFoundException when ex.ID = USERDEF_FILENOTFOUND_GUID
    ‘create the file and try again

    end try

  5. I’d say that’s a badly written library. The library writer should know enough about specific cases to say whether users will want to distinguish between types of errors. (If they get it wrong in Rev. 1, they can easily introduce new, more specific sub-types in Rev. 2 without breaking any implicit contracts, too.)

    I mean, look, in the situation you’re giving, the library writers know enough about the exception to know that they want to assign a new enum value to it in this instance — if they think it’s distinguishable enough to do that, they ought to think it’s distinguishable enough to create a subclass for. All you’re really doing in this situation is substituting an enum for type-checking, which is awkward and (as a practical matter) not supported in C#.

  6. Tristan says:

    Hi Mike,

    You haven’t addressed the issue I raised: a subtype is just not specific enough. If you throw 3 different RecoverableLoadExceptions, the client code may still want to handle the first RecoverableLoadException one way and the other 2 exceptions another way.

    Besides, I don’t agree that a library knows enough about the client to know whether or not some exceptions are to be treated differently then others. The library can only throw exceptions with enough information such that the client is capable of handling it in the most effective means possible, if the client code so chooses. I’m merely suggesting that the only way to give enough information is to uniquely identify different exceptions through some other means then the creation of a new exception subtype.

    Note that I’m in favor of exception subtyping. But I believe it’s only part of the solution.

  7. If the library doesn’t know enough to subclass an exception, how does it know enough to set a GUID?

  8. Dhominator says:

    Interesting that this thread doesn’t mention the base class for [Un]RecoverableLoadException… from original post I guess that means from ApplicationException? What about inheriting based on "functionality", in this case from IoException? Though all these cases imply, to me, that you have to double-check the exception type (base or inherited) and possibly spin through InnerException. BTW, when are we gonna get Exception metadata in assemblies? (CR not working… lol)