Логирование: классификация доменных ошибок (500→409/400) + конвенции
Штатные конфликты и промахи ввода возвращались голым fmt.Errorf, поэтому
classifyErr отправлял их в 500 «внутренняя ошибка» вместо 409/400 (и logCmd
писал ERROR вместо DEBUG). Продолжение f8fb4fa (Tier A), по итогам ревью Fable.
Классификация ошибок:
- новый sentinel worker.ErrInvalidInput → 400 для валидации ввода команд
(refine/set type/ignore/add source/set provider/choose candidate);
- обёртки %w ErrConflict в Cancel/Retry/Defer/Undo (штатный конфликт состояния);
- classifyErr: ErrInvalidInput→400, layout.ErrCollision→409 (коллизия цели
штатно уводит в review); ветка ErrCollision в tgbot (сообщение + refreshCard);
- logCmd относит ErrInvalidInput и ErrCollision в DEBUG «command rejected».
Конвенции (docs/conventions):
- logging.md: публичные команды воркера = доменная граница (лог один раз,
logCmd); таблица уровней доменных отказов (граница команды vs асинхронная
стадия); правило про *url.Error/секреты в URL; канон категории
state transition; уровень повторяющихся сбоев фоновых циклов;
- errors.md: таблица маппинга ошибка→статус; развилка «транзиентный ответ vs
персистентная диагностика» решена как (а) — error_msg/reasons на review-экране
и tg-карточке = операторская поверхность владельца (сырой текст ок, секреты
запрещены; аудит подтвердил, что секреты туда не текут).
Унификация категории лога state transition: cancel/retry/relink/recovery
переведены с семантических msg на общий state transition (from/to) — весь
жизненный цикл собирается одним jq-фильтром.
Мелочи: reason-коды linkPlan в const-блок; httpapi лог-поля id→download_id и
msg «… failed»; комментарий «почему» у parseIgnored; preview build failure в
ReviewData DEBUG→WARN.
Беклог: задача сведена к остатку (ext.* ERROR-шторм при недоступном qBittorrent
+ эскалация устойчивого сбоя тика), понижена в приоритете.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,88 +1,54 @@
|
||||
# Классификация доменных ошибок (500→409/400) и конвенции логирования
|
||||
# Шум ERROR фоновых циклов при недоступной зависимости
|
||||
|
||||
**Приоритет:** средний · **Теги:** review-fable, errors, logging
|
||||
**Приоритет:** низкий · **Теги:** review-fable, logging, reliability
|
||||
|
||||
Продолжение коммита `f8fb4fa` (логирование ошибок: доменная граница + защита
|
||||
секретов). Тот коммит закрыл «ядро + секреты» (Tier A) по итогам ревью двумя
|
||||
сабагентами Fable (инфраструктурные и доменные ошибки). Здесь — отложенные
|
||||
Tier B (классификация) и Tier C (конвенции + политика), не вошедшие в scope.
|
||||
Остаток от задачи «классификация доменных ошибок + конвенции логирования»
|
||||
(основное реализовано, см. ниже). Здесь — два смежных пункта про уровень
|
||||
повторяющихся сбоев фоновых циклов, каждый требует небольшого решения, а не
|
||||
только правки.
|
||||
|
||||
## Tier B — классификация ошибок (сейчас штатные конфликты/валидация → 500)
|
||||
## Что уже сделано (не переоткрывать)
|
||||
|
||||
Проблема: часть доменных отказов возвращается голым `fmt.Errorf` без sentinel,
|
||||
поэтому `classifyErr` (httpapi) отправляет их в `default` → **500 «внутренняя
|
||||
ошибка»** вместо 409/400. Побочно: новый `logCmd` (worker) логирует такие
|
||||
отказы как **ERROR**, хотя это норма (адресат — пользователь), должно быть
|
||||
DEBUG.
|
||||
Коммит `f8fb4fa` (Tier A) + коммит этой задачи закрыли:
|
||||
|
||||
- **Обернуть `ErrConflict`** в командах, где конфликт состояния не обёрнут (в
|
||||
отличие от `requireReviewable`/`Apply`/`Undo`/`Delete`, где уже сделано):
|
||||
- `worker.go:789` — `cancel: ... is already terminal`;
|
||||
- `worker.go:809` — `retry: ... only failed/stuck are retriable`;
|
||||
- `review.go:515` — `defer: ... is terminal`;
|
||||
- `review.go:551` — `undo: nothing to revert` (скорее конфликт).
|
||||
- **Sentinel для валидации ввода** (`ErrInvalidInput` → 400) или дооборачивание,
|
||||
чтобы промах пользователя не выглядел сбоем ни в статусе, ни в уровне лога:
|
||||
- `review.go` — `refine: empty hint`, `set type: invalid type`,
|
||||
`ignore: empty path`, `add source: invalid provider`/`empty id`,
|
||||
`set provider: invalid provider`/`empty id`,
|
||||
`choose candidate: candidate ... does not belong`.
|
||||
- **`classifyErr` не знает `layout.ErrCollision`**: ручной Apply с коллизией
|
||||
цели штатно уводит задачу в review с причиной (`review.go` linkPlan), но
|
||||
пользователь в вебе получает 500, а tgbot — «Не удалось выполнить действие».
|
||||
Добавить кейс (409 или сценарий «ушло в ревью: коллизия цели») и исключение
|
||||
в tgbot по аналогии с `ErrNotReady`.
|
||||
- **Классификация доменных ошибок:** sentinel `worker.ErrInvalidInput`→400;
|
||||
обёртки `ErrConflict` в Cancel/Retry/Defer/Undo; `layout.ErrCollision`→409 в
|
||||
`classifyErr` и ветка в tgbot; `logCmd` относит новые классы в DEBUG.
|
||||
- **Конвенции:** `logging.md` — команды воркера = доменная граница, таблица
|
||||
уровней доменных отказов (граница команды vs асинхронная стадия), правило про
|
||||
`*url.Error`/секреты в URL, канон категории `state transition` (унифицированы
|
||||
cancel/retry/relink/recovery). `errors.md` — таблица маппинга ошибка→статус,
|
||||
развилка «транзиентный ответ vs персистентная диагностика» решена как (а):
|
||||
`error_msg`/`reasons` — операторская поверхность владельца (сырой текст ок,
|
||||
секреты запрещены; аудит показал, что секреты туда не текут).
|
||||
- **Мелочи:** reason-коды const-блок; лог-поля `id`→`download_id`; preview
|
||||
WARN; комментарий у `parseIgnored`.
|
||||
|
||||
После sentinel-обёрток обновить `logCmd` (worker.go) — он уже относит
|
||||
`ErrConflict/ErrNotReady/ErrNotFound` в DEBUG; добавить туда новый
|
||||
`ErrInvalidInput`.
|
||||
## Остаток
|
||||
|
||||
## Tier C — конвенции и политика (docs/conventions)
|
||||
### ERROR-шторм при недоступном qBittorrent
|
||||
|
||||
Оба ревьюера предложили закрепить в `logging.md`/`errors.md` (чтобы дыры и
|
||||
дубли не возникали снова — уже дважды выстрелило: ingest, затем команды):
|
||||
Клиент `qbt` логирует `ext.*` `Failure` → **ERROR** на каждом тике поллинга
|
||||
(`torrents/info`, `internal/qbt/qbt.go`), пока qBittorrent недоступен (рестарт
|
||||
демона, сеть). Домен уже пишет `poll failed` = WARN (по новой конвенции), но
|
||||
транспортная `ext.*`-запись остаётся ERROR по правилу ext-конвенции («сервис
|
||||
недоступен → ERROR»). При частом поллинге это шумит.
|
||||
|
||||
- **Команды воркера = доменная граница.** Явно дописать в `logging.md`, раздел
|
||||
«Ошибки»: публичные методы воркера (`Apply`/`Refine`/`Cancel`/…) — граница
|
||||
домена, логируют исход ровно один раз (реализовано в `logCmd`); транспорты не
|
||||
логируют возвращённую ошибку. Сейчас формулировка «стадии воркера» двусмысленна.
|
||||
- **Таблица уровней доменных отказов:** `ErrConflict`/`ErrNotReady`/валидация →
|
||||
DEBUG (адресат — пользователь); нарушенный инвариант хранилища → INFO/WARN;
|
||||
прочее (БД/ФС/зависимости) → ERROR. Правило: у каждой доменной ошибки ровно
|
||||
один логирующий, уровень — по адресату, а не по месту.
|
||||
- **Правило про `*url.Error`/секреты в URL** (раздел «Безопасность»): ошибки
|
||||
HTTP-транспорта встраивают URL, который может нести секрет — санитизировать на
|
||||
границе клиента до лога и обёртки. Частично реализовано (`logging.SanitizeErr`,
|
||||
применён в `ExtCall`/tgbot/metadata) — осталось задокументировать + правило
|
||||
«секрет не кладём в URL, если у API есть заголовок».
|
||||
- **Уровень повторяющихся сбоев фоновых циклов:** `poll/sweep/list failed` =
|
||||
WARN, а та же ошибка БД в `ingest.Ingest` = ERROR. Договориться о едином
|
||||
правиле (транзиентный сбой тика → WARN; эскалация в ERROR при устойчивом сбое
|
||||
N тиков) — сейчас уровень зависит от места. Смежно: ERROR-шторм при
|
||||
недоступном qBittorrent (`qbt` логирует Failure на каждом тике поллинга).
|
||||
- **Единая категория переходов состояния:** свести к одному msg `state
|
||||
transition` (from/to/code); `download cancelled`/`download retried`/`relink
|
||||
re-recognizing` не теряются из-под `jq 'select(.msg=="state transition")'`.
|
||||
Развилка (решить до правки):
|
||||
|
||||
## Развилка — сырой `err.Error()` на публичных поверхностях
|
||||
- (а) Ввести у `logging.ExtCall` вариант с пониженным уровнем для рутинно-частых
|
||||
вызовов (симметрично `SuccessDebug`) — поллинг-вызовы (`torrents/info`) на
|
||||
транзиентном сбое пишут WARN, не ERROR;
|
||||
- (б) Дедуп/circuit-breaker: первый ERROR, дальше тишина до восстановления;
|
||||
- (в) Оставить как есть, признав `ext.*` ERROR легитимным сигналом «зависимость
|
||||
лежит» (тогда шум гасить уровнем сбора, а не кодом).
|
||||
|
||||
`reasons` (`review.go` recognizeOne) и `error_msg` переходов (`linkPlan`)
|
||||
показываются в review-экране и Telegram-карточке и могут нести детали
|
||||
реализации (тело LLM/qBit, абсолютные пути). Решить: (а) узаконить в `errors.md`
|
||||
как «операторская поверхность владельца» с явным запретом секретов; либо (б)
|
||||
держать там reason-код + нейтральный текст, полную ошибку — в лог по
|
||||
`download_id`.
|
||||
### Эскалация устойчивого сбоя тика
|
||||
|
||||
## Мелочь (по желанию, из тех же ревью)
|
||||
Сейчас транзиентный сбой тика = WARN всегда. Договорённость на будущее
|
||||
(`logging.md`): устойчивый сбой N тиков подряд эскалировать в ERROR (реальная
|
||||
деградация, а не разовый промах). Не реализовано — нужен счётчик подряд-сбоев по
|
||||
циклу и порог в конфиге.
|
||||
|
||||
- `httpapi/review.go` — поле `"id"` вместо словарного `"download_id"`; msg
|
||||
«review data» → категория «review data failed».
|
||||
- `worker/review.go` `parseIgnored` — `_ = json.Unmarshal` без обязательного
|
||||
комментария «почему» (битый override молча обнуляет игнор).
|
||||
- Reason-коды переходов (`"resolve"`/`"build"`/`"persist"`/`"collision"`/…) —
|
||||
свести в const-блок рядом с `errCode*` (сейчас только `reasonTitleFolderDesync`).
|
||||
- Превью в `ReviewData` при сбое — DEBUG, хотя это видимая деградация (пропадает
|
||||
кнопка «Применить»); уместнее WARN.
|
||||
|
||||
Вердикт: change (Tier B — правки кода + миграция статусов ошибок; Tier C —
|
||||
правки конвенций + одно продуктовое решение по `err.Error()`).
|
||||
Вердикт: мелкая надёжностная полировка, не блокер. Делать вместе (обе про
|
||||
уровень сбоев фоновых циклов) или отдельной строкой.
|
||||
|
||||
Reference in New Issue
Block a user