S6667 versus S2139

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

Hi,

We are switching our logging system to Serilog but we have quite a few cases where we catch an exception log some details and then retrow it to be further handled upstream.

So this code throws S6667

      catch (Exception)
      {
        // S6667: Logging in a catch clause should pass the caught exception as a parameter.
        Log.Error("Some details about the method or parameters");
        throw;
      }

When we add the exception to the Log we get S2139

      catch (Exception ex)
      {
        // S2139: Exceptions should be either logged or rethrown but not both
        Log.Error(ex, "Some details about the method or parameters");
        throw;
      }

Any advice how to deal with this issue is welcome.

2

Hey @Valentijn

What version of SonarQube are you using?

3

SonarQube ID information
Version: 10.6.0.92116
Date: 2024-08-21

6

Hello @Valentijn,

Thank you for reporting this issue to us.

I believe there is room for improvement for S6667; it makes sense to log additional information before rethrowing without having to log the exception as well.

Here I am assuming that when you handle the exception, you do log it.

I have created a ticket in our backlog to tackle it in the future.

In the meantime, you can accept the issue when it pops up in your analysis in a similar scenario.

Have a nice day!

11

Another option is to create a new exception with the additional information.

      catch (Exception ex)
      {
        throw new HigherLevelException("Some details about the method or parameters", ex);
      }

This will make it easier to correlate the various logs and will not log duplicate backtraces, but the exceptions will get unwieldy if it is done too many times.

1 Like