SONARPY-4048 Rule S8572 "logging.exception()" should be used instead of "logging.error()" in exception handlers (#1082)

Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com>
Co-authored-by: Sebastian Zumbrunn <sebastian.zumbrunn@sonarsource.com>
GitOrigin-RevId: 4fc8aefd30f88d7c07e91a68255fecb9732a6ce4

Author: 3 people

python-checks/src/main/java/org/sonar/python/checks/LoggingExceptionCheck.java

@@ -0,0 +1,80 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.QualifiedExpression;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S8572")
+public class LoggingExceptionCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Use \"logging.exception()\" or explicitly pass \"exc_info=False\".";
+
+  private static final TypeMatcher LOGGING_ERROR_MATCHER = TypeMatchers.any(
+    TypeMatchers.isType("logging.error"),
+    TypeMatchers.isType("logging.Logger.error"),
+    TypeMatchers.isType("logging.LoggerAdapter.error")
+  );
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, LoggingExceptionCheck::checkCall);
+  }
+
+  private static void checkCall(SubscriptionContext ctx) {
+    CallExpression callExpr = (CallExpression) ctx.syntaxNode();
+    if (!(callExpr.callee() instanceof QualifiedExpression qualifiedCallee)) {
+      return;
+    }
+    if (!LOGGING_ERROR_MATCHER.isTrueFor(qualifiedCallee, ctx)) {
+      return;
+    }
+    if (!isDirectlyInsideExceptClause(callExpr)) {
+      return;
+    }
+    RegularArgument excInfoArg = TreeUtils.argumentByKeyword("exc_info", callExpr.arguments());
+    if (excInfoArg != null && !Expressions.isTruthy(excInfoArg.expression())) {
+      return;
+    }
+    Name errorName = qualifiedCallee.name();
+    ctx.addIssue(errorName, MESSAGE);
+  }
+
+  private static boolean isDirectlyInsideExceptClause(Tree tree) {
+    Tree parent = tree.parent();
+    while (parent != null) {
+      if (parent.is(Tree.Kind.FUNCDEF, Tree.Kind.LAMBDA)) {
+        return false;
+      }
+      if (parent.is(Tree.Kind.EXCEPT_CLAUSE, Tree.Kind.EXCEPT_GROUP_CLAUSE)) {
+        return true;
+      }
+      parent = parent.parent();
+    }
+    return false;
+  }
+}

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

@@ -312,6 +312,7 @@ public Stream<Class<?>> getChecks() {
       ListIterableFirstElementCheck.class,
       LocalVariableAndParameterNameConventionCheck.class,
       LoggersConfigurationCheck.class,
+      LoggingExceptionCheck.class,
       LongIntegerWithLowercaseSuffixUsageCheck.class,
       LoopExecutingAtMostOnceCheck.class,
       LoopOverDictKeyValuesCheck.class,

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8572.html

@@ -0,0 +1,76 @@
+<p>This rule raises an issue when <code>logging.error()</code> is used within an exception handler (<code>except</code> block) to log exception
+information.</p>
+<h2>Why is this an issue?</h2>
+<p>When handling exceptions in Python, it’s important to capture not just the error message, but also the full traceback. The traceback shows the
+sequence of function calls that led to the exception, which is essential for understanding what went wrong and where.</p>
+<p>The <code>logging</code> module provides two methods that might seem similar:</p>
+<ul>
+  <li><code>logging.error()</code>: Logs an error message at the ERROR level</li>
+  <li><code>logging.exception()</code>: Logs an error message at the ERROR level AND automatically includes the full exception traceback</li>
+</ul>
+<p>When you’re inside an exception handler (<code>except</code> block), you’re dealing with an exception that just occurred. In this context,
+<code>logging.exception()</code> is the appropriate choice because:</p>
+<ul>
+  <li>It automatically captures the traceback without requiring <code>exc_info=True</code></li>
+  <li>It clearly signals to other developers that you’re logging an exception</li>
+  <li>It follows Python’s principle of being explicit about intent</li>
+</ul>
+<p>Using <code>logging.error()</code> in an exception handler means you’re either:</p>
+<ul>
+  <li>Missing the traceback entirely (if you don’t pass <code>exc_info=True</code>), making debugging much harder</li>
+  <li>Writing more verbose code (if you do pass <code>exc_info=True</code>) when a simpler, more idiomatic option exists</li>
+</ul>
+<p>The <code>logging.exception()</code> method is specifically designed for this use case and is the established Python idiom for logging
+exceptions.</p>
+<p>If omitting the traceback is intentional, explicitly passing <code>exc_info=False</code> (or <code>exc_info=None</code>) makes that intent clear
+and is therefore not flagged.</p>
+<h3>What is the potential impact?</h3>
+<p>Using <code>logging.error()</code> instead of <code>logging.exception()</code> in exception handlers reduces code maintainability and makes
+debugging more difficult.</p>
+<p>When traceback information is missing from logs, developers must spend additional time reproducing issues to understand their root cause. In
+production environments, where issues may be intermittent or difficult to reproduce, missing traceback information can make it nearly impossible to
+diagnose problems effectively.</p>
+<p>Additionally, using <code>logging.error()</code> with <code>exc_info=True</code> instead of the more idiomatic <code>logging.exception()</code>
+makes the code less readable and may confuse developers who are unfamiliar with this pattern.</p>
+<h2>How to fix it</h2>
+<p>Replace <code>logging.error()</code> with <code>logging.exception()</code> when logging within an exception handler. If you were using
+<code>exc_info=True</code>, you can remove it since <code>logging.exception()</code> includes traceback information by default.</p>
+<p>If you intentionally do not want to log the traceback, explicitly pass <code>exc_info=False</code> to <code>logging.error()</code> to signal that
+intent.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+import logging
+
+try:
+    raise ValueError("Invalid value")
+except ValueError:
+    logging.error("Exception occurred", exc_info=True)  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+import logging
+
+try:
+    raise ValueError("Invalid value")
+except ValueError:
+    logging.exception("Exception occurred")
+</pre>
+<p>If logging the traceback is not desired, explicitly opt out with <code>exc_info=False</code>:</p>
+<pre>
+import logging
+
+try:
+    raise ValueError("Invalid value")
+except ValueError:
+    logging.error("Exception occurred", exc_info=False)
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python logging documentation - logging.exception() - <a href="https://docs.python.org/3/library/logging.html#logging.exception">Official Python
+  documentation for the logging.exception() method</a></li>
+  <li>Python logging HOWTO - <a href="https://docs.python.org/3/howto/logging.html">Python’s official guide to using the logging module
+  effectively</a></li>
+</ul>
+

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8572.json

@@ -0,0 +1,24 @@
+{
+  "title": "\"logging.exception()\" should be used instead of \"logging.error()\" in exception handlers",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5 min"
+  },
+  "tags": [
+    "logging",
+    "exception"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8572",
+  "sqKey": "S8572",
+  "scope": "Main",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "HIGH"
+    },
+    "attribute": "COMPLETE"
+  }
+}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

@@ -342,6 +342,7 @@
     "S8517",
     "S8519",
     "S8520",
-    "S8521"
+    "S8521",
+    "S8572"
   ]
 }

python-checks/src/test/java/org/sonar/python/checks/LoggingExceptionCheckTest.java

@@ -0,0 +1,30 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class LoggingExceptionCheckTest {
+
+  private final LoggingExceptionCheck check = new LoggingExceptionCheck();
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/loggingException.py", check);
+  }
+}

python-checks/src/test/resources/checks/loggingException.py

@@ -0,0 +1,162 @@
+import logging
+import sys
+
+logger = logging.getLogger(__name__)
+
+
+def noncompliant_bare_except():
+    try:
+        risky()
+    except:
+        logging.error("Something went wrong")  # Noncompliant {{Use "logging.exception()" or explicitly pass "exc_info=False".}}
+
+
+def noncompliant_typed_except():
+    try:
+        raise ValueError("bad value")
+    except ValueError:
+        logging.error("Value error occurred")  # Noncompliant
+
+
+def noncompliant_with_exc_info_true():
+    try:
+        raise RuntimeError("runtime error")
+    except RuntimeError:
+        logging.error("Runtime error occurred", exc_info=True)  # Noncompliant
+
+
+def compliant_with_exc_info_sys():
+    try:
+        risky()
+    except Exception:
+        logging.error("Exception occurred", exc_info=sys.exc_info())
+
+
+def compliant_with_exc_info_variable(some_var):
+    try:
+        risky()
+    except Exception:
+        logging.error("Exception occurred", exc_info=some_var)
+
+
+def compliant_with_exc_info_function_call():
+    try:
+        risky()
+    except Exception:
+        logging.error("Exception occurred", exc_info=should_log_traceback())
+
+
+def noncompliant_named_logger():
+    try:
+        risky()
+    except Exception:
+        logger.error("Named logger error")  # Noncompliant {{Use "logging.exception()" or explicitly pass "exc_info=False".}}
+
+
+def noncompliant_named_logger_exc_info():
+    try:
+        risky()
+    except Exception:
+        logger.error("Named logger error", exc_info=True)  # Noncompliant
+
+
+def noncompliant_nested_except():
+    try:
+        risky()
+    except ValueError:
+        try:
+            other()
+        except TypeError:
+            logging.error("Nested error")  # Noncompliant
+
+
+def noncompliant_except_group():
+    try:
+        risky()
+    except* ValueError:
+        logging.error("Grouped error")  # Noncompliant
+
+
+class MyService:
+    def noncompliant_in_method(self):
+        try:
+            risky()
+        except Exception:
+            logging.error("Error in method")  # Noncompliant
+
+
+def compliant_logging_exception():
+    try:
+        risky()
+    except Exception:
+        logging.exception("Exception occurred")
+
+
+def compliant_named_logger_exception():
+    try:
+        risky()
+    except Exception:
+        logger.exception("Named logger exception")
+
+
+def compliant_error_outside_except():
+    logging.error("Not inside an except block")
+    if some_condition:
+        logging.error("Error in conditional")
+
+
+def compliant_other_log_levels_in_except():
+    try:
+        risky()
+    except Exception:
+        logging.warning("Warning")
+        logging.debug("Debug info")
+        logging.critical("Critical failure")
+        logging.info("Informational message")
+
+
+def compliant_error_in_finally():
+    try:
+        risky()
+    except Exception:
+        logging.exception("Caught exception")
+    finally:
+        logging.error("Cleanup step")
+
+
+def compliant_error_in_nested_function():
+    try:
+        risky()
+    except Exception:
+        def handle():
+            logging.error("Inside nested function, not directly in except")
+        handle()
+
+
+def compliant_error_in_lambda():
+    try:
+        risky()
+    except Exception:
+        handler = lambda: logging.error("Inside lambda, not directly in except")
+        handler()
+
+
+def compliant_exc_info_false():
+    try:
+        risky()
+    except Exception:
+        logging.error("No traceback wanted", exc_info=False)
+
+
+def compliant_named_logger_exc_info_false():
+    try:
+        risky()
+    except Exception:
+        logger.error("No traceback wanted", exc_info=False)
+
+
+def compliant_exc_info_none():
+    try:
+        risky()
+    except Exception:
+        logging.error("No traceback wanted", exc_info=None)