David has posted 2 posts at DZone. View Full User Profile

The Curse of the Swallowed Exception

10.01.2008
| 12495 views |
  • submit to reddit

Ever since I've been working in (and maintaining) Java code, I have found myself regularly trying to track down an issue where either there is no error message or the error message is mysterious and unhelpful like "read error at byte 0."

I think one of the most common and most expensive (in terms of maintenance) programming errors I see is the handling of exceptions.

I am sure everybody who is passionate about programming has their own principles, but this is my blog, so here are mine. You can give me yours in the comments.

David's Principles of Exception Handling and Reporting


  • Silence is deadly - This is the overriding principle. The stack trace is a beautiful thing to someone trying to fix an issue. An error message without a stack trace, or worse yet, silence, is the cloaked harbinger of hours, days, even weeks of hair-pulling debugging. Please, please, do not just quietly catch an exception and do nothing or just print out the message and not the stack trace.

  • When in doubt, throw it - If you call a method that throws an checked exception, generally you should rethrow the exception. The only time you shouldn't is if your method is responsible for communicating with the user.

  • Have one exception class per module - Here a module is a conceptual grouping of classes that together provide a service. I generally follow the principle that a module should have one and only one exception class, and all exceptions thrown by that module should be of that class. It's not helpful to creatively invent new exceptions for each condition. But, as usual, there are exceptions, particularly when you want to communicate a very particular situation. Bot those exceptions are rare, IMHO.

  • Don't break the chain - Because of the two principles above, you need to wrap exceptions you re-throw in the exception class for your module. Please don't just throw a new exception without wrapping the old one - vital information is lost that way, and you are likely cursing somebody (maybe even yourself) to hours or even days of head-scratching. Java has had exception chaining for years - learn it and use it.

  • The buck stops here - If you have nobody to re-throw to (generally because your method was invoked by a user action), sorry, but it is your responsibility to report the error the user. Who the user is and how you report it depends on your application. If you're a server application, you need a way to send the error message to the client. If you're a user application, you need to report the error through the UI. In either case, you need translate geek-speak into user-speak. Thus the next principle...

  • Be a butler - When you report an error to a user, don't be a gruff soup nazi. Be helpful. Describe the error, provide a likely cause, and offer possible actions the user can take. So instead of "I/O error: unable to read next 10 bytes from stream" say something like "We encountered an error while trying to talk to the server. It is possible the network connection was lost or the server was stopped. Please check to see if the network is working and the server is running and try again."

  • Log it - When you report a nice helpful error message to the user, log the full stack trace to the error log and not just the message. Log anything else you think is useful, the more information the better. This essential for the poor sod who has to try to track down the cause of the error. If you don't have an error log, get one.


Now what - You've just reported an error, what are you going to do next? The answer, of course, is "it depends." It's a discussion left for another day, but my general principle is, if data or long-term state is involved, it's time to fail quickly to avoid data corruption. If data is not involved - it's about a user session and user interaction, you generally report the error and move on - the user is responsible for any corrective action.

From http://davidvancouvering.blogspot.com/

Published at DZone with permission of its author, David Van Couvering.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Basil ____ replied on Thu, 2008/10/02 - 12:36am

What do you think about InterruptedException when you do Thread.sleep() or alike? Should it be logged before we handle it?

Thomas Mueller replied on Thu, 2008/10/02 - 2:30am

A very good article!

Have one exception class per module: I agree. Many API and libraries have too many exception types. Also, many libraries don't wrap exceptions (like IOExceptions). As you wrote, wrapping is important, because that's the place where you can add context information to the message ('Could not read the configuration file c:/data/config.properties' instead of 'java.io.EOFException').

You didn't write about checked versus unchecked exception. Newer languages like Scala don't have checked exceptions, and probably that's the right direction. What about only using unchecked exceptions? Of course you need to wrap checked exceptions in many places, but you need to wrap them anyway (see above).


Alessandro Puzielli replied on Thu, 2008/10/02 - 2:33am

Great post!

A question: in your opinion, why the developer not send any message to user?

Why often dont' they write the correct message in the logs but they prefer a general

logger.error(e);

instead a more specific

logger.error("Error in nameMethod in {I/O,SQL, BLAH BLAH}",e);

 ?

Jeroen Wenting replied on Thu, 2008/10/02 - 2:49am in response to: Basil ____

[quote=apple_fan]

What do you think about InterruptedException when you do Thread.sleep() or alike? Should it be logged before we handle it?

[/quote]

 Brian Goetz has something to say about that in his book "Java Concurrency in practice". He states that most often it's a symptom of a deeper problem inside the application and while ignoring it might not break the client session, it should be investigated.

Dave Las replied on Thu, 2008/10/02 - 6:51am

Thanks for the article, some great points here!

One thing I'd like to expand on a bit is wrapping exception. Lots of times you see:

...
} catch (Exception ex) {
log.error("When trying to do xyz", ex);
throw new SomeOtherException("describe what happened");
}

 OK that's not bad, but it could be better:

...
} catch (Exception ex) {
log.error("When trying to do xyz", ex);
SomeNewException up = new SomeNewException("describe what happened");
up.initCause(ex);
throw up;
}

 That's 2 extra lines in the catch block, but potentially hours of effort saved when the exception happens in production, nested 4 or 5 levels deep.

Thomas Mueller replied on Thu, 2008/10/02 - 7:07am in response to: Dave Las

> That's 2 extra lines

In many cases you can avoid them:

throw new SomeOtherException("describe what happened", ex);

Brian Sayatovic replied on Thu, 2008/10/02 - 8:35am in response to: Basil ____

Heinz Kabutz covered this in one of his fantastic Java Specialist newsletters: http://www.javaspecialists.co.za/archive/Issue146.html

Brian Sayatovic replied on Thu, 2008/10/02 - 8:47am

Be careful when chaining exceptions, though. Wne you start throwing across remote boundaries, you have to worry about some things. For example, are the chained exceptions' classes available in the calling system (avoid NoClassDefFoundError)?

Jason Kilgrow replied on Thu, 2008/10/02 - 8:52am

Great entry! Thanks for verbalizing some great advice! I will share this with the rest of my colleagues.

Jay Ceron replied on Thu, 2008/10/02 - 11:26am

I agree with most of these items except one.  Why would you need a module-specific exception?  What does that buy you?  Logging and the stack trace will tell you exactly where the exception came from.  Hence, you should have context exceptions, like a BusinessException or LoginException, etc..

Some schools of thought go further and state that you should not have user defined expections as standard Java exception can cover just about anything. I think that's a bit extreme as user-defined exception do have advantages.

David Van Couvering replied on Thu, 2008/10/02 - 11:36am in response to: Dave Las

That's why I said "Please don't just throw a new exception without wrapping the old one - vital information is lost that way".  By the way, if the constructor for the exception supports it, there is an easier way to do this:

catch (Exception ex) { 
   throw new SomeNewException("describe what happened here";, ex);
}

When I write exception classes, I always write the constructor:

public MyException extends Exception {
  public MyException(String message, Throwable cause) {
    super(message,cause);
  }
}

David Van Couvering replied on Thu, 2008/10/02 - 11:41am in response to: Jay Ceron

I suppose having an exception class per module is not necessary - I find it a nice way of signalling who owns the problem.  But you're right, it can result in a lot of unnecessary chaining.  What I don't like is bubbling up lower-level exception types like SQLException and IOException with no context.  So even without an exception class per module, chaining is very valuable as it adds context as you go up the levels of abstraction from lower-level IO to application domains.

Another reader had a good point about just not using checked exceptions.  I have been chewing on this for a while.  I think the reason checked exceptions are good are specifically because they allow you to handle *expected* errors and add context.   I think of unchecked exceptions as "whoops, I never thought that would happen."

 But I'm not sure.  I'm still thinking about this.  The fact that a langauge like Scala doesn't even bother with checked exceptions is interesting - I wonder how it works in production when you are trying to debug nasty issues and want to have as much context as possible.  How do the higher layer add context?

David Van Couvering replied on Thu, 2008/10/02 - 11:45am in response to: Brian Sayatovic

It sounds like you are assuming a remoting environment like RMI where there is some kind of marshalling of classes between client and server.  In many (most) environments, the client/server protocol is not a remoting protocol but something quite different like HTTP. 

 In that case you have to log the full stack trace *on the server* and then send an error that is valuable to the user back to the client.  I'd probably do this even if I were using something like RMI.  The user of the client software likely isn't interested in seeing the server stack trace anyway, although they might want to see the client stack trace (if the client is even in Java, which it often isn't).  IMHO another reason that the idea of "location transparency" is a bit of a myth when writing software.

 

Jay Ceron replied on Thu, 2008/10/02 - 12:03pm

"Another reader had a good point about just not using checked exceptions. I have been chewing on this for a while. I think the reason checked exceptions are good are specifically because they allow you to handle *expected* errors and add context. I think of unchecked exceptions as "whoops, I never thought that would happen."

Don't chew on it. Checked exception are a vital part of the language. They're meant, as you stated, to "inform" the caller that something needs to be done with a possible exception condition which is likely recoverable.  Unchecked exceptions are strictly for conditions from which your application cannot recover, such as a lost database connection or a downed messaging queue.

Robert Greathouse replied on Thu, 2008/10/02 - 4:04pm

"Don't chew on it. Checked exception are a vital part of the language. They're meant, as you stated, to "inform" the caller that something needs to be done with a possible exception condition which is likely recoverable."

How can a calling class tell the caller that something is likely recoverable? If the method throwing the exception assumes the exception is recoverable, why wouldn't the method just recover instead of throwing the exception?

Joe Farmer replied on Fri, 2008/10/03 - 12:32pm

Do not throw exceptions in finally block - it may discard the previous one.

If you can, put some input parameters in the exception you throw - it may help to identify the cause.

I disagree with "one exception per module" principle. I would replace it with "one parent exception per module" - all the exceptions thrown by the module should descend from the same parent.

 

phil swenson replied on Fri, 2008/10/03 - 4:22pm in response to: Jay Ceron

I knew this would end up being a checked vs unchecked exception argument :)

 

Count me in the "Checked Exceptions are retarded" camp:

 

see Java Exception Handling

Jeroen Wenting replied on Mon, 2008/10/06 - 12:04am in response to: Brian Sayatovic

[quote=Trinition]Be careful when chaining exceptions, though. Wne you start throwing across remote boundaries, you have to worry about some things. For example, are the chained exceptions' classes available in the calling system (avoid NoClassDefFoundError)?[/quote]

You mean InvocationTargetException, but yes, it can be a nasty one to track down.

Jeroen Wenting replied on Mon, 2008/10/06 - 12:06am in response to: phil swenson

[quote=philswenson]

I knew this would end up being a checked vs unchecked exception argument :)

Count me in the "Checked Exceptions are retarded" camp:

see Java Exception Handling

[/quote]

There is no argument. Checked exceptions are vital.

Jay Ceron replied on Mon, 2008/10/06 - 7:01am

[quote=greathr]<p>&quot;Don't chew on it. Checked exception are a vital part of the language.
They're meant, as you stated, to &quot;inform&quot; the caller that something
needs to be done with a possible exception condition which is likely
recoverable.&quot; </p><p>How can a calling class tell the caller that something is likely recoverable? If the method throwing the exception assumes the exception is recoverable, why wouldn't the method just recover instead of throwing the exception? </p>[/quote]

It’s about how the exception is handled, by whom, and where. The method throwing the exception only knows that an error condition has occurred, not how to handle it.  For example, suppose you have a method called getInvestorAccount(Double invId), which will throw a checked exception if it can’t find an investor account in a data source that it knows about.  However, the caller (another class or another system altogether) could catch that exception and check an alternate data source for that account, or simply default the account or pass on the exception so something else can handle it.

 

That’s why the exception handling mechanism exists in the first place. Otherwise, you end up with a quagmire of if/else code.

 

Liezel Jane Jandayan replied on Thu, 2011/08/11 - 5:48am

A broken application that tries to limp onwards in an inconsistent state is a danger to itself, its data, and ultimately its users.

 Senior Healthcare Consultants

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.