Ayende @ Rahien

Refunds available at head office

Kobe – an example of exception handling done wrong

In Kobe, pretty much all exception handling is this:

catch (Exception ex)
{
    bool rethrow = ExceptionPolicy.HandleException(ex, "Service Policy");
    if (rethrow && WebOperationContext.Current == null)
    {
        throw;
    }
    return null;
}

These lines appear in just about any method in the application.

They are wrong. In fact, they are wrong on more than one level. Violating DRY is a big offence, but even ignoring that, they are still wrong.

Exception handling is not something that you should take lightly, and there is a strong tie between the way that you write the code and the way errors are handled. The whole purpose of this exception handling is to avoid throwing an exception by setting some configuration parameter.

The problem with this is that this purpose is wrong. This is a case of writing the code without actually trying to understand what the implications are. You cannot do this sort of thing, because the calling code never expects a null, it expect a valid result or an exception.

This approach bring us back to the bad old days of error codes. Not to mention that even if we actually checked for a null return value, what the hell is the error that actually caused this to happen.  Oh, it was put in the log, so now we need to go and look at the log file and try to match timestamps (if we even can).

Exception handling should appear in exactly two places:

  • When an error is expected (making a web request call, for example) and there is some meaningful behavior to be done in the case of a failure (such as retrying after some delay)
  • On a system boundary, in which case you need to make a decision about how you are going to expose the error to the outside world.

It is important to note that I explicitly used the term system boundary, instead of service boundary, which is more commonly used. Exception handling within a system should not try to hide errors. Error should propagate all the way to the system boundary, because otherwise we lose a lot of important information.

System boundary is defined as whatever is visible to the actual user or other systems.

Oh, and exception handling in the system boundary is a system infrastructure problem, it is not something that you have to write code for all over the place.

Comments

Poul Foged Nielsen
04/17/2009 09:45 AM by
Poul Foged Nielsen

When you talk about handling of exceptions, you're not covering actual wrapping of exceptions (though innerexception) right?

Becourse I still think that's pretty important ... (other places than the two you mention).

Ayende Rahien
04/17/2009 11:22 AM by
Ayende Rahien

Poul,

Wrapping exceptions (to add additional contextual information) is important, but I wouldn't call it handling the exception

huey
04/17/2009 01:01 PM by
huey

I guess it depends on specifics... but the controller is a decent place for exception handling? It seems like the visible boundary to the user.

Dirk
04/17/2009 01:30 PM by
Dirk

The exception has to be handeld somewhere doesn't it?

Otherwise you would be seeing stack traces in the UI...

Ayende Rahien
04/17/2009 01:36 PM by
Ayende Rahien

Dirk,

The exception is handled at the system boundary

meisinger
04/17/2009 04:40 PM by
meisinger

totally agree on the system boundry part

i like to think of it more like a "machine" boundry

so if i am talking to the database... i have exception handling

if i am talking to a web service or message service... i have exception handling

the only other time that i have exception handling is when i am about to send the data to the user (typically in a global manner)

JeroenH
04/17/2009 08:38 PM by
JeroenH

I could not agree more with this remark. This kind of exception 'handling' (which I suspect stems from the Enterprise Library Exception Handling Block and the way it is 'advertised') is a fundamental flaw in several applications I have been forced to work on in the past.

Mr_Simple
04/18/2009 02:25 PM by
Mr_Simple

Gosh my little pea brain likes to wrap everything and send all errors to my log file as soon as possible along with notifying the user.

This way my programs are bulletproof and I can fix any bugs and get on with programming other much needed applications in record time.

Sometimes I wonder if keeping things simple is - well, too simple.

Hendry Luk
04/20/2009 03:17 AM by
Hendry Luk

meisinger

I think what Oren means by 'system boundary' is the outmost presentation layer of your system that interacts with the clients (i.e. human user or other systems that uses your system); instead of low-level parts of the system that touches other systems (DB, web-service), which is exactly where this post argues not to put exception handling.

Although in reality, I do have exception handling in those low-level parts to deal with retry/fallover. When the failure is impossible to resolve, the exception is bubbled up to system boundary.

Asbjørn Ulsberg
04/24/2009 02:13 PM by
Asbjørn Ulsberg

You are completely right, Ayende. This example isn't exception "handling", it's exception "swallowing". It's catching the exception to forget about it and never know it ever occurred. That's exceptionally (pun intented ;) bad architecture.

Exception handling, e.g. writing try/catch, should as you write, happen when you know what exception may occur and act upon it to do something constructive, or when presenting something to another system or user.

Swallowing exceptions and returning null is almost never a good idea, unless that's well-defined behaviour in your system. For example, catching a FileNotFound exception and returning null could make sense in some situations, especially if the reason for not getting any content back doesn't matter to the client.

Comments have been closed on this topic.