5

I am generating a Checkstyle report embedded in maven site and for one of the issues it is pointing out that Catching exception is not allowed. How can I fix this issue? I just don't simply want to remove the code and if I don't what other alternatives are there to fix this issue. 

public void contextInitialized(ServletContextEvent event) {
    super.contextInitialized(event);

    ServletContext context = event.getServletContext();
    setupContext(context);
    LoggingHandler logging = (LoggingHandler) AppContext.getBean( "loggingHandler" );

    try {
        loadClientUserData( context, logging );
        loadMBeans( context, logging );

    } catch (Exception e) {
        throw new RuntimeException( "Error during startup of service !!!" );
    }
}

I am still learning Java, so any sort of guidance would be appreciated.

Thanks

Michael
  • 41,989
  • 11
  • 82
  • 128
  • 1
    I basically means that you should be catching more specific exception, rather than catching generic `Exception`, which would catch everything extending from it – Abubakkar Jul 18 '17 at 15:02
  • 1
    Do not discard an exception you have caught—it tells you (and other developers) what went wrong and where! If you’re going to throw a different exception, include the caught exception as a [cause](http://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#getCause--): `throw new RuntimeException("Error during startup of service", e);` – VGR Jul 18 '17 at 16:06
  • Every other language scala, C#, kotlin got rid of checked exceptions specifically as new people(or new out of college) never know what to do here and should not have to. Only once you know you want to recover (which is extremely rare) do you add code. Generally, you want to fail the client and keep throwing up. In this case, catch whatever and I typically do throw SneakyThrow.sneak(e) which does the work you need in 99% of catch blocks. The other 1% are for batching, fail-open and other advanced use-cases. – Dean Hiller Oct 24 '22 at 08:43
  • also, fail-open is dangerous (as well as batching where you recover) as your standard API metrics don't cover it so you need to do extra work there. I wrote an article on some of this here when I was at Twitter -> https://blog.twitter.com/engineering/en_us/topics/insights/2019/gotta-catch--em-all – Dean Hiller Oct 24 '22 at 08:44

3 Answers3

8

What it's warning you about is that catching Exception is a bad idea. Exception is the most general type of exception you can catch. You're basically saying "whatever the problem, I can handle it." That's not true. There's any number of weird and wonderful problems that could happen: keyboard interrupts, disk space full, the list goes on.

You said loadClientUserData throws a ManagerException, so you should catch that specific exception and leave any others to propagate further up:

try {
     loadClientUserData( context, logging );
     loadMBeans( context, logging );
} catch (ManagerException e) {
     throw new RuntimeException( "Error during startup of service !!!" );
}

For more information, see these questions:

Michael
  • 41,989
  • 11
  • 82
  • 128
  • 1
    Michael. You are awesome. I just mentioned the same think above in my post. loadClientUserData throws ManagerException so if I catch it would that take care of this situation? –  Jul 18 '17 at 15:08
  • Yep, exactly. I'll find a link with more info on catching `Exception`s. – Michael Jul 18 '17 at 15:10
  • I totally agree with your answer that we must never catch the generic Exception especially it also catch RuntimeException. However I found few cases where there's no choice: e.g. a framework throwing Exception instead a specific one. Example: https://camel.apache.org/maven/current/camel-core/apidocs/org/apache/camel/CamelContext.html#resolvePropertyPlaceholders(java.lang.String) In that case you have to catch the Exception thrown by resolvePropertyPlaceholders and re-throw a RuntimeException containing the original one to prevent having "throws Exception" in your method signature. – рüффп May 31 '18 at 06:26
  • @рüффп this is useful when one wants to handle an exception in another thread – Line Oct 26 '18 at 11:45
2

You are catching a Generic Exception and doing nothing with that which seems to be the problem with Checkstyle. You can either rethrow the exception, handle it better, or ignore the Checkstyle validation for by creating a suppressions file.

Arpit
  • 323
  • 4
  • 13
0

You should catch the narrowest subclass of Exception you can, for reasons that are too long-winded to explain here, but briefly Exception includes unchecked exceptions.

If your methods are declared as throws Exception that too is a style problem.

Bohemian
  • 412,405
  • 93
  • 575
  • 722