Best Practices for Exception Handling
Subject:   Don't wrap exceptions!
Date:   2003-11-26 07:54:51
From:   fjalvingh
I have some points to make:
- I very much disagree with "wrapping" exceptions. IMO you should NEVER replace exception A with exception B. When you build your part of an application, deciding that "the upper layer cannot ever handle the exception" you WILL severely limit usability of your code!

The example using the SQLexception is a good one: by percolating this to the upper layers they are FREE to handle them IF THEY WANT TO. Perhaps I want to close and reopen the connection and try again?

This decision should NEVER be made by any library code because it shifts too much knowledge of a process into the client code; this in term severely limits reusability.

In addition, by "reclassifying" exceptions you make it difficult to do proper debugging: the REAL reason for an error is hidden in a chained exception somewhere in the wrapped exception. If you have multiple layers of software wrapping exceptions makes it very difficult to find a real reason for a problem.

All this HAS the sad effect of having to specify throws clauses in much of the code used; this is imo best handled by just specifying a generic exception type in the throws clause. Which brings me to another point: I do not really see the need for checked exceptions - they are not helping to write proper code. So I find myself sadly siding with Microsoft here (ehh).

- A basic premise in exception handling should be that you only handle (catch) an exception if you KNOW what to do with it. For typical code this means that the code contains heaps of try .. finally blocks but hardly ever a try .. catch block! In your example catching the exception and just printing it to system.err is dumb indeed, but not knowing what to do with it and obfuscating (wrapping) the exception is imo as bad as that.

- And finally: I agree that exceptions should only be the "vehicle" for efficiently transporting error states thru the application.

They should not normally be used directly in reporting the problem to an end user except when you have made a special error exception class/class hierarchy which is able to report proper (i.e. user-understandable) and perhaps even i18n-ned messages.

Full Threads Newest First

Showing messages 1 through 4 of 4.

  • Don't wrap exceptions!
    2003-11-27 05:39:31  anonymous2 [View]

    "This decision should NEVER be made by any library code because it shifts too much knowledge of a process into the client code; this in term severely limits reusability."

    But it should *also* not be done by your business code - business code shouldn't know about SQL at all!

    So, what you need is a persistence layer, specific to your application. If that layer decides that the Exception can't be handled reasonably, it's a good thing to wrap the original SqlException into a more generic PersistenceException or something.
    • Don't wrap exceptions!
      2003-12-01 08:14:43  anonymous2 [View]

      Sorry, but you're missing the point. Whether the business code knows about SQL or not is simply of no concern. Whether it is a SQLException or your "PersistenceException": the business code should not handle any exceptions it knows nothing about. And wrapping it only removed a single layer: if the business code itself is layered also do you wrap all exceptions of the lower layers???

      Wrapping SQLException in something else does not solve anything: you still have to pass the exception around, and have some layer handle it.

      What I'm saying is that IF you have a SQL error that you do not fix in the originating layer you may as well pass it upwards and have topmost layer decide what to do with it: report some critical failure and log the ACTUAL problem instead of logging the wrapper.
      • Don't wrap exceptions!
        2003-12-03 13:58:50  anonymous2 [View]

        I've read this thread and I disagree with your point of view. What you're basically saying is that any low-level exception that can't be dealt with correctly by the layer above it, should be thrown all the way to the top layer.

        Then consider a typical J2EE application that has a simple web layer, a business layer and a database layer. If some SQL query in the database layer goes completely wrong I will get an SQLException of some sort. In your point of view the web application (being the topmost layer) will catch this exception in the end.

        This defeats the entire purpose of having a multitier (3+) architecture. I'm now dependent on the database layer in the web layer, because all exceptions are thrown all the way upwards.

        The point of the PersistenceException is that it keeps the tiers independent from each other, except for the tier beneath it. The web layer doesn't care if it's an SQLException. It only needs to know something went wrong with the business logic, and more specifically with the persistence of business data objects.

        In any case, the 'business code' knows more of what to do than the layers above it. If it can't do anything meaningful with it, you can either log it or perform some other action. The question is: what would a layer above it (eg. the web layer) know more of what to do than the business layer?
        • Don't wrap exceptions!
          2008-07-31 00:20:09  kdgar [View]

          This is 100% correct.

          Just to add to this argument (which is already well put), say your presentation layer catches only SQLExceptions. You then switch to a different type of data source like a web service. Your presentation will then be catching things that never happen. In fact, all your code may completely fail to compile.

          Had you been catching a wrapped exception, like PersistenceException, you'd be in good shape.