Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does squid:S1166 not accept exception messages only when logging caught exceptions?

Quote from the description of the rule (SonarQube 4.5.5):

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

By providing the exception class to the logger a stack trace is written to the logs.

The problem in our code base is this: By following the Tell, don't ask principle, we use checked exceptions as part of the, what we consider, normal execution paths and we don't want them to result in unreasonably large log messages.

A few examples: Servers responding with error codes, database statement executions failing on optimistic locking (concurrent users)...

My suggestion: Split this case in two.

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

and

// Compliant - exception is lost (only message is preserved) but there is business logic handling the situation      

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

The rule squid:S00108 (code blocks must not be empty) would not catch the problem since there is a logging statement.

Is this not reasonable? Have I missed something of importance?

Note: I've rewritten the question to clarify my use case

like image 216
Alix Avatar asked Oct 09 '15 07:10

Alix


2 Answers

I understand the arguments for maintaining the stack trace and all that, but I think it's going to bloat your logs for a < ERROR level event. One solution is to log the message as a WARN and log the exception object as DEBUG or TRACE. That way a normal user log config would not be flooded with business as usual stack traces, but it would still be possible to get a stack trace if necessary.

like image 56
Novaterata Avatar answered Nov 12 '22 14:11

Novaterata


If it's causing hundreds of what you consider to be FP's then you should think about turning the rule off, or excluding it from your project files.

But to answer your question:

The point of exception logging is to leave enough information for investigators to figure out the cause of a problem.

If your messages are detailed, e.g.

The x in the y method broke because the frabjous was not day enough

then perhaps they fulfill that purpose. But what about a message like

Something went wrong

?

Further, you know exactly what each exception message means, but someday you'll presumably move on to bigger and better things. Will the next guy who supports the system have the same depth of knowledge? He may be grateful for the stacktraces and line numbers that tell him where to start looking...

But finally, I have to ask: why are you getting and logging so many exceptions that you flood the logger?

like image 2
G. Ann - SonarSource Team Avatar answered Nov 12 '22 13:11

G. Ann - SonarSource Team