Any Work around for not using a broader Exception, the "System.Exception"?

Question

I am a fresher, and after successfully completing my project I scanned it through fortify. it gave me a list of issues, out of which just one categories remain. (Fortify a code quality scanning tool)

it says, "Not to use Broader Exception i.e. System.Exception" to catch exceptions unless in some conditions.

But I have a few methods that have 25-30 lines of code with different types of operations, in such case how to figure out which all specific exceptions to catch.

Should we also throw all these exceptions to be caught at a higher level catch, as i read "throw first catch late".

Please suggest me a clean way to do this.

e.g.

public SomeMethod(arg) throws IOException,SQLException,beanNotFoundException { 
    try {
        someCode
    } catch (IOException|SQLException|beanNotFoundException ex) {
        logger.log(ex);
        throw ex;
    }
}

But also if i don't use Exception Class altogether till the end, i also have to make sure that I am not missing any exception to handle.

Is there a better approach.

Answer 1

In general, you only need to worry about handling/throwing checked exceptions. A clean way to write the method depends on the code base and structure of your application. Here are some alternatives:

If you want the client code (code calling someMethod) to handle the exception, you can do this:

public void someMethod(String arg) throws IOException, SQLException,
                                   BeanNotFoundException {
   doSomething(arg);
}

If you can handle the exception locally in someMethod, do so, but do not rethrow the same exception up in the call stack:

public void someMethod(String arg) {
    try {
        doSomething(arg);
    } catch (IOException | SQLException | BeanNotFoundException ex) {
        logger.log(ex);
        handleException(ex);
    }
}

If the exception is something you cannot recover from, and you don't want to enforce client code to handle the exception, you can wrap it as a RuntimeException. This will potentially terminate the program (which is usually what you want when a fatal error occurs):

 public void someMethod(String arg) {
    try {
        doSomething(arg);
    } catch (IOException | SQLException | BeanNotFoundException ex) {
        logger.log(ex);
        throw new RuntimeException(ex);
    }
}

For a good explanation about checked vs. unchecked exceptions, you can refer to Java: checked vs unchecked exception explanation.

Answer 2

Static Analysis

First and foremost, let me start with a little fallacy that most people fall subject to (and I see a lot in the corporate world): Static analysis tools are not infallible. They make mistakes. There are some warning classes that, with all the computing power known to man and with the remaining lifespan of the universe, the tool may not be able to exhaustively analyze a piece of code related to a particular warning class. Moreover, there are some warning classes that can complete before the end of time, but would be unreasonable to expect you to wait 7 days for an answer for one warning on one section of code for one execution path. That is, static analysis tools have to make guesses sometimes. The better tools are better at making guesses (you pay for what you get), but in the end, they are all capable of guessing right (true positive/negative) or wrong (false positive/negative).

Additionally, some warning classes may be applicable and some may not be. Some other warning classes may be important to you and some may not be. For example, PMD has a warning class to the effect of "Unused Import". An unused import has no effect on runtime, and while it may not be ideal, you can leave it in your code and it will not affect you unless your codebase is large and requires time to compile, and you have lots of these (unused imports make it longer to build your project). I particularly mention static analysis, because it sounds like you ran Fortify on your codebase and fixed anything and everything without questioning the validity. Most of the time it will probably be right, but just don't take it as fact because it told you so. You need to question whether these tools are always right.

Your example

So your example is not a false positive. If you do

throw new Exception("");

that's generally not what you want to do. It's fine for debugging and just quickly throwing code together. But, as your codebase gets larger, handling Exceptions will get more difficult.

This leads me right into my next point. You make the statement

But I have a few methods that have 25-30 lines of code with different types of operations, in such case how to figure out which all specific exceptions to catch.

...

But also if i don't use Exception Class altogether till the end, i also have to make sure that I am not missing any exception to handle.

Which seems to indicate to me that you either have something to the effect of

try{
  //25-30 lines of code
} catch (Exception ex) { /*ex.printStackTrace(); Logger.getLogger(...).log(...); etc etc...whatever it is you do here, or maybe nothing at all*/

This is pretty bad - in most cases.

To get back to answering your question before I explain why, yes, you should absolutely catch each individual exception. The reason for this, and why your catch all is bad, is because if there is something specific that goes wrong, you can't have a specific type of error handling.

For example, take a function Foo#bar(java.lang.String), which throws an IOException when a disk access fails because you tried to write to a bad file, a BooException if you pass in a String without a special character in it, or a BazException for some other arbitrary reason. Now let's go back to your example from above: what if I wanted to realize that the file I was writing to was bad, and maybe prompt the user for some clarification before I moved on? What if I knew that if a BooException was thrown, that the user should have never been here in the first place, and I need to redirect them to some location? What if I knew that when a BazException that the system was out of sync with some other system and that this is a fatal problem, and now I need to do resource cleanup before I forcefully crash the JVM?

The above reasons are why you should do individual try/catch blocks for each statement and each exception.

That being said, there are reasons to not do this. Imagine, with the above example, that all you want to do for a IOException, BooException and BazException (and also any runtime exceptions, i.e. NullPointerException) that I just want to log the exception and move on. In this case, I would say it's OK to do a try/catch around a block of code - so long as it's just around the code that this applies to.

EDIT: You made a point about missing an exception, but putting a response to that in the comments seemed unruly to look at. In any case, let me start off by saying if it is not a runtime exception, then you will not even be able to compile without handling it. Catching Exception catches everything, runtime or not, which is how you compile now. From my example above, if there are runtime exceptions you are worried about missing, instead of just starting off with catching Exception:

Foo myFooInstance = new Foo();
String someValue = "value";
try {
  myFooInstance.bar(someValue);
} catch (IOException ioe) {
  /*handle file access problem*/
} catch (BooException boe) {
  /*handle user in wrong spot*/
} catch (BazException bze) {
  /*handle out-of-sync fatal error*/
} catch (Exception ex) {
  LogRecord lr = new LogRecord(Level.SEVERE, "Unhandled exception!! returning immediately!!");
  lr.setThrown(ex);
  lr.setParameters(new Object[]{someValue});
  Logger.getLogger(MyClass.class.getName()).log(lr);
  return;
}

Here you end with your catch-all rather than starting with it. Think of it as your last-ditch effort to try to handle everything. It may not mean the exception is fatal to your program, but you probably shouldn't continue either (hence why my example uses return;)

Another small thing to consider, is that it becomes exponentially more difficult for the JVM to catch the exception properly the larger the try block gets (if the exception is never thrown, there is no overhead). This is more trivial to powerful machines, but something to keep in mind. With that in mind, I also don't have any sources for performance about exceptions being thrown in large try blocks, so take that with a grain of salt unless somebody finds something and mentions it.

Answer 3

My advice would be to ignore what Fortify is saying. Or better still, figure out how to selectively suppress these warnings.

The alternative to your current version that Fortify is suggesting is objectively bad in most contexts. Your current version is not bad, only (a bit) verbose.

Another alternative would be to wrap the exceptions in a custom exception or exceptions that have a common custom checked exception as the base-class (NOT Exception or Throwable or RuntimeException). That could make code like you example neater, but it means that you need to add code (somewhere) to do the wrapping and (maybe) unwrapping. In short, that's no idea too.

OK so what is the problem with catching Exception? Well basically, it catches everything ... including any unexpected uncheck exceptions that are evidence of bugs. If you then declare the method as throws Exception it only gets worse.

I have seen an example (no names) where someone did this in a large Java product. The result is that the product's Java APIs are horrible to program against. Every non-trivial API method is declared as throws Exception, so someone programming against the APIs has no idea what to expect. And the code-base is proprietary and obfuscated. (And the worst thing is that the lead developer still thinks it was a good idea!)

What about wrapping the exceptions in RuntimeException? That's not quite as bad¹ as Exception but the problem is that the programmer still can't tell what exceptions to expect.

^{1 - It is not quite as bad in the sense that throws Exception is effectively "noise" once you have gone done the path that anything can throw anything.}

Answer 4

The general rule of thumb is that checked exceptions are thrown for errors that are external or unpredictable, and which the caller can reasonably be expected to do something about, even if that only means displaying a message to the user. Whether you catch them or pass them up the call stack depends on where it makes the most sense to handle them.

Unchecked exceptions are thrown for unsatisfied preconditions or invalid arguments. In other words, the programmer screwed up. That's why you're not forced to catch them; they're typically things that "shouldn't happen." You generally don't need to worry about them if you're using the API correctly. The only time you should catch unchecked exceptions is when an API doesn't follow these conventions and throws unchecked exceptions for external or unpredictable reasons.