Sonarcloud alerts "not enough arguments" when logging exceptions using SLF4J

Question

I manage an open source project in Java and have about 20 places in my code where I log exceptions using the following pattern (slf4j version 1.7.30)

private static final Logger logger = LoggerFactory.getLogger();

... 

try {
  interfaces = NetworkInterface.getNetworkInterfaces(); 
} catch (SocketException ex) {
  logger.error("Socket exception when retrieving interfaces: {}", ex);
}

or similarly

try {
  // stuff
} catch (IOException ioe) {
  logger.error("Server error: {}", ioe);
}

Starting today, the SonarCloud automated code quality review has begun flagging these with rule java:S2275 (Printf-style format strings should not lead to unexpected behavior at runtime) with the specific message "Not enough arguments."

EDIT: Of note, this appears to consistently happen when an Exception is the final argument. The following pattern does not flag:

try {
  // Server connection code
} catch (IOException e) {
  logger.error("Server Connection error: {}", e.getMessage());
}

A review of this other StackOverflow question indicates that perhaps an extra argument for the exception is optional and would result in different behavior, so I'm not clear how that would apply here and why it would suddenly change.

Is there something I can/should do to better translate these exceptions to log messages (e.g., use getMessage() on all of them instead of relying on the automated toString() parsing), or is this a false positive?

(Sonar's list of my 20 issues linked here.)

score 10 · Accepted Answer · answered Mar 12 '20 at 05:50

10

This is pure conjecture, but each of the issues points to a log line that can be generalized as:

LOG.something(format, custom_arguments, exception)

where format has {} appearing count(custom_arguments) + 1 (the 1 reserved for exception).

As you've seen the linked answer, exceptions get treated specially by slf4j, so it's possible that due to some reason SonarCloud is doing the same thing. Unfortunately there's no documentation.

The "fix" would be to remove the final {} intended for the exception, so e.g.

LOG.error("boom: {}", e);
LOG.error("boom2 {}: {}", something, e);

becomes

// exceptions handled in a special way
LOG.error("boom", e);
LOG.error("boom2 {}", something, e);

answered Mar 12 '20 at 05:50

Adam Kotwasinski

4,377
3
17
40

I thought of that, but then it looked like adding the exception as a `+1` argument actually threw the exception, which is not the behaviour I want. Also, in the first example, I extract a `String` from the exception... – Daniel Widdis Mar 12 '20 at 05:56
@DanielWiddis it's pretty weird, as I don't see `logger.error("Server Connection error: {}", e.getMessage());` in any of the 20 issues linked; absolutely surprised by the throwing behaviour though – Adam Kotwasinski Mar 12 '20 at 06:05
Hmm, you're right... I opened one of the flagged files and grabbed one as an example pattern, but it's fine. It's only the ones leading to the exceptions that are the issue. I'll edit my post. – Daniel Widdis Mar 12 '20 at 06:37
2

OK, did some testing and while it's not obvious/documented, the behavior is identical when the {} is removed as per your examples. Seems Sonar may have recently changed their checking to be more strict. – Daniel Widdis Mar 12 '20 at 06:59
OK, not "identical". The brackets are output unnecessarily... so in fact it is an error and your solution is spot on. – Daniel Widdis Mar 12 '20 at 07:10
I thought using the curly braces in the logger is a better practice? How come sonarqube detects that as code smell? – Henry Dec 08 '20 at 07:43
how to do LOGGER.error("This happened: {} while doing {}",event) in slf4j 1.6 – fiddle Aug 05 '21 at 14:39

Sonarcloud alerts "not enough arguments" when logging exceptions using SLF4J

1 Answers1