[Jboss-dev-forums] [Design of Mail Services] - Exception Handling

[mikezzz <nu...@jb...>]

Hi,

I think we need a consistant approach to exception handling.  We have a mix of no exceptions (swallowing and returning false/null), checked and unchecked exceptions.  I have been think about this for a while, but today an article cropped up on JavaWorld that sums up my preferred approach.

http://www.javaworld.com/javaworld/jw-07-2005/jw-0711-exception.html

I think the 3 guidelines specfied at the beginning of the article would work quite well.  How should these be applied to JBoss-Mail?  All module interfaces should be configured to only throw a MailException (or a sub-class).  This would only apply to methods exposed to other modules.  Internal to a module any type of exception may be declared.  MailExceptions should be thrown in the case of an application error.  E.g.  User does not exist.  A rule of thumb would be that you should throw a MailException in the case of an application check failing e.g.

User user = Hibernate.getUser(); // Some system find method
  | if (user != null) {
  |     return getFolderForUser(user);
  | }
  | else {
  |     throw new UserNotFoundException("......");  // extends MailException
  | }

Calling code may attempt handle the exception if appropriate.

Folder f;
  | try {
  |     f = getFolder();
  | }
  | catch (UserNotFoundException e) {
  |     f = createFolderForUser();
  | }

If the calling decides not to handle the exception it should be allowed to cascade up.

try {
  |     f = getFolder();
  | } // no catch
  | finally {
  |    // Do some clean up if necessary.
  | }

There is a danger of this becoming programming by execption which is not good.  However the cases where exception are simply allowed to cascade up the stack will outweigh the ones where it is caught and handled, i.e. the exceptional cases (pun intended).

If an exception is thrown which can not be handled locally and it is not possible rethrow the same exception then a wrapped MailSystemException (sub-class of RuntimeException) should be thrown.

try {
  |     doStuff();
  | }
  | catch (SQLException e) {
  |     throw new MailSystemException("...", e);
  | }

Exceptions should be handled at the entry point into the mail server.  This includes the protocol implementations and delivery MDBs.  Each entry point will contain a static table that maps internal mail exceptions to error responses appropriate to the particular entry (as defined by POP, IMAP etc.)


try {
  |     // do some stuff.
  | }
  | catch (MailException e) {
  |     Response r = processException(e);
  | }
  | catch (RuntimeException e) {
  |     Response r = processUnknownException(e);
  | }

The motivation for this is there are a number of interfaces that don't throw exceptions on error, which causes problems when introducing AOP transactions that use exceptions to trigger a rollback when required.  Also it would be cleaner to have a common approach to handling errors.

I am interested in hearing peoples' thoughts on the subject.

Mike.

View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=3884433#3884433

Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=3884433