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
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).
Poul,
Wrapping exceptions (to add additional contextual information) is important, but I wouldn't call it handling the exception
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.
The exception has to be handeld somewhere doesn't it?
Otherwise you would be seeing stack traces in the UI...
Dirk,
The exception is handled at the system boundary
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)
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.
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.
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.
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.
Comment preview