peter
4aaaaa7860
The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").
Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.
Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).
One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
72 lines
2.9 KiB
Python
72 lines
2.9 KiB
Python
"""Every swallowed exception has been ruled on.
|
|
|
|
A handler that neither logs nor re-raises is a decision: someone looked at it and decided the
|
|
failure is not worth a line. 114 of them were in `app/` at the start of this epic, and almost none
|
|
carried that decision anywhere — which means nobody could tell the deliberate ones from the ones
|
|
that were never thought about. That is the whole defect: not that they are silent, but that their
|
|
silence says nothing.
|
|
|
|
So the rule is not "log everything". It is: a silent handler carries `# silent-ok: <why>` on the
|
|
`except` line or the line above it. Ruff's S110 and BLE001 sit under this and catch the two narrow
|
|
shapes (`except: pass`, blind `except Exception`); this case covers the rest — the handler that
|
|
quietly returns a fallback, or `continue`s past one item of a loop.
|
|
"""
|
|
|
|
import ast
|
|
import pathlib
|
|
|
|
from app import logs
|
|
|
|
APP = pathlib.Path(logs.__file__).resolve().parent
|
|
_MARKER = "silent-ok"
|
|
_LOG_METHODS = frozenset(
|
|
{"debug", "info", "warning", "warn", "error", "exception", "critical", "log"}
|
|
)
|
|
|
|
|
|
def _is_log_call(node: ast.AST) -> bool:
|
|
if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Attribute):
|
|
return False
|
|
if node.func.attr not in _LOG_METHODS:
|
|
return False
|
|
base, name = node.func.value, ""
|
|
while isinstance(base, ast.Attribute):
|
|
name = f"{base.attr}.{name}"
|
|
base = base.value
|
|
if isinstance(base, ast.Name):
|
|
name = f"{base.id}.{name}"
|
|
return "log" in name.lower()
|
|
|
|
|
|
def _swallows(handler: ast.ExceptHandler) -> bool:
|
|
"""Neither reports nor re-raises. A nested `raise` counts: the caller still finds out."""
|
|
for node in ast.walk(handler):
|
|
if isinstance(node, ast.Raise) or _is_log_call(node):
|
|
return False
|
|
return True
|
|
|
|
|
|
def test_every_silent_handler_says_why_it_is_silent():
|
|
offenders = []
|
|
for path in sorted(APP.rglob("*.py")):
|
|
src = path.read_text(encoding="utf-8")
|
|
lines = src.splitlines()
|
|
for node in ast.walk(ast.parse(src)):
|
|
if not isinstance(node, ast.ExceptHandler) or not _swallows(node):
|
|
continue
|
|
# The marker may sit in the comment block attached above the `except`, on the
|
|
# `except` line itself, or anywhere inside the handler — all three are this handler
|
|
# and nothing else, so no justification can excuse a neighbour.
|
|
start = node.lineno - 1
|
|
while start > 0 and lines[start - 1].lstrip().startswith("#"):
|
|
start -= 1
|
|
end = node.end_lineno or node.lineno
|
|
if _MARKER in " ".join(lines[start:end]):
|
|
continue
|
|
offenders.append(f"{path.relative_to(APP)}:{node.lineno}")
|
|
assert offenders == [], (
|
|
f"{len(offenders)} exception handlers neither log nor re-raise and do not say why. Add a "
|
|
f"log line with an event code, or `# {_MARKER}: <why the failure is not worth a line>` — "
|
|
f"{offenders}"
|
|
)
|