Either log or rethrow this exception code smell (but it's already logged)

I’m in the process of upgrading our community edition (version 7.9) to the developer edition (8.9) and I’m doing a comparison of our two projects to see if they are the same, but I’m seeing some regressions. One I don’t understand at all:

As you can see, the exception is logged but Sonarqube doesn’t see it… I could of course review each of them and put them as “False positive” but that’s going to be long and bothersome. I would expect an update to not produce more problems :smiley:

Hi,

when logging only the exception message, you’re losing the context.
You need to log the exception itself instead, so use ex not ex.getMessage().

Gilbert

getLogger().error(message, ex); ==> I’m logging both the message and the exception.

Hi @Lusheez

As you’ve seen, a lot can change from LTS to LTS. Once you get to 9.9, please let us know if this is still replicable.

 
Ann

Hello @ganncamp,
We updated to 9.9 and still have the issue.

1 Like

Hey @Lusheez ,

Unfortunately, I’m not able to reproduce the issue locally with what you are providing. I’m afraid there is not enough Information.

  1. I believe that the rule you are talking about is java:S2139. Correct?
  2. Can you provide a self-contained and minimal reproducer that reproduces the FP?
  3. On your screenshot, I have to “guess” what are some of the classes. Can you answer to the following questions?
    • What’s the fully qualified name of the class being returned by the call getLogger()?
      • Is it a custom logger class? If yes, does it depends on a known logging API?
    • What’s the signature of the Promise.rejected(...) method call?
      • From what API is it coming from? Is it an internal class?

On my side, I tried the following to get closer to your code:

public class A {

  Object bar(Promise foo) throws Exception {
    try {
      foo.doSomething();
    // ------------------------------
    } catch (Exception ex) {
      String message = String.format("Foo %s", ex.getMessage());
      getLogger().error(message, ex);
      return Promise.rejected(message);
    }
    // ------------------------------
    return new Object();
  }

  /**
   * Testing slf4j since signature seems similar
   */
  private org.slf4j.Logger getLogger() {
    return org.slf4j.LoggerFactory.getLogger(A.class);
  }

  /**
   * Mocked unknown classes...
   */
  @SuppressWarnings({
    // suppress issues unrelated to reproducer
    "java:S112", /* Generic exceptions should never be thrown */
    "java:S1130" /* Exceptions in "throws" clauses should not be superfluous */
  })
  interface Promise {
    void doSomething() throws java.sql.SQLException;

    static Object rejected(String s) throws Exception {
      return s;
    }
  }
}