Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is critical SonarLint issue S1166 in my Java code a false positive or not?

Tags:

java

sonarlint

SonarLint 1.0.0 for Eclipse flags a critical issue in my code and I can't see why and how to fix it. It really looks like a false positive to me—or am I missing something?

import org.apache.log4j.Logger;

[...]

public final class Foo {

    private static final Logger logger = Logger.getLogger(Foo.class);

    [...]

    public static void foo() {

        MyCommand command = new MyCommand(foo, bar);
        try {
            commandService.executeCommand(command);
        } catch (CommandException e) {
            logger.error("My command execution failed", e);
        }
    }

    [...]

Here's an excerpt of the matching SonarLint rule description:

When handling a caught exception, the original exception's message and stack trace should be logged or passed forward.

Noncompliant Code Example

// Noncompliant - exception is lost
try { /* ... */ } catch (Exception e) { LOGGER.info("context"); }   

// Noncompliant - exception is lost (only message is preserved)       
try { /* ... */ } catch (Exception e) { LOGGER.info(e.getMessage()); }

// Noncompliant - exception is lost
try { /* ... */ } catch (Exception e) { throw new RuntimeException("context"); }

Compliant Solution

try { /* ... */ } catch (Exception e) { LOGGER.info(e); }   

try { /* ... */ } catch (Exception e) { throw new RuntimeException(e); }

try {   /* ... */ } catch (RuntimeException e) {
    doSomething();  
    throw e;
} catch (Exception e) {
    // Conversion into unchecked exception is also allowed
    throw new RuntimeException(e);
}

In my opinion, my code qualifies for the first variant of the given compliant solutions, but SonarLint does not accept it.

There was another discussion of Sonar rule S1166 a little while ago, but it's not really about the same issue I have.

Edit: In response to question below: I use log4j for logging. I expanded the code to reflect this.

like image 521
anothernode Avatar asked Dec 01 '15 10:12

anothernode


1 Answers

You are, in fact, logging the original exception's message and stack trace; this is an erroneous finding.

It may be that the rule doesn't have specific knowledge of Log4j, but lacking omniscience of all logging libraries, the fact that the exception is passed as a parameter could suffice.

like image 91
erickson Avatar answered Oct 28 '22 06:10

erickson