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

Clean Code Report False-positive / False-negative...
,
1

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:

image
image1744×263 21.3 KB

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:

2

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

3

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

4

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

5

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

1 Like
8

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;
    }
  }
}