[`flake8-logging`] Allow closures in except handlers for LOG004 by anishgirianish · Pull Request #24464 · astral-sh/ruff

anishgirianish · 2026-04-07T07:36:33Z

Summary

outside_handlers walks AST ancestors looking for a Try statement whose handler range contains the call offset. It used to bail out at FunctionDef nodes to avoid crossing scope boundaries, which meant any logging.exception() inside a closure defined in an except block got flagged.

The range check applies to where the function is defined, not where it's called (which Ruff can't track). This means functions defined inside an except block but called outside of it won't be flagged. This tradeoff fixes the more common false positive at the cost of a less common false negative.

Dropped the FunctionDef break.

Test Plan

Updated fixture to move function-inside-except to no-errors section
Added closure-called-within-except test case
local ecosystem runs showed expected results

astral-sh-bot · 2026-04-07T07:49:43Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -1 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)

zulip/zulip (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL

- zerver/worker/email_senders_base.py:38:17: LOG004 `.exception()` call outside exception handlers

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
LOG004	1	0	1	0	0

charliermarsh · 2026-04-09T01:44:27Z

This would now lead to the following false negative:

import logging

def make_later():
    try:
        raise ValueError()
    except Exception:
        def later():
            logging.exception("x")                     # should be LOG004
            logging.error("x", exc_info=True)   # should be LOG014
        return later

make_later()()

...the range check already handles scoping correctly.

The range check only applies to where the function was defined, not where it's called, which we can't determine / track in Ruff. So if we want to fix this false positive, we'll have to introduce some false negatives. I suppose it might be worth it though.

anishgirianish · 2026-04-09T02:15:59Z

This would now lead to the following false negative:
import logging

def make_later():
    try:
        raise ValueError()
    except Exception:
        def later():
            logging.exception("x")                     # should be LOG004
            logging.error("x", exc_info=True)   # should be LOG014
        return later

make_later()()
...the range check already handles scoping correctly.

The range check only applies to where the function was defined, not where it's called, which we can't determine / track in Ruff. So if we want to fix this false positive, we'll have to introduce some false negatives. I suppose it might be worth it though.

Thank you so much for the detailed review, @charliermarsh I've updated the PR description to reflect the limitation around definition site vs call site.

Happy to add your example as a documented known false negative in the test fixture. Would you recommend going with this tradeoff, or would you prefer a different approach?

Thank you

charliermarsh · 2026-04-09T02:25:32Z

I think we should document this limitation, but otherwise I'm comfortable with it. I'll add a note to the docs before merging.

## Summary Fixes #18646. `outside_handlers` walks AST ancestors looking for a `Try` statement whose handler range contains the call offset. It used to bail out at `FunctionDef` nodes to avoid crossing scope boundaries, which meant any `logging.exception()` inside a closure defined in an except block got flagged. The range check applies to where the function is defined, not where it's called (which Ruff can't track). This means functions defined inside an except block but called outside of it won't be flagged. This tradeoff fixes the more common false positive at the cost of a less common false negative. Dropped the `FunctionDef` break. ## Test Plan - Updated fixture to move function-inside-except to no-errors section - Added closure-called-within-except test case - local ecosystem runs showed expected results --------- Co-authored-by: Charlie Marsh <[email protected]>

@charliermarsh

[flake8-logging] Allow closures in except handlers for LOG004

59f0674

astral-sh-bot Bot assigned ntBre Apr 7, 2026

astral-sh-bot Bot requested a review from ntBre April 7, 2026 07:36

[flake8-logging] add bit of a context

f31749c

updated snapshot

e0abf20

MichaReiser assigned dylwil3 and unassigned ntBre Apr 7, 2026

dylwil3 requested review from dylwil3 and removed request for ntBre April 7, 2026 13:00

ntBre added bug Something isn't working rule Implementing or modifying a lint rule preview Related to preview mode features labels Apr 8, 2026

charliermarsh approved these changes Apr 9, 2026

View reviewed changes

charliermarsh added 2 commits April 8, 2026 22:32

Add known limitations

8805c1a

Modify comment

10f56a2

charliermarsh enabled auto-merge (squash) April 9, 2026 02:35

charliermarsh changed the title ~~[flake8-logging] Allow closures in except handlers for LOG004~~ [flake8-logging] Allow closures in except handlers for LOG004 Apr 9, 2026

charliermarsh disabled auto-merge April 9, 2026 02:35

charliermarsh enabled auto-merge (squash) April 9, 2026 02:35

charliermarsh merged commit 60d4694 into astral-sh:main Apr 9, 2026
43 checks passed

BrewTestBot mentioned this pull request Apr 9, 2026

ruff 0.15.10 Homebrew/homebrew-core#276824

Merged

This was referenced Apr 27, 2026

Always include panic payload in panic diagnostic message #24873

Merged

ASCII identifier fast path #24876

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`flake8-logging`] Allow closures in except handlers for LOG004#24464

[`flake8-logging`] Allow closures in except handlers for LOG004#24464
charliermarsh merged 5 commits into
astral-sh:mainfrom
anishgirianish:fix/log004-closure-in-except-handler

anishgirianish commented Apr 7, 2026 •

edited

Loading

Uh oh!

astral-sh-bot Bot commented Apr 7, 2026 •

edited

Loading

Uh oh!

charliermarsh commented Apr 9, 2026

Uh oh!

anishgirianish commented Apr 9, 2026 •

edited

Loading

Uh oh!

charliermarsh commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

anishgirianish commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

astral-sh-bot Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

charliermarsh commented Apr 9, 2026

Uh oh!

anishgirianish commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charliermarsh commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anishgirianish commented Apr 7, 2026 •

edited

Loading

astral-sh-bot Bot commented Apr 7, 2026 •

edited

Loading

`ruff-ecosystem` results

anishgirianish commented Apr 9, 2026 •

edited

Loading