5

Possible duplicate of Sonar complaining about logging and rethrowing the exception.

This is my code in a class:

try
    {
        this.processDeepLinkData(data);
    }
    catch (final Exception e)
    {
        // Error while parsing data
        // Nothing we can do
        Logger.error(TAG, "Exception thrown on processDeepLinkData. Msg: " + e.getMessage());
    }

and my Logger class:

    import android.content.Context;
    import android.util.Log;
    import com.crashlytics.android.Crashlytics;

    public final class Logger
    {
        /**
         * Convenience method.
         *
         * @see Logger#log(String, String)
         */
        public static void error(final String tag, final String msg)
        {
            if (Logger.DEBUG)
            {
                Log.e(tag, "" + msg);
            }
            else
            {
                Logger.log(tag, "" + msg);
            }
        }

        private static void log(final String tag, final String msg)
        {
            Crashlytics.log(tag + ": " + msg);
        }
}

Sonar is pointing to catch (final Exception e) and says Either log or rethrow this exception. What do you think?

Community
  • 1
  • 1
Hesam
  • 52,260
  • 74
  • 224
  • 365

1 Answers1

11

If you look at the description of rule RSPEC-1166, especially the title:

Exception handlers should preserve the original exception

In your case you are only taking care of the message of the exception, thus not preserving the original exception (including its stacktrace). Thus your resulting logs will hide the root cause of the failure.

This rule detects that you are not using the caught exception as an entire object in the catch block.

Suitable fixes

This might not be suitable in your case:

  • either mark the rule as "won't fix"
  • or deactivate the rule in your quality profile
hc_dev
  • 8,389
  • 1
  • 26
  • 38
benzonico
  • 10,635
  • 5
  • 42
  • 50
  • Thanks for you info, however I am still confuse. If you look at `Noncompliant code example`, second sample, `try { /* ... */ } catch (Exception e) { LOGGER.info(e.getMessage()); }` looks like mine. Isn't it? – Hesam Aug 25 '15 at 07:31
  • 1
    yes, and this is considered as _NON_compliant so this is expected isn't it ? – benzonico Aug 25 '15 at 07:43
  • Yup, thanks. I got headache for spending time on this.... You are absolutely right. – Hesam Aug 25 '15 at 07:52
  • Excellent explanation with link and fixes which deserved a hopefully improving edit. I just [answered related question with RSPEC-2139](https://stackoverflow.com/a/70608728/5730279) maybe you like to "improve back" ️ – hc_dev Jan 06 '22 at 20:32