chore(lint): interdire d avaler une exception sans laisser de trace
Regle BLE de ruff activee. Ce qu'elle enforce n'est pas "ne jamais attraper large" : elle se satisfait d'un logger.exception. C'est exactement la discipline visee — une frontiere peut tout avaler, a condition de laisser de quoi distinguer un defaut d'une panne. Les cinq noqa que j'avais prepares d'avance etaient donc inertes ; la raison reste en commentaire simple. Ce que la regle a trouve, une fois activee : app.py, demarrage du bot — les deux facons d'echouer, jeton invalide et import casse, se lisaient a l'identique sur une seule ligne et aucune n'etait diagnosticable. Passe en error avec exc_info : un club qui ne recoit plus aucun rappel a perdu une fonctionnalite, et warning mettait ca a cote des avis de depreciation. services/notifications.py — le bloc webhook attrapait large autour d'un requests.post. RequestException couvre toutes les facons dont un appel HTTP echoue ; le reste est un defaut. La branche DM et la branche webhook etaient en plus imbriquees dans un seul try alors qu'elles s'excluent. logging_config.py et les deux scripts CLI gardent leur largeur, avec la raison sur la ligne. Le filtre de journalisation est le cas ou la trace que BLE001 reclame est precisement ce qu'il ne faut pas produire : journaliser depuis un filtre rentre dans le meme filtre. RUF100 (noqa inutile) n'est volontairement pas active : il ferait remonter des directives preexistantes sans rapport avec ce chantier. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
+13
-2
@@ -609,8 +609,19 @@ def create_app(config=None):
|
|||||||
from app.discord_bot import start_bot
|
from app.discord_bot import start_bot
|
||||||
|
|
||||||
start_bot(flask_app=app)
|
start_bot(flask_app=app)
|
||||||
except Exception as e:
|
except Exception: # the site must come up even if the bot cannot
|
||||||
app.logger.warning('Could not start Discord bot: %s', e)
|
# With the message alone, the two ways this fails — a bad token
|
||||||
|
# and a broken import in discord_bot — read identically, and
|
||||||
|
# neither is diagnosable from one line. Notifications are down
|
||||||
|
# either way, so the traceback is the whole value of the log.
|
||||||
|
# Error, not warning: a club that receives no reminders has lost
|
||||||
|
# a feature, and the old level put that next to the deprecation
|
||||||
|
# notices.
|
||||||
|
app.logger.error(
|
||||||
|
'Could not start the Discord bot. The site is up; no notification '
|
||||||
|
'will be sent until this is fixed.',
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
|
||||||
return app
|
return app
|
||||||
|
|
||||||
|
|||||||
@@ -88,8 +88,12 @@ class SensitiveDataFilter(logging.Filter):
|
|||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
rendered = record.getMessage()
|
rendered = record.getMessage()
|
||||||
except Exception:
|
except Exception: # noqa: BLE001 — see below; this one cannot log its own failure
|
||||||
# A malformed format string must not lose the record entirely.
|
# A malformed format string must not lose the record entirely.
|
||||||
|
# Nor can it be logged: this runs inside a filter, and logging
|
||||||
|
# from here re-enters the same filter on the new record. The
|
||||||
|
# traceback BLE001 normally asks for is the one thing this
|
||||||
|
# handler must not produce, hence the waiver.
|
||||||
return True
|
return True
|
||||||
|
|
||||||
for pattern, replacement in self.SENSITIVE_PATTERNS:
|
for pattern, replacement in self.SENSITIVE_PATTERNS:
|
||||||
|
|||||||
@@ -4,10 +4,15 @@ Extracted from app/routes/users.py, where it sat between two route
|
|||||||
definitions and pulled `requests`, `logging` and the Discord bot into a
|
definitions and pulled `requests`, `logging` and the Discord bot into a
|
||||||
module whose subject is HTTP handlers (ARCH-003).
|
module whose subject is HTTP handlers (ARCH-003).
|
||||||
|
|
||||||
Every failure here is swallowed and logged on purpose: a notification that
|
Failures here are swallowed and logged on purpose: a notification that does
|
||||||
does not reach Discord must not roll back the session it was announcing.
|
not reach Discord must not roll back the session it was announcing. That is
|
||||||
That is the one place in this codebase where `except Exception` is the
|
a property of the caller — a Flask request whose work is already committed —
|
||||||
right answer rather than an oversight.
|
not of the failure, which is why the breadth is argued for at each of the
|
||||||
|
two boundaries below rather than assumed (ARCH-008 / QUA-004).
|
||||||
|
|
||||||
|
The webhook branch is narrower than it was: `requests.RequestException`
|
||||||
|
covers every way an HTTP call can fail, and anything else coming out of it
|
||||||
|
is a defect worth seeing.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
@@ -61,14 +66,18 @@ def send_discord_notification(
|
|||||||
points=points,
|
points=points,
|
||||||
request_id=request_id,
|
request_id=request_id,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception: # noqa: BLE001 — the request that booked the meeting is already committed
|
||||||
logger.warning(f"Failed to send Discord DM: {e}")
|
logger.warning('Failed to hand the One on One DM to the bot', exc_info=True)
|
||||||
|
|
||||||
if DISCORD_WEBHOOK_URL:
|
if not DISCORD_WEBHOOK_URL:
|
||||||
try:
|
return
|
||||||
|
|
||||||
|
# A bare snowflake here means "DM this person instead", and only when the
|
||||||
|
# coach has no id of their own. Queueing it cannot raise (see _enqueue).
|
||||||
|
if DISCORD_WEBHOOK_URL.isdigit():
|
||||||
|
if not coach_discord_id:
|
||||||
from app.discord_bot import send_one_on_one_dm
|
from app.discord_bot import send_one_on_one_dm
|
||||||
|
|
||||||
if DISCORD_WEBHOOK_URL.isdigit() and not coach_discord_id:
|
|
||||||
send_one_on_one_dm(
|
send_one_on_one_dm(
|
||||||
coach_name=coach_name,
|
coach_name=coach_name,
|
||||||
coach_discord_id=DISCORD_WEBHOOK_URL,
|
coach_discord_id=DISCORD_WEBHOOK_URL,
|
||||||
@@ -79,7 +88,8 @@ def send_discord_notification(
|
|||||||
end_time=end_time_str,
|
end_time=end_time_str,
|
||||||
points=points,
|
points=points,
|
||||||
)
|
)
|
||||||
elif not DISCORD_WEBHOOK_URL.isdigit():
|
return
|
||||||
|
|
||||||
embed = {
|
embed = {
|
||||||
"embeds": [
|
"embeds": [
|
||||||
{
|
{
|
||||||
@@ -111,6 +121,7 @@ def send_discord_notification(
|
|||||||
}
|
}
|
||||||
],
|
],
|
||||||
}
|
}
|
||||||
|
try:
|
||||||
requests.post(DISCORD_WEBHOOK_URL, json=embed, timeout=5)
|
requests.post(DISCORD_WEBHOOK_URL, json=embed, timeout=5)
|
||||||
except Exception as e:
|
except requests.RequestException as exc:
|
||||||
logger.warning(f"Failed to send Discord notification: {e}")
|
logger.warning('Failed to post the One on One webhook: %s', exc)
|
||||||
|
|||||||
@@ -249,7 +249,11 @@ def backup_documents():
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
shutil.make_archive(archive_basename, 'zip', DOCUMENTS_DIR)
|
shutil.make_archive(archive_basename, 'zip', DOCUMENTS_DIR)
|
||||||
except Exception as exc:
|
except Exception as exc: # noqa: BLE001 — a failed document archive must not lose the dump
|
||||||
|
# This runs after the database dump has already succeeded. Letting
|
||||||
|
# anything through here would abort the script with a traceback and
|
||||||
|
# take the one part that worked down with it. Reported to stdout, in
|
||||||
|
# the format the rest of this script uses; it has no logger.
|
||||||
print(f'[ERROR] Document backup failed: {exc}')
|
print(f'[ERROR] Document backup failed: {exc}')
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|||||||
@@ -332,10 +332,15 @@ def check_flask_config():
|
|||||||
else:
|
else:
|
||||||
print('[OK] DEBUG mode: disabled')
|
print('[OK] DEBUG mode: disabled')
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e: # noqa: BLE001 — any failure to load the app is a failed check
|
||||||
# Returning all_ok (still True) here meant that failing to load the
|
# Returning all_ok (still True) here meant that failing to load the
|
||||||
# application at all was counted as a passing check — the most
|
# application at all was counted as a passing check — the most
|
||||||
# important section of the report silently never ran.
|
# important section of the report silently never ran.
|
||||||
|
#
|
||||||
|
# The breadth is the point: this section's question is "does the
|
||||||
|
# application load with a safe configuration", and every way of not
|
||||||
|
# loading answers it the same way. Reported on stdout because this
|
||||||
|
# script is read by a CI job, not by a log collector.
|
||||||
print(f'[FAIL] Cannot check Flask config: {e}')
|
print(f'[FAIL] Cannot check Flask config: {e}')
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|||||||
+8
-1
@@ -45,7 +45,14 @@ exclude = [".venv", "venv", "migrations", "docs"]
|
|||||||
# I isort; enabled in its own commit, for the same reason the
|
# I isort; enabled in its own commit, for the same reason the
|
||||||
# formatting got one — the diff is churn and must not hide
|
# formatting got one — the diff is churn and must not hide
|
||||||
# behind a behavioural change
|
# behind a behavioural change
|
||||||
select = ["E4", "E7", "E9", "F", "B", "C4", "RET", "SIM", "UP", "I"]
|
# BLE blind `except Exception`. Added with QUA-004/ARCH-008, and
|
||||||
|
# the point of it is what happens next: the rule cannot be
|
||||||
|
# satisfied by narrowing every catch, because a handful of
|
||||||
|
# boundaries genuinely need the breadth. It can only be
|
||||||
|
# satisfied by writing down which ones and why, in a
|
||||||
|
# `# noqa: BLE001 — reason` on the line. That turns a count
|
||||||
|
# that drifts back up into a decision someone has to defend.
|
||||||
|
select = ["E4", "E7", "E9", "F", "B", "C4", "RET", "SIM", "UP", "I", "BLE"]
|
||||||
|
|
||||||
# Forcing a ternary reads worse than the if/else it replaces in the one place
|
# Forcing a ternary reads worse than the if/else it replaces in the one place
|
||||||
# it fires (evaluations.py, choosing a sort direction).
|
# it fires (evaluations.py, choosing a sort direction).
|
||||||
|
|||||||
Reference in New Issue
Block a user