S2139 should not be triggered for this specific case

Sonar Qube version

  • Version 6.7.6 (build 38781)
  • Language Java

This rule, squid:S2139, should not be triggered for this specific case:

} catch (AnyException ex) {
    LOGGER.warn("I want to log the stack somewhere", ex);
    // Third party library exception which does not support instantiation with the Throwable cause
    throw new ThirdPartyLibraryError("code", "message");

In this example I want to log the root exception, and throw another third party exception which does not provide the ability to instantiate it with the Throwable cause. (there is only one constructor with error code).

Note that this ThirdPartyLibraryError exception, depending on its content, has impacts on the way this third party solution behaves (In the real life, I’m throwing a BpmnError which is used by a BPMN business process).

IMO, this rule should only be triggered when both logging and throwing another exception instantiated with the cause

1 Like

Hello @remibantos,

this seems to be a quite peculiar case to me to justify the change in the rule. How often do you encounter this kind of issue? Maybe you could instead pass the stacktrace as part of the message?

Hi @saberduck ,

For now we have added a temporary @SupressWarnings as a workaround in many classes where this pattern is applied.
Since last update of Sonar Qube, we get a lot of major false positive bugs for this reason.

Each time one of the class (many) where we use such pattern gets analyzed.

I would not recommend this workaround

Hi @saberduck, should I report a bug for that?
For now I have excluded all these false positives explicitly in the code.

hello @remibantos ,

just to better understand your context, do you consider the rule to be otherwise valuable?
I have created this ticket to add exception to the rule implementation https://jira.sonarsource.com/browse/SONARJAVA-3073

Note that, fix version is tentative.

Thanks for the ticket

I think this rule is usefull, as it is generally a bad idea to log many times the same exception…
It just needs to be adjusted in order not to catch this specific case as you explained.

I see that as false-positive, too.

And even the description of that rule confirms that: “Exceptions should be either logged or rethrown but not both”

In @remibantos case the exception is logged but it is not rethrown. Instead a new exception without the cause is thrown. So there will be no stacktrace-cascading and the rule does not apply.

Even more: “In applications where the accepted practice is to log an Exception and then rethrow it, you end up with miles-long logs that contain multiple instances of the same exception.” - that is not the case.

1 Like