3

Can some one help me why SonarLint is showing this:

Either log this exception and handle it, or rethrow it with some contextual information.

for below piece of code.

public static <T> T getObjectFromJson(final Object jsonString, final Class<T> valueType) {
        T object = null;
        if (jsonString != null) {
            try {
                object = MAPPER.readValue(jsonString.toString(), valueType);
            } catch (IOException io) {
                log.error(ERROR_LOG_STR + " in method getObjectFromJson(). Exception Message={}, Exception Stack ={}",
                        io.getMessage(), io);
                throw new ServiceException(ErrorMessages.JSON_SERIALIZATION_ERROR, io.getCause());
            }
        }
        return object;
    }

enter image description here

hc_dev
  • 8,389
  • 1
  • 26
  • 38
Krish
  • 4,166
  • 11
  • 58
  • 110
  • 1
    Does this answer your question? [Sonar complaining about logging and rethrowing the exception](https://stackoverflow.com/questions/28122271/sonar-complaining-about-logging-and-rethrowing-the-exception) – eol Jan 06 '22 at 13:59
  • No my catch block diff compare to above solution – Krish Jan 06 '22 at 14:03
  • 1
    @eol Probably we should ask for the __RSPEC id__ shown when clicking on _Rule_ details. I suppose [RSPEC-1166](https://jira.sonarsource.com/browse/RSPEC-1166) ( answered by your marked duplicate) is __not the only__ non-compliant issue here: also and foremost [RSPEC-2139](https://rules.sonarsource.com/java/tag/error-handling/RSPEC-2139), see my answer ️ – hc_dev Jan 06 '22 at 20:10

1 Answers1

6

It's about the either ... or, see Sonar's Rule spec RSPEC-2139:

Exceptions should be either logged or rethrown but not both

To be compliant with the rule, decide for one:

  1. either log only:
            try {
                object = MAPPER.readValue(jsonString.toString(), valueType);
            } catch (IOException io) {
                log.error(ERROR_LOG_STR + " in method getObjectFromJson(). Exception Message={}, Exception Stack ={}",
                        io.getMessage(), io);
            }
  1. or throw only:
           try {
               object = MAPPER.readValue(jsonString.toString(), valueType);
           } catch (IOException io) {
               throw new ServiceException(ErrorMessages.JSON_SERIALIZATION_ERROR, io);  // would use the full exception, not only the wrapped .getCause()
           }

Bonus: How to achieve both and simplify

You can log additionally in a global or local error-handling interceptor like Spring's @ControllerAdvice annotated error-handler.

Most loggers allow to pass a Throwable when logging at error-level. For example using Slf4j: log.error(ERROR_LOG_STR + " in method getObjectFromJson(): " + io.getMessage(), io)

See also:

Related Rules

RSPEC-1166: Exception handlers should preserve the original exceptions

The spec feature was resolved 2018-11-02 with version 5.9, implementation for Java fixed 2019-02-15 with version 5.11.

⚠️ Partial fix only: The marked duplicate and obourgain's answer do not solve the two-fold case here completely:

  1. they fix RSPEC-1166
  2. but not RSPEC-2139

Similar questions coping with RSPEC-1166:

hc_dev
  • 8,389
  • 1
  • 26
  • 38