9

Is it a good practice to rethrow the same exception to provide more specific information? For example:

var sitemap = "a string containing an XML document";
try {
   // throw InvalidXmlException if the document is not well formed
   parse(sitemap);
} catch (InvalidXmlException e) {
   throw new InvalidXmlException("The sitemap is not well formed: " + e.getMessage());
}
Bergi
  • 1,331
Cequiel
  • 213

5 Answers5

23

Providing more context information is always good. But there is an issue in the code that I wanted to point out (perhaps an issue with the API - the underlying exception class). You are not passing the root cause. Don't append e.getMessage() it may be redundant. Instead set the root cause like this

catch(SameExceptionClass e) {
    throw new SameExceptionClass("Some more Details", e);
}

When this exception gets printed it will contain your message from e and the additional details you just added. If your exception class doesn't accept the root cause consider adding a constructor that can accept it. Not providing root cause leads to problems related to "swallowed" exceptions, where you will not know the real point where the original exception occured.

But yes do this ONLY if you feel you will add more value or more context info in your new exception.

Y123
  • 494
  • 3
    +1, adding outer context information without removing already provided information from the inner levels are exactly the key points to get this right! – Doc Brown Apr 01 '15 at 20:57
  • 1
    Thanks @Cequiel, please consider "accepting" (green check mark) an answer that answer your question. – Y123 Apr 01 '15 at 23:51
  • I know, but you are a Master :) Sorry about that. – Cequiel Apr 01 '15 at 23:54
  • +1 Using an Inner Exception is a great way to throw a new exception while preserving the stack trace of the original exception. – Derek W Apr 02 '15 at 02:09
3

Generally the rule for exceptions is, don't create a new exception type, and don't re-throw, unless you are sure that the caller is going to be able to do something different with the new information you provide (i.e. you are providing more information, such as a better error message, or you provide information to recover).

Frank Hileman
  • 3,932
2

There's something else here that no one has mentioned yet: Abstraction. Perhaps more importantly, leaky abstractions?

Does it make sense to let your DataService throw an IO or Database exception? I'd say no, because it "leaks" the underlying implementation to the client code. Your FooController shouldn't care whether DataService is talking to a database or file system. All it needs to know is that the data it asked for couldn't be retrieved. So, yes. Re-throw the exception and add the additional information (properly preserving the stack trace) at the proper level of abstraction. In the example I provided, at the level of the DataService.

RubberDuck
  • 8,961
  • 5
  • 35
  • 44
1

Depends

If your method is doing something that will be visible to the end user, security becomes a concern. You should never throw low-level exceptions back to the user on distributed systems.

Your catch() statement should encapsulate the low-level error and only write it to a (more) secure logging mechanism. One that is only easily accessible by support personnel, not to the general public.

Then, create and throw a new, generic, exception type that tells the user enough that they can intelligently report it, but doesn't give them the keys to the kingdom.

For all other exceptions, it's generally best to catch the exception at the point where it can actually be handled.

For example: If you have a business class that calls several data classes, the business class may be best suited to actually handle any data exceptions (meaning give them context in the grand scheme of things). So, don't catch in the data classes at all.

Your mileage may vary.

Worrystone
  • 39
0

Often the stack trace in an exception is of no value and in such cases no harm is done in throwing a new exception.

If you don't know what happened you of course retain all the data you can for debugging purposes. However, in well-written code there will be few such exceptions, most exceptions will be because the program is a victim of outside forces. If you know what the outside force is that caused the problem the most informative thing to do is tell the user that.

If you know it failed to find the job you just told it to work on why do you care what it was executing? The important information is that the job wasn't found. (I have in mind a case where each job has a directory with some megabytes of data that's not in the database.)

Loren Pechtel
  • 3,391