Skip to content

cleanup: 30 except Exception: handlers have no comment, against the documented rule #46

Description

@LeyckerS

Context

CONTRIBUTING.md states the rule:

No blanket except: — name the exception, or except Exception: with a comment when the swallow is deliberate.

The rule is right. 30 handlers in the codebase do not follow it, swallowing everything with no comment saying why.

These numbers were recounted against main on 1 August 2026, after #54 annotated its five. An earlier version of this issue said 25, which was wrong in both directions — moon_extract.py alone has 23, and three of the files #54 touched still have unannotated handlers elsewhere.

File Count Lines
moon_extract.py 23 101, 188, 378, 391, 471, 537, 556, 653, 713, 735, 781, 796, 802, 806, 943, 1025, 1077, 1088, 1095, 1098, 1179, 1219, 1249
moon_engine.py 3 265, 371, 505
moon_bridge.py 2 170, 297
moon_cli.py 1 207
moon_download.py 1 564

Most are almost certainly deliberate — this is a codebase that talks to a browser over CDP and to hosts that change behaviour without notice, so defensive swallowing is often the correct call. That is exactly why the rule asks for a comment: from the outside, a deliberate swallow and a forgotten one look identical, and the next person cannot tell which is which.

The moon_extract.py ones matter most. Several of them currently mean a broken selector is indistinguishable from a dead link, which is the class of bug that costs someone an afternoon of debugging the wrong thing.

Take a slice — four people can work in parallel

Claim one slice by commenting which you are taking. Do not take two.

Slice Scope Handlers
A moon_extract.py lines 101–556 7
B moon_extract.py lines 653–943 8
C moon_extract.py lines 1025–1249 8
D moon_engine.py, moon_bridge.py, moon_cli.py, moon_download.py 7

Slice D touches four files but is the smallest and most self-contained — a good one if it is your first PR here.

What to do

For each handler in your slice:

  • If a narrower exception is obviously right — name it. except (OSError, asyncio.TimeoutError): says far more than except Exception:. refactor(cleanup): annotate exception swallows in moon_bridge.py and … #54 narrowed native_dialog to (subprocess.TimeoutExpired, OSError) and that is the standard to follow.
  • If the swallow is deliberate and has to stay broad — add a one-line comment saying what is expected to fail and why continuing is correct. State a specific fact. "Best-effort cleanup during shutdown" is a reason; "just in case" is not.
  • If you cannot work out why it is there — leave it and say so in the PR. A # unclear note from a contributor is more useful than a confident guess, and I can fill it in.

Do not change behaviour beyond narrowing an exception set you are confident about. No new logging, no refactoring, no reordering.

Acceptance criteria

  • Every handler in the claimed slice is either narrowed or carries a one-line reason
  • No comment restates the code (# catch exception is not a reason)
  • pytest tests/ -q passes
  • Nothing outside the claimed slice is modified
  • The PR says which slice it covers

Notes for the contributor

No Windows needed. pytest tests/ -q stubs Chrome and the network at the moon_extract boundary, so the suite runs on Linux and macOS.

The useful skill here is reading a defensive handler and working out what it is actually defending against. That is worth more than the diff, and it is why a PR that says "these four are deliberate, this fifth one I could not explain" is a better PR than one that comments all five confidently.

Comment with your slice letter before you start.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions