From 7d8a455e478070ee4f8f7fc45198e6e868503523 Mon Sep 17 00:00:00 2001 From: Anton Vakhrushev Date: Fri, 10 Jul 2026 14:57:12 +0300 Subject: [PATCH] =?UTF-8?q?=D0=9B=D0=BE=D0=B3=D0=B8=D1=80=D0=BE=D0=B2?= =?UTF-8?q?=D0=B0=D0=BD=D0=B8=D0=B5:=20=D0=BA=D0=BB=D0=B0=D1=81=D1=81?= =?UTF-8?q?=D0=B8=D1=84=D0=B8=D0=BA=D0=B0=D1=86=D0=B8=D1=8F=20=D0=B4=D0=BE?= =?UTF-8?q?=D0=BC=D0=B5=D0=BD=D0=BD=D1=8B=D1=85=20=D0=BE=D1=88=D0=B8=D0=B1?= =?UTF-8?q?=D0=BE=D0=BA=20(500=E2=86=92409/400)=20+=20=D0=BA=D0=BE=D0=BD?= =?UTF-8?q?=D0=B2=D0=B5=D0=BD=D1=86=D0=B8=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Штатные конфликты и промахи ввода возвращались голым 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) --- docs/backlog/README.md | 2 +- ...klassifikaciya-i-konvencii-logirovaniya.md | 116 +++++++----------- docs/conventions/errors.md | 45 +++++-- docs/conventions/logging.md | 55 ++++++++- internal/httpapi/download.go | 2 +- internal/httpapi/httpapi.go | 20 ++- internal/httpapi/httpapi_test.go | 35 ++++++ internal/httpapi/review.go | 6 +- internal/tgbot/bot.go | 9 ++ internal/worker/errors.go | 7 ++ internal/worker/reconcile.go | 4 +- internal/worker/review.go | 56 +++++---- internal/worker/worker.go | 13 +- 13 files changed, 247 insertions(+), 123 deletions(-) diff --git a/docs/backlog/README.md b/docs/backlog/README.md index 3a4124d..67193d8 100644 --- a/docs/backlog/README.md +++ b/docs/backlog/README.md @@ -35,7 +35,6 @@ Tududi (проект `jellybit`) больше **не** держит беклог - [Обучение на правках человека (few-shot из прошлых ревью)](obuchenie-na-pravkah.md) — Когда человек поправил матч, тип или нумерацию — сохранять это как пример и подмешивать… - [Confidence-гейт авто-раскладки: узаконить в спеке + сделать выключаемым (дефолт 0.7)](gate-confidence-spec-vs-code.md) — Решено (B): гейт оставляем как доп. проверку на ревью — выключаемый порог, дефолт 0.85→0.7, записать в спеку - [Внешние субтитры: пары VobSub и языковой суффикс](vneshnie-subtitry.md) — Привязка субтитр→серия уже работает; остались пары VobSub .idx+.sub и потеря Lang/Flags -- [Классификация доменных ошибок (500→409/400) и конвенции логирования](oshibki-klassifikaciya-i-konvencii-logirovaniya.md) — Продолжение коммита f8fb4fa (Tier A сделан): sentinel-обёртки ErrConflict/валидации, classifyErr для ErrCollision, правки docs/conventions _(ревью Fable)_ - [Defer из catched → лимбо → необратимый deleted (MAJOR-6)](review-major6-defer-catched.md) — Defer из ещё-не-добавленного catched уводит задачу в необратимый deleted _(ревью 2026-07-08)_ - [processCatched: promote-without-add если торрент уже в qBittorrent (F2)](review-f2-promote-without-add.md) — торрент уже в qBittorrent → processCatched зациклен на Add вместо promote _(ревью 2026-07-08)_ - [Cancel во время add оставляет неуправляемый торрент в qBittorrent (F3/NIT-13)](review-f3-cancel-during-add.md) — Cancel во время add оставляет неуправляемый торрент в qBittorrent _(ревью 2026-07-08)_ @@ -44,6 +43,7 @@ Tududi (проект `jellybit`) больше **не** держит беклог - [Панель действий ревью вне htmx-свопа блока источника](panel-review-vne-swap.md) — При выборе источника одним кликом обновляется только блок источника (#source-block)… - [Мгновенные обновления через SSE](sse-obnovleniya.md) — Живые обновления прогресса сейчас на htmx-поллинге (фаза 2 веб-UI) — просто и работает… +- [Шум ERROR фоновых циклов при недоступной зависимости](oshibki-klassifikaciya-i-konvencii-logirovaniya.md) — Остаток задачи логирования: ext.* ERROR-шторм при недоступном qBittorrent + эскалация устойчивого сбоя тика _(ревью Fable)_ - [Версии/качество одного тайтла (репаки, апгрейд 1080p → 2160p)](versii-kachestvo-repaki.md) — По калибровке болей (2026-07-02) — не боль, из приоритета выпало - [[идея] Многоступенчатая верификация привязки](mnogostupenchataya-verifikaciya.md) — ИДЕЯ (требует проработки) - [Выбор из нескольких находок метабазы в Telegram](telegram-vybor-nahodok.md) — Когда распознавание даёт несколько подходящих кандидатов в метабазе, предлагать их в… diff --git a/docs/backlog/oshibki-klassifikaciya-i-konvencii-logirovaniya.md b/docs/backlog/oshibki-klassifikaciya-i-konvencii-logirovaniya.md index 7e98e12..d5b6890 100644 --- a/docs/backlog/oshibki-klassifikaciya-i-konvencii-logirovaniya.md +++ b/docs/backlog/oshibki-klassifikaciya-i-konvencii-logirovaniya.md @@ -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()`). +Вердикт: мелкая надёжностная полировка, не блокер. Делать вместе (обе про +уровень сбоев фоновых циклов) или отдельной строкой. diff --git a/docs/conventions/errors.md b/docs/conventions/errors.md index c9298a8..042590d 100644 --- a/docs/conventions/errors.md +++ b/docs/conventions/errors.md @@ -68,14 +68,45 @@ jellybit — **приложение, а не библиотека**: внешн к загрузке) либо `request_id`, чтобы по нему найти полную ошибку в логах. Пример: «При обработке загрузки произошла ошибка, download_id=12345», а не «произошла ошибка» и не сырой текст; - - **маппинг доменной ошибки → статус/сообщение**: `ErrNotFound` → 404 - «не найдено», валидация/`ErrNotMagnet` → 400 «некорректный источник», - конфликт состояния (`ErrConflict` — операция недопустима в текущем - состоянии) → 409 «действие недоступно в текущем состоянии», прочее → - 500 «внутренняя ошибка». + - **маппинг доменной ошибки → статус/сообщение** (в jellybit — + `httpapi.classifyErr`, единая точка для REST и веб-UI): -Граница публичная по умолчанию. Истинно приватный для владельца канал — -логи; отдельной «операторской» поверхности с сырыми ошибками не заводим. + | Доменная ошибка | Статус | Сообщение | + |---|---|---| + | `store.ErrNotFound` | 404 | «не найдено» | + | `magnet.ErrNotMagnet` / `torrent.ErrNotTorrent` | 400 | «некорректный источник» | + | `worker.ErrInvalidInput` (промах ввода команды) | 400 | «некорректный ввод» | + | `worker.ErrNotReady` (источник ещё качается) | 409 | «торрент ещё качается…» | + | `layout.ErrCollision` (цель занята, ушло в review) | 409 | «целевой файл уже существует…» | + | `worker.ErrConflict` (операция недопустима сейчас) | 409 | «действие недоступно в текущем состоянии» | + | прочее | 500 | «внутренняя ошибка» | + + Новую штатную ветвь отказа (конфликт/валидация) заводим sentinel’ом и + добавляем сюда — иначе `default` отдаст 500 «внутренняя ошибка» на + нормальный конфликт (и логирующая граница спишет его в `ERROR` вместо + `DEBUG`, см. [logging.md](logging.md)). + +### Транзиентный ответ vs персистентная диагностика + +У публичной границы две разные поверхности, и правило сырого текста для них +разное: + +- **Транзиентный ответ на действие** (тело REST/`?err=`/answer бота по + результату команды) — строго нейтральный: маппинг выше, `err.Error()` наружу + не идёт, полная ошибка — в логах по `download_id`/`request_id`. +- **Персистентная диагностика состояния** — `error_msg` перехода (причина ухода + в review/failed: коллизия, рассинхрон, сбой ФС) и `reasons` распознавания, + сохранённые в БД и показываемые на экране ревью и в Telegram-карточке. Это + **операторская поверхность владельца**: сервис однопользовательский в + доверенной LAN (см. [architecture.md](../specs/architecture.md)), эти поля — + диагностический контекст для того, кто разбирает задачу. Здесь сырой текст + ошибки (пути, фрагмент ответа LLM/qBittorrent) **допустим и полезен** — но: + - **секреты запрещены** абсолютно (токены/ключи/пароли/`Authorization`) — так + же, как в логах ([logging.md](logging.md), «Безопасность»). Источник + error_msg вычищаем на границе клиента (`logging.SanitizeErr` для ошибок + транспорта, несущих URL с секретом); + - это **не** канал для транзиентных отказов команд — те остаются нейтральными + (см. выше). ## panic diff --git a/docs/conventions/logging.md b/docs/conventions/logging.md index 6d45a1b..5d93a19 100644 --- a/docs/conventions/logging.md +++ b/docs/conventions/logging.md @@ -39,6 +39,13 @@ log.Info(fmt.Sprintf("download %s accepted as movie", id)) - `msg` — чистая категория без неймспейс-префикса: `recognition done`, а не `recognize: done`. Подсистему выносим в поле `capability` (`ingest`/`recognition`/`file-layout`/`review`), не в текст. +- **Смена состояния загрузки — единая категория `state transition`** с полями + `from`/`to`/`code` (какое именно состояние и по какой причине — это данные, + не текст). Любой переход (в т.ч. `cancel`/`retry`/`relink`) пишет этот + `msg`, чтобы весь жизненный цикл собирался одним фильтром: `jq + 'select(.msg=="state transition" and .download_id=="…")'`. Физический эффект + сверх перехода — отдельная запись своей категории (`layout linked`, + `layout reverted`, `review hint added`), не подменяет запись перехода. ## Уровни @@ -141,13 +148,44 @@ log.Error(err.Error()) - Идиома Go — **либо лог, либо возврат, не оба**. Промежуточные слои только оборачивают и возвращают (`fmt.Errorf("…: %w", err)`), не логируя — контекст накапливается в цепочке `%w`. -- Логируем ошибку **один раз — на границе доменного слоя** (use-case - `Ingest`, стадии воркера), которая определяет исход операции: полем - `error`, уровень `ERROR`. В Go логирует этот единый чокпоинт, а не каждый - транспорт — так транспорты остаются тонкими. +- Логируем ошибку **один раз — на границе доменного слоя**, которая + определяет исход операции: полем `error`. В Go логирует этот единый + чокпоинт, а не каждый транспорт — так транспорты остаются тонкими. Границы + в jellybit: + - use-case `Ingest` (приём); + - **асинхронные стадии воркера** (поллинг, распознавание, авто-раскладка) — + исход стадии, вызванной таймером/циклом; + - **публичные команды воркера** (`Apply`/`Refine`/`Cancel`/`Retry`/`Undo`/ + `Delete`/…), вызываемые транспортами. Исход команды логирует ровно один + чокпоинт (`worker.logCmd`, в `defer` при именованном возврате), а не + HTTP/web/Telegram — они одну и ту же команду зовут из трёх мест. - Транспорты (HTTP/web/Telegram) переводят возвращённую ошибку в свой ответ (статус, сообщение пользователю) и **не логируют** её повторно — иначе один сбой даёт дубли. +- **Уровень доменного отказа — по адресату, а не по месту.** У каждой + доменной ошибки ровно один логирующий; уровень выбирает он. На **границе + команды** (пользователь инициировал действие и ждёт ответа — `worker.logCmd`): + + | Класс отказа | Кому | Уровень | + |---|---|---| + | штатный конфликт состояния / некорректный ввод (`ErrConflict`, `ErrNotReady`, `ErrInvalidInput`, `ErrNotFound`, `layout.ErrCollision`) | пользователю (уже получил ответ на поверхности) | `DEBUG` | + | нарушенный инвариант хранилища/учёта (не безопасность данных: файлы уже разложены) | команде, «может стать проблемой» | `WARN` | + | сбой БД / ФС / недоступность зависимости | команде, в разбор | `ERROR` | + + Тот же класс отказа в **асинхронной стадии** (пользователь не ждёт: авто- + раскладка, поллинг) адресован уже команде как деградация автоматики — уровень + поднимается. Пример: `layout.ErrCollision` в ручном `Apply` — `DEBUG` (человек + видит причину в карточке), а в авто-раскладке — `WARN` («auto-apply failed, + left for review»): автоматика не довела задачу, это «может стать проблемой». + +- **Повторяющийся сбой фонового цикла (поллинг/сверка) — `WARN`, не `ERROR`.** + Одиночный промах тика (`poll`/`sweep`/`list failed`, недоступный + qBittorrent) транзиентен: следующий тик повторит. Тот же класс сбоя внутри + синхронной операции (`ingest.Ingest`) — `ERROR`, потому что операция + провалилась целиком и повтора нет. То есть уровень задаёт не текст ошибки, а + наличие штатного ретрая: тик повторится → `WARN`, разовая операция упала → + `ERROR`. (Устойчивый сбой N тиков подряд эскалировать в `ERROR` — на будущее, + сейчас не реализовано.) - Телеметрия внешнего вызова (`ext.*`, см. ниже) — отдельная запись о поведении зависимости, не дубль доменной ошибки. - Глушить ошибку без лога — только с однострочным комментарием «почему». @@ -206,6 +244,15 @@ log.Error(err.Error()) быть большим) — только на `DEBUG`, с вычисткой секретов и обрезкой по длине. - При сомнении — не логируем значение, логируем факт его наличия (`"has_api_key", true`). +- **Ошибка HTTP-транспорта несёт URL — потенциальный носитель секрета.** + `*url.Error` (стандартный `net/http`) встраивает полный URL запроса, а + секрет может жить прямо в нём: токен Telegram в пути (`…/bot/…`), + `api_key` метабазы в query. Санитизируем на границе клиента **до** лога и + обёртки — `logging.SanitizeErr(err)` разворачивает `*url.Error` в + первопричину (URL отбрасывается, `errors.Is` на причину сохраняется). + Применяется в `ext.*`-обёртке (`ExtCall`), клиентах metadata и tgbot. Общее + правило: **секрет не кладём в URL, если у API есть заголовок** — тогда его + нет и в ошибке транспорта. ## Куда пишем и уровень diff --git a/internal/httpapi/download.go b/internal/httpapi/download.go index e644c9a..1d2aab8 100644 --- a/internal/httpapi/download.go +++ b/internal/httpapi/download.go @@ -81,7 +81,7 @@ func (s *server) handleDownload(w http.ResponseWriter, r *http.Request) { http.Error(w, "задача не найдена", http.StatusNotFound) return } - s.deps.Logger.Error("download detail data", "id", id, "error", err) + s.deps.Logger.Error("download detail data failed", "download_id", id, "error", err) http.Error(w, "внутренняя ошибка", http.StatusInternalServerError) return } diff --git a/internal/httpapi/httpapi.go b/internal/httpapi/httpapi.go index e550ae9..9b1353e 100644 --- a/internal/httpapi/httpapi.go +++ b/internal/httpapi/httpapi.go @@ -24,6 +24,7 @@ import ( "git.vakhrushev.me/av/jellybit/internal/ident" "git.vakhrushev.me/av/jellybit/internal/ingest" + "git.vakhrushev.me/av/jellybit/internal/layout" "git.vakhrushev.me/av/jellybit/internal/magnet" "git.vakhrushev.me/av/jellybit/internal/store" "git.vakhrushev.me/av/jellybit/internal/torrent" @@ -747,20 +748,29 @@ func writeJSON(w http.ResponseWriter, status int, v any) { // classifyErr транслирует доменную ошибку в HTTP-статус и нейтральное // человекочитаемое сообщение публичного канала (без сырого err.Error() и -// деталей реализации): ErrNotFound → 404, валидация источника -// (magnet.ErrNotMagnet) → 400, недокачанный источник (worker.ErrNotReady) и -// конфликт состояния (worker.ErrConflict) → 409, прочее → 500. Полная ошибка -// уже в логах на доменной границе — наружу отдаём только сообщение + -// корреляционный ключ. +// деталей реализации): ErrNotFound → 404; валидация источника +// (magnet.ErrNotMagnet) и некорректный ввод команды (worker.ErrInvalidInput) → +// 400; недокачанный источник (worker.ErrNotReady), коллизия цели +// (layout.ErrCollision) и конфликт состояния (worker.ErrConflict) → 409; прочее +// → 500. Полная ошибка уже в логах на доменной границе — наружу отдаём только +// сообщение + корреляционный ключ. func classifyErr(err error) (int, string) { switch { case errors.Is(err, store.ErrNotFound): return http.StatusNotFound, "не найдено" case errors.Is(err, magnet.ErrNotMagnet), errors.Is(err, torrent.ErrNotTorrent): return http.StatusBadRequest, "некорректный источник" + case errors.Is(err, worker.ErrInvalidInput): + // Промах пользователя (пустая подсказка, неизвестный тип/провайдер, …), + // не сбой сервера. + return http.StatusBadRequest, "некорректный ввод" case errors.Is(err, worker.ErrNotReady): // Источник ещё качается — actionable причина, показываем конкретно. return http.StatusConflict, "торрент ещё качается, дождитесь докачки" + case errors.Is(err, layout.ErrCollision): + // Целевой путь уже занят: задача штатно ушла в review с причиной — + // это не сбой, а требующий разбора конфликт. + return http.StatusConflict, "целевой файл уже существует, задача отправлена в ревью" case errors.Is(err, worker.ErrConflict): // Нормальный конфликт состояния (операция недопустима сейчас), не сбой. return http.StatusConflict, "действие недоступно в текущем состоянии" diff --git a/internal/httpapi/httpapi_test.go b/internal/httpapi/httpapi_test.go index c66969e..9acd3cf 100644 --- a/internal/httpapi/httpapi_test.go +++ b/internal/httpapi/httpapi_test.go @@ -247,6 +247,41 @@ func TestAPICommandNotReady(t *testing.T) { } } +func TestAPICommandInvalidInput(t *testing.T) { + // Промах пользователя (worker.ErrInvalidInput) → 400, не 500. + cmd := &fakeCommander{err: fmt.Errorf("set type: invalid type %q: %w", "foo", worker.ErrInvalidInput)} + srv := newServer(t, httpapi.Deps{Ingestor: &fakeIngestor{}, Commander: cmd, Reader: &fakeReader{}}) + + resp, err := http.Post(srv.URL+"/api/downloads/"+tid+"/cancel", "", nil) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", resp.StatusCode) + } +} + +func TestAPICommandCollision(t *testing.T) { + // Коллизия цели (layout.ErrCollision) → 409 (штатно ушло в review), не 500. + cmd := &fakeCommander{err: fmt.Errorf("apply: %w", layout.ErrCollision)} + srv := newServer(t, httpapi.Deps{Ingestor: &fakeIngestor{}, Commander: cmd, Reader: &fakeReader{}}) + + resp, err := http.Post(srv.URL+"/api/downloads/"+tid+"/cancel", "", nil) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusConflict { + t.Fatalf("status = %d, want 409", resp.StatusCode) + } + var got map[string]any + _ = json.NewDecoder(resp.Body).Decode(&got) + if msg, _ := got["error"].(string); !strings.Contains(msg, "уже существует") { + t.Errorf("error = %q, want содержащее «уже существует»", msg) + } +} + func TestIndexRenders(t *testing.T) { reader := &fakeReader{list: []store.Download{ {ID: tid, SourceType: store.SourceMagnet, SourceRef: "magnet:?xt=urn:btih:abc", State: store.StateDownloading}, diff --git a/internal/httpapi/review.go b/internal/httpapi/review.go index 81288af..466afe3 100644 --- a/internal/httpapi/review.go +++ b/internal/httpapi/review.go @@ -87,7 +87,7 @@ func (s *server) handleReview(w http.ResponseWriter, r *http.Request) { http.Error(w, "задача не найдена", http.StatusNotFound) return } - s.deps.Logger.Error("review data", "id", id, "error", err) + s.deps.Logger.Error("review data failed", "download_id", id, "error", err) http.Error(w, "внутренняя ошибка", http.StatusInternalServerError) return } @@ -383,7 +383,7 @@ func (s *server) reviewAction(w http.ResponseWriter, r *http.Request, fn func(co rd, err := s.deps.Reviewer.ReviewData(r.Context(), id) if err != nil { - s.deps.Logger.Error("review data", "id", id, "error", err) + s.deps.Logger.Error("review data failed", "download_id", id, "error", err) http.Error(w, "внутренняя ошибка", http.StatusInternalServerError) return } @@ -443,7 +443,7 @@ func (s *server) reviewBlockAction(w http.ResponseWriter, r *http.Request, fn fu // и рендерим свежий партиал блока. rd, err := s.deps.Reviewer.ReviewData(r.Context(), id) if err != nil { - s.deps.Logger.Error("review data", "id", id, "error", err) + s.deps.Logger.Error("review data failed", "download_id", id, "error", err) http.Error(w, "внутренняя ошибка", http.StatusInternalServerError) return } diff --git a/internal/tgbot/bot.go b/internal/tgbot/bot.go index aa45e41..9fae1ec 100644 --- a/internal/tgbot/bot.go +++ b/internal/tgbot/bot.go @@ -15,6 +15,7 @@ import ( "git.vakhrushev.me/av/jellybit/internal/ident" "git.vakhrushev.me/av/jellybit/internal/ingest" + "git.vakhrushev.me/av/jellybit/internal/layout" "git.vakhrushev.me/av/jellybit/internal/logging" "git.vakhrushev.me/av/jellybit/internal/worker" ) @@ -345,6 +346,14 @@ func (b *Bot) handleCallback(ctx context.Context, cq *tgbotapi.CallbackQuery) { b.send(chatID, opErr("Торрент ещё качается — дождитесь докачки", id), nil) return } + if errors.Is(err, layout.ErrCollision) { + // Коллизия цели: задача штатно ушла в review с причиной — показываем + // конкретно и обновляем карточку (в ней теперь причина коллизии). + b.answer(cq.ID, "Коллизия цели") + b.send(chatID, opErr("Целевой файл уже существует — задача отправлена в ревью", id), nil) + b.refreshCard(ctx, chatID, msgID, id) + return + } // Ошибку логирует доменная граница (соответствующая команда worker); // транспорт лишь переводит её в ответ пользователю (logging.md). b.answer(cq.ID, "Ошибка") diff --git a/internal/worker/errors.go b/internal/worker/errors.go index 96980be..1e988f6 100644 --- a/internal/worker/errors.go +++ b/internal/worker/errors.go @@ -13,3 +13,10 @@ var ErrConflict = errors.New("conflict") // от ErrConflict (тоже 409), потому что причина actionable — «дождись докачки» // — и транспорт показывает её конкретным текстом, а не генериком конфликта. var ErrNotReady = errors.New("source not ready") + +// ErrInvalidInput — команда отклонена из-за некорректного пользовательского +// ввода (пустая подсказка, неизвестный тип/провайдер, пустой id, кандидат не из +// текущей рекогниции). Это промах пользователя, а не сбой сервера: транспорт +// матчит его через errors.Is и отвечает 400, а логирующая граница домена пишет +// DEBUG (адресат — пользователь, он уже получил ответ на поверхности). +var ErrInvalidInput = errors.New("invalid input") diff --git a/internal/worker/reconcile.go b/internal/worker/reconcile.go index 6323ca6..3f0c168 100644 --- a/internal/worker/reconcile.go +++ b/internal/worker/reconcile.go @@ -208,7 +208,9 @@ func (w *Worker) reconcileOneRecovery(ctx context.Context, d store.Download, byH logctx.From(ctx).Warn("recovery activate failed", "error", err) return } - logctx.From(ctx).Info("recovery from failure", "from", d.State, "to", want, "qbit_state", t.State) + // Восстановление из failed/stuck — тоже переход состояния: единый msg + // `state transition` (from/to), qbit_state — отличительная деталь авто-воскрешения. + logctx.From(ctx).Info("state transition", "from", d.State, "to", want, "qbit_state", t.State) } // torrentProgressed сообщает, продвинулся ли торрент за условие, по которому diff --git a/internal/worker/review.go b/internal/worker/review.go index 1901e19..af64cfd 100644 --- a/internal/worker/review.go +++ b/internal/worker/review.go @@ -286,7 +286,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize // живого якоря; рассинхрон (несколько разных живых папок) → review. folderBase, desync, err := w.resolveFolderBase(ctx, d.ID, provider, providerID, layout.MediaType(plan.Type)) if err != nil { - w.transition(ctx, *d, store.StateReview, "resolve", err.Error()) + w.transition(ctx, *d, store.StateReview, reasonResolve, err.Error()) return fmt.Errorf("link plan: %w", err) } if desync { @@ -296,7 +296,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize links, err := w.layouter.BuildLinks(toLayoutPlan(plan, savePath, providerTag(provider, providerID), folderBase)) if err != nil { - w.transition(ctx, *d, store.StateReview, "build", err.Error()) + w.transition(ctx, *d, store.StateReview, reasonBuild, err.Error()) return fmt.Errorf("build links: %w", err) } @@ -323,7 +323,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize // файлы висели бы без file_link — MAJOR-4): уводим в review с // причиной. Повторный Apply идемпотентен — Apply вернёт StatusExists // на уже созданных ссылках и допишет учёт. - w.transition(ctx, *d, store.StateReview, "persist", err.Error()) + w.transition(ctx, *d, store.StateReview, reasonPersist, err.Error()) return fmt.Errorf("persist links: %w", err) } // Инвариант «один целевой путь — один владелец»: забираем владение @@ -346,7 +346,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize if applyErr != nil { if errors.Is(applyErr, layout.ErrCollision) { - w.transition(ctx, *d, store.StateReview, "collision", applyErr.Error()) + w.transition(ctx, *d, store.StateReview, reasonCollision, applyErr.Error()) return applyErr } w.transition(ctx, *d, store.StateFailed, "apply", applyErr.Error()) @@ -394,7 +394,7 @@ func (w *Worker) Relink(ctx context.Context, id string) (err error) { } return fmt.Errorf("relink: %w", err) } - logctx.From(ctx).Info("relink re-recognizing", "from", d.State) + logctx.From(ctx).Info("state transition", "from", d.State, "to", store.StateRecognizing) return nil } @@ -424,7 +424,7 @@ func (w *Worker) Refine(ctx context.Context, id string, hint string) (err error) defer func() { w.logCmd(ctx, "refine", id, err) }() hint = strings.TrimSpace(hint) if hint == "" { - return fmt.Errorf("refine: empty hint") + return fmt.Errorf("refine: empty hint: %w", ErrInvalidInput) } w.mu.Lock() defer w.mu.Unlock() @@ -450,7 +450,7 @@ func (w *Worker) Refine(ctx context.Context, id string, hint string) (err error) func (w *Worker) SetType(ctx context.Context, id string, mediaType string) (err error) { defer func() { w.logCmd(ctx, "set_type", id, err) }() if mediaType != string(recognize.MediaMovie) && mediaType != string(recognize.MediaSeries) { - return fmt.Errorf("set type: invalid type %q", mediaType) + return fmt.Errorf("set type: invalid type %q: %w", mediaType, ErrInvalidInput) } w.mu.Lock() defer w.mu.Unlock() @@ -483,7 +483,7 @@ func (w *Worker) IgnoreFile(ctx context.Context, id string, src string) (err err defer func() { w.logCmd(ctx, "ignore_file", id, err) }() src = strings.TrimSpace(src) if src == "" { - return fmt.Errorf("ignore: empty path") + return fmt.Errorf("ignore: empty path: %w", ErrInvalidInput) } w.mu.Lock() defer w.mu.Unlock() @@ -519,7 +519,7 @@ func (w *Worker) Defer(ctx context.Context, id string) (err error) { return fmt.Errorf("defer: %w", err) } if d.State.IsTerminal() { - return fmt.Errorf("defer: download %s is terminal (%s)", id, d.State) + return fmt.Errorf("defer: download %s is terminal (%s): %w", id, d.State, ErrConflict) } ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash()) w.transition(ctx, *d, store.StateDeferred, "", "") @@ -556,7 +556,7 @@ func (w *Worker) Undo(ctx context.Context, id string) (err error) { return fmt.Errorf("undo: %w", err) } if batch == "" { - return fmt.Errorf("undo: nothing to revert") + return fmt.Errorf("undo: nothing to revert: %w", ErrConflict) } rows, err := w.store.ListFileLinksByBatch(ctx, batch) if err != nil { @@ -653,8 +653,9 @@ func (w *Worker) Delete(ctx context.Context, id string) (err error) { // (в) Терминальный deleted с пользовательским маркером инициатора // (отличает от reconcile-deleted, который кладёт "reconcile"). w.transition(ctx, *d, store.StateDeleted, "user_delete", "удалено пользователем") - logctx.From(ctx).Info("download deleted by user", - "from", d.State, "removed_links", removed, "code", "user_delete") + // Запись физического эффекта (снятые ссылки) сверх перехода: from/code уже в + // каноническом `state transition` выше — здесь только отличительное поле. + logctx.From(ctx).Info("download deleted by user", "removed_links", removed) return nil } @@ -693,7 +694,7 @@ func (w *Worker) ChooseCandidate(ctx context.Context, id, candidateID string) (e return fmt.Errorf("choose candidate: %w", err) } if rec == nil || cand == nil || cand.RecognitionID != rec.ID { - return fmt.Errorf("choose candidate: candidate %s does not belong to the current recognition", candidateID) + return fmt.Errorf("choose candidate: candidate %s does not belong to the current recognition: %w", candidateID, ErrInvalidInput) } return w.chooseCandidateLocked(ctx, id, d, rec, *cand) } @@ -708,10 +709,10 @@ func (w *Worker) AddManualSource(ctx context.Context, id, provider, providerID s switch provider { case "tmdb", "tvdb", "imdb": default: - return fmt.Errorf("add source: invalid provider %q (tmdb/tvdb/imdb)", provider) + return fmt.Errorf("add source: invalid provider %q (tmdb/tvdb/imdb): %w", provider, ErrInvalidInput) } if providerID == "" { - return fmt.Errorf("add source: empty id") + return fmt.Errorf("add source: empty id: %w", ErrInvalidInput) } w.mu.Lock() defer w.mu.Unlock() @@ -804,10 +805,10 @@ func (w *Worker) SetProviderID(ctx context.Context, id string, provider, provide switch provider { case "tmdb", "tvdb", "imdb": default: - return fmt.Errorf("set provider: invalid provider %q (tmdb/tvdb/imdb)", provider) + return fmt.Errorf("set provider: invalid provider %q (tmdb/tvdb/imdb): %w", provider, ErrInvalidInput) } if providerID == "" { - return fmt.Errorf("set provider: empty id") + return fmt.Errorf("set provider: empty id: %w", ErrInvalidInput) } w.mu.Lock() defer w.mu.Unlock() @@ -957,7 +958,9 @@ func (w *Worker) ReviewData(ctx context.Context, id string) (*ReviewData, error) if links, lerr := w.layouter.BuildLinks(toLayoutPlan(rd.Plan, "", tag, base)); lerr == nil { rd.Preview = links } else { - log.Debug("review data build preview failed", "error", lerr) + // Видимая деградация: без превью на экране ревью пропадает + // кнопка «Применить» — не рядовой Debug, а WARN. + log.Warn("review data build preview failed", "error", lerr) } } // Единый список источников: нейронка + кандидаты, каждый с @@ -1061,9 +1064,16 @@ func (w *Worker) effectivePlan(ctx context.Context, id string) (plan recognize.P return applyOverrides(plan, overrides), prov, pid, nil } -// reasonTitleFolderDesync — код причины ухода в review, когда у тайтла нашлось -// несколько разных живых папок с одним матчем (правило сходимости папки). -const reasonTitleFolderDesync = "title_folder_desync" +// Коды причины (error_code) ухода задачи в review при раскладке (linkPlan) — +// корреляционный ключ шага, на котором раскладка остановилась. Свод в одном +// месте (как errCode* в worker.go); человекочитаемый текст кладётся в error_msg. +const ( + reasonResolve = "resolve" // не удалось разрешить базу папки тайтла + reasonBuild = "build" // не удалось построить план ссылок + reasonPersist = "persist" // ссылки на диске, но учёт не записан + reasonCollision = "collision" // целевой путь уже занят (layout.ErrCollision) + reasonTitleFolderDesync = "title_folder_desync" // ≥2 разных живых папок тайтла с одним матчем +) // resolveFolderBase применяет правило сходимости папки (см. file-layout spec): // при подтверждённом матче наследует базу имени от живой папки-якоря того же @@ -1270,6 +1280,10 @@ func parseIgnored(s string) []string { return nil } var out []string + // Ошибку разбора глотаем намеренно: битый JSON в override ignored_files + // (не должен возникать — пишем его сами через json.Marshal) трактуем как + // «нет игнора», а не роняем команду. Худший исход — файл не будет пропущен, + // человек увидит его в превью и пометит заново. _ = json.Unmarshal([]byte(s), &out) return out } diff --git a/internal/worker/worker.go b/internal/worker/worker.go index d001662..82d208e 100644 --- a/internal/worker/worker.go +++ b/internal/worker/worker.go @@ -791,7 +791,8 @@ func (w *Worker) logCmd(ctx context.Context, cmd, id string, err error) { } log := logctx.FromOr(ctx, w.log) switch { - case errors.Is(err, ErrConflict), errors.Is(err, ErrNotReady), errors.Is(err, store.ErrNotFound): + case errors.Is(err, ErrConflict), errors.Is(err, ErrNotReady), errors.Is(err, ErrInvalidInput), + errors.Is(err, store.ErrNotFound), errors.Is(err, layout.ErrCollision): log.Debug("command rejected", "command", cmd, "download_id", id, "error", err) default: log.Error("command failed", "command", cmd, "download_id", id, "error", err) @@ -810,12 +811,13 @@ func (w *Worker) Cancel(ctx context.Context, id string) (err error) { return fmt.Errorf("cancel: %w", err) } if d.State.IsTerminal() { - return fmt.Errorf("cancel: download %s is already terminal (%s)", id, d.State) + return fmt.Errorf("cancel: download %s is already terminal (%s): %w", id, d.State, ErrConflict) } if err := w.store.SetDownloadState(ctx, id, store.StateCancelled, "", ""); err != nil { return fmt.Errorf("cancel: %w", err) } - logctx.From(w.scoped(ctx, capReview, id, d.PrimaryInfohash())).Info("download cancelled", "from", d.State) + logctx.From(w.scoped(ctx, capReview, id, d.PrimaryInfohash())).Info("state transition", + "from", d.State, "to", store.StateCancelled) return nil } @@ -831,7 +833,7 @@ func (w *Worker) Retry(ctx context.Context, id string) (err error) { return fmt.Errorf("retry: %w", err) } if d.State != store.StateFailed && d.State != store.StateStuck { - return fmt.Errorf("retry: download %s is %s, only failed/stuck are retriable", id, d.State) + return fmt.Errorf("retry: download %s is %s, only failed/stuck are retriable: %w", id, d.State, ErrConflict) } // Если раздача уже жива и ЗДОРОВА в qBittorrent — перецепляемся к ней, // повторный Add не нужен (и вреден: вслепую дублировал бы торрент). Add — @@ -901,7 +903,8 @@ func (w *Worker) Retry(ctx context.Context, id string) (err error) { "capability", capReview, "download_id", id, "error", err) } } - logctx.From(w.scoped(ctx, capReview, id, d.PrimaryInfohash())).Info("download retried", "from", d.State) + logctx.From(w.scoped(ctx, capReview, id, d.PrimaryInfohash())).Info("state transition", + "from", d.State, "to", store.StateDownloading) return nil }