LOG004 false positive

[sk-]

Summary

The preview rule LOG004 (log-exception-outside-except-handler) is producing a false positive when explicitly passing an exc_info argument.

The code below (playground link) reproduces the issue:

import logging

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)
logger.exception("Test LOG004", exc_info=IndexError("test message"))

producing the output

test_logging.py:5:1: LOG004 `.exception()` call outside exception handlers
  |
3 | logging.basicConfig(level=logging.INFO)
4 | logger = logging.getLogger(__name__)
5 | logger.exception("Test LOG004", exc_info=IndexError("test message"))
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG004
  |
  = help: Replace with `.error()`

Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).

However, this code is perfectly valid and it outputs

ERROR:__main__:Test LOG004
IndexError: test message

Note that the justification for the rule states:

Calling .exception() outside of an exception handler attaches None as exception information, leading to confusing messages:

However in this case, the exception information is indeed attached because it was passed explicitly.

I could see why someone may prefer to just use error and avoid using exception in this case, but that IMO would be a stylistical rule similar to LOG007. Such rule could get some good reception as others have manifested that they never use logger.exception.

Version

0.11.9

[ntBre]

I don't have a strong opinion either way here, but I think there's a good argument that this is not a false positive. The logging.exception docs say:

This function should only be called from an exception handler.

without any qualification, which seems to be what this lint rule enforces. I think this is also in line with the upstream linter, although I haven't tested to be sure.

If we decide that this is a true positive, we could at least update the rule documentation to align more closely with the Python docs.

[sk-]

I agree with you assessment and sorry for missing the logging.exception docs, I was looking at the docstrings, which are different to what is published on the docs.

It'd be great if the rule's documentation could be changed so that it is clear that the rules enforces what the logging docs suggest.

In any case, jut like others we opted to get rid of all calls to .exception in favor of calls to .error.

[spaceone]

I think the rule docs should suggest to use error method, if exception(exc_info=…) is used.

[ntBre]

@spaceone I don't think I'm following your suggestion. Both the error message shown in this issue and the rule docs already recommend using logging.error.

[spaceone]

Well yes, https://docs.astral.sh/ruff/rules/log-exception-outside-except-handler/ says to replace exception() to error().
I was only irritated first because:
log.exception("foo", exc_info=exc_info) is valid in a non-exception context. but should of course also be migrated to just use error().