доменные ошибки сравниваются через errors.As, отказ Close не теряется
- признаки «работы нет» и «задача не найдена» узнаются по смыслу, а не приведением типа: обёртка `%w` на пути больше не превращает пустой прогон воркера в отказ раз в секунду - отказ закрытия соединения с распознавателем доходит до вызывающего (`errors.Join`) либо до журнала; у `errcheck` включён `check-blank`, иначе критерий принимал реализацию, выбрасывающую отказ в пустоту - заведены первые тесты пакета worker и capability `pipeline`; долг из четырёх замечаний линтера закрыт, гейт зелёный целиком
This commit is contained in:
@@ -6,6 +6,10 @@ linters:
|
||||
- errorlint
|
||||
settings:
|
||||
errcheck:
|
||||
# Без этого `_ = x.Close()` снимает замечание, и критерий «отказ не
|
||||
# теряется молча» принимается реализацией, которая его теряет. Отказ,
|
||||
# который решено не проверять, теперь объявляют ниже поимённо — заметно.
|
||||
check-blank: true
|
||||
exclude-functions:
|
||||
# Закрытие через defer и лучшая-попытка уборки файла — осознанно без проверки
|
||||
- (io.Closer).Close
|
||||
|
||||
@@ -102,23 +102,19 @@ task gate # весь набор проверок разом
|
||||
их скилл `av-dev-docs:healthcheck`, и звать его надо руками;
|
||||
- покрытие изменённых строк не считается ничем.
|
||||
|
||||
**Гейт на `master` сегодня красный, и это объявленный долг, а не поломка дня.**
|
||||
Известный отказ один:
|
||||
**Гейт на `master` сегодня зелёный целиком, и объявленных долгов у него нет.**
|
||||
Красный шаг означает поломку — свою или чужую, но поломку, а не наследство.
|
||||
Списывать отказ на долг больше нельзя: списывать не на что.
|
||||
|
||||
- `golangci-lint run` даёт 4 замечания в существующем коде: два непроверенных
|
||||
`Close` (`adapter/recognizer/yandex/speechkit.go:55`, `main.go:124`) и два
|
||||
сравнения ошибок приведением типа (`controller/worker/worker.go:51`,
|
||||
`service/transcribe.go:394`). Долг записан в
|
||||
[docs/conventions/errors.md](docs/conventions/errors.md), заведён задачей
|
||||
`errors-as-instead-of-typecast`.
|
||||
Два прежних долга закрыты и здесь названы, чтобы отказ на их месте читался как
|
||||
новый:
|
||||
|
||||
Новые отказы отличай от этого. Пока он жив, «зелёный гейт» в определении
|
||||
сделанного означает «не добавилось ничего сверх перечисленного».
|
||||
|
||||
**`go test ./...` больше долгом не считается.** Задача
|
||||
`http-handler-tests-never-green` починила тесты приёма по HTTP; красный
|
||||
`go test` теперь означает
|
||||
поломку, и списывать его на наследство нельзя.
|
||||
- `golangci-lint run` давал 4 замечания — два непроверенных `Close` и два
|
||||
сравнения ошибок приведением типа. Закрыто задачей
|
||||
`errors-as-instead-of-typecast` 2026-08-11; тогда же у `errcheck` включена
|
||||
настройка `check-blank`, поэтому `_ = x.Close()` больше не снимает замечание:
|
||||
отказ, который решено не проверять, объявляют в `exclude-functions` поимённо;
|
||||
- `go test ./...` чинила задача `http-handler-tests-never-green`.
|
||||
|
||||
## Запреты
|
||||
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
# ADR-2026-08-11. Границу распознавания доменного признака держит норма, а не код
|
||||
|
||||
- **Дата:** 2026-08-11
|
||||
- **Источник:** [openspec/changes/archive/2026-08-11-errors-as-instead-of-typecast/design.md](../../openspec/changes/archive/2026-08-11-errors-as-instead-of-typecast/design.md), раздел `Decisions`, Решение 3
|
||||
|
||||
## Решение
|
||||
|
||||
Признак «работы нет» узнаётся через `errors.As`, то есть на любой глубине цепочки
|
||||
ошибки. Встречный риск — отказ, к которому признак примешался по дороге, — закрыт
|
||||
**требованием спеки**, а не проверкой в коде воркера.
|
||||
|
||||
Дословно из источника:
|
||||
|
||||
> Граница ставится **нормой, а не кодом**: спека требует, чтобы признак рождался
|
||||
> только ответом хранилища на опрос этим же шагом, и запрещает слою сохранять
|
||||
> чужой признак в цепочке своей ошибки.
|
||||
|
||||
## Почему
|
||||
|
||||
Приведение типа видело только вершину цепочки — потому и ломалось от первой же
|
||||
обёртки. `errors.As` эту проблему устраняет, но устраняет симметрично: признак
|
||||
теперь виден и там, где его никто не клал осознанно. Отказ, к которому признак
|
||||
примешался обёрткой или `errors.Join`, воркер зачёл бы пустым прогоном — задача
|
||||
осталась бы в своём состоянии и переопрашивалась раз в секунду без единой записи.
|
||||
Это тот же класс, от которого защищает инвариант «Принятая запись не теряется
|
||||
молча», только с обратным знаком относительно чинимого дефекта.
|
||||
|
||||
Кодовый вариант рассмотрен и отвергнут по цене:
|
||||
|
||||
> **Рассмотрено и отвергнуто — научить воркер различать «признак на вершине» от
|
||||
> «признака в глубине».** Отвергнуто по цене: `errors.As` такого различения не
|
||||
> даёт вовсе, пришлось бы либо проверять вершину вручную (то есть вернуть
|
||||
> приведение типа, которое чинится), либо заводить свой обход цепочки. Код
|
||||
> усложняется ради случая, которого сегодня нет ни одного, а защита от него нужна
|
||||
> на входе — при написании нового слоя, — где норма работает, а проверка в
|
||||
> рантайме опоздала бы.
|
||||
|
||||
## Последствия
|
||||
|
||||
- `+` код остался коротким: одна проверка вместо разбора цепочки вручную.
|
||||
- `+` защита стоит там, где ошибку совершают, — за письменным столом автора
|
||||
нового слоя, а не в рантайме, где она уже случилась.
|
||||
- `−` норма не механизирована: её нарушение поймает только ревью или чтение.
|
||||
Единственный `MUST` требования `pipeline` без машинного оракула — этот.
|
||||
- `−` правило живёт в двух документах: требованием в
|
||||
[openspec/specs/pipeline/spec.md](../../openspec/specs/pipeline/spec.md) и
|
||||
прозой в [conventions/errors.md](../conventions/errors.md), где оно нужно
|
||||
автору в момент письма. Второй адрес ссылается на первый и нормой не является —
|
||||
разойтись они могут только правкой, сделанной мимо спеки.
|
||||
@@ -0,0 +1,54 @@
|
||||
# ADR-2026-08-11. Отказ, который решено не проверять, объявляется поимённо
|
||||
|
||||
- **Дата:** 2026-08-11
|
||||
- **Источник:** [openspec/changes/archive/2026-08-11-errors-as-instead-of-typecast/design.md](../../openspec/changes/archive/2026-08-11-errors-as-instead-of-typecast/design.md), раздел `Decisions`, Решение 2
|
||||
|
||||
## Решение
|
||||
|
||||
У `errcheck` включена настройка `check-blank`: присваивание отказа в `_` больше
|
||||
не снимает замечание линтера. Место, где отказ решено не проверять, вносится в
|
||||
`exclude-functions` поимённо.
|
||||
|
||||
Дословно из источника:
|
||||
|
||||
> Правило `errcheck` сегодня молчит на `_ = conn.Close()`: настройка
|
||||
> `check-blank` не выставлена, а её умолчание — «пропускать». То есть
|
||||
> реализация, выбрасывающая отказ в пустоту, удовлетворяет критерию приёмки
|
||||
> «линтер не даёт замечаний `errcheck`», не удовлетворяя самому критерию —
|
||||
> «отказ возвращается либо попадает в журнал».
|
||||
|
||||
## Почему
|
||||
|
||||
Решение принято не ради строгости, а потому что **оракул не мог упасть**. Задача
|
||||
`errors-as-instead-of-typecast` закрывала два непроверенных `Close`, и её
|
||||
критерий приёмки опирался на молчание линтера. Ревью дизайна показало, что этому
|
||||
критерию удовлетворяет и негодная реализация: `_ = conn.Close()` теряет отказ
|
||||
целиком, а линтер молчит. Критерий, который нельзя уронить, не проверяет ничего —
|
||||
и вместе с ним в `CLAUDE.md` снималась запись о долге, то есть сигнал исчез бы
|
||||
навсегда и без следа.
|
||||
|
||||
Очевидный путь был другим и отвергнут намеренно:
|
||||
|
||||
> **Рассмотрено и отвергнуто — дописать оба типа в `exclude-functions`
|
||||
> `.golangci.yml`.** Соблазн сильный: список исключений там уже есть, и в нём
|
||||
> записана ровно эта политика […] Отвергнуто: политика в конфиге относится к
|
||||
> закрытию, у которого **отказ ничего не значит** […] Записав их в исключения,
|
||||
> мы бы расширили политику молча, самим фактом добавления строки, и потеряли бы
|
||||
> оба сигнала навсегда.
|
||||
|
||||
Включение проверено прогоном до правки кода: на тогдашнем коде правило не давало
|
||||
ни одного нового замечания, то есть включалось чисто и отдельного коммита
|
||||
приведения не требовало.
|
||||
|
||||
## Последствия
|
||||
|
||||
- `+` критерий «отказ не теряется молча» стал проверяемым машиной: мутация
|
||||
(замена обоих мест на `_ = …Close()`) роняет линтер — проверено прогоном.
|
||||
- `+` умолчание сместилось в сторону заметности: спрятать отказ по месту больше
|
||||
нельзя, отказ от проверки виден в одном файле списком.
|
||||
- `−` осознанное игнорирование подорожало: вместо одного символа `_` нужна строка
|
||||
в `exclude-functions` с полным именем метода. Для одноразового случая это
|
||||
заметная церемония.
|
||||
- `−` список исключений будет расти, и каждая его строка — это политика на весь
|
||||
проект, а не на одно место. Разрастание списка — сигнал, что правило выбрано
|
||||
неверно, и повод пересмотреть эту запись.
|
||||
@@ -32,6 +32,8 @@
|
||||
|
||||
| Дата | Запись | Статус |
|
||||
| --- | --- | --- |
|
||||
| 2026-08-11 | [Границу распознавания доменного признака держит норма, а не код](ADR-2026-08-11-domain-marker-boundary-by-norm.md) | |
|
||||
| 2026-08-11 | [Отказ, который решено не проверять, объявляется поимённо](ADR-2026-08-11-errcheck-check-blank.md) | |
|
||||
| 2026-08-11 | [Наружу расширение выходит только приведённым к перечню](ADR-2026-08-11-known-format-label.md) | |
|
||||
| 2026-08-11 | [Приложение пишем на Vue, а Node входит в гейт и в образ](ADR-2026-08-11-spa-on-vue.md) | |
|
||||
| 2026-08-11 | [Очередь остаётся своей таблицей, но коллекцией PocketBase](ADR-2026-08-11-queue-as-pocketbase-collection.md) | |
|
||||
|
||||
+15
-7
@@ -8,11 +8,19 @@
|
||||
[passport.md](passport.md) и в [tasks/ROADMAP.md](../tasks/ROADMAP.md); что из
|
||||
этого ещё не решено — в разделе «Открытые вопросы».
|
||||
|
||||
Заведена одна capability — [intake](../openspec/specs/intake/spec.md), и в ней
|
||||
описан **только приём по HTTP**: его нормируют проверки, написанные задачей
|
||||
`http-handler-tests-never-green` 2026-08-11. Поведение прочих узлов, включая
|
||||
приём из Telegram, по-прежнему живёт только в коде. Задача, которая его трогает,
|
||||
дописывает спеку своей capability.
|
||||
Заведены две capability, и каждая описана частично:
|
||||
|
||||
- [intake](../openspec/specs/intake/spec.md) — **только приём по HTTP**: его
|
||||
нормируют проверки, написанные задачей `http-handler-tests-never-green`
|
||||
2026-08-11;
|
||||
- [pipeline](../openspec/specs/pipeline/spec.md) — **только пустой прогон
|
||||
воркера**: задача `errors-as-instead-of-typecast` 2026-08-11. Переходы
|
||||
состояний, захват и срок его протухания, отмена контекста посреди шага в неё
|
||||
**не** переехали и остаются долгом; что именно не описано, перечисляет раздел
|
||||
`Purpose` самой спеки.
|
||||
|
||||
Поведение прочих узлов, включая приём из Telegram, по-прежнему живёт только в
|
||||
коде. Задача, которая его трогает, дописывает спеку своей capability.
|
||||
|
||||
## Принципы
|
||||
|
||||
@@ -24,7 +32,7 @@
|
||||
решено 2026-08-11,
|
||||
[ADR](adr/ADR-2026-08-11-queue-as-pocketbase-collection.md), сравнение
|
||||
кандидатов в [research/job-queue.md](research/job-queue.md).
|
||||
<!-- канон: поведение → openspec/specs/pipeline -->
|
||||
<!-- канон: поведение → openspec/specs/pipeline; ещё НЕ переехало: спека заведена, но это в ней не описано -->
|
||||
- **Шаг конвейера идемпотентен по повтору.** Задача, брошенная на середине,
|
||||
достаётся снова по истечении срока захвата и проходит шаг заново.
|
||||
- **Ядро зависит от интерфейсов.** `internal/service` знает только
|
||||
@@ -48,7 +56,7 @@
|
||||
| Отправитель Telegram | `internal/adapter/telegram` | Отправка текста, деление длинного по словам |
|
||||
| Репозитории | `internal/adapter/repo/sqlite` | Задачи и файлы, запросы через goqu |
|
||||
|
||||
<!-- канон: поведение → openspec/specs/pipeline -->
|
||||
<!-- канон: поведение → openspec/specs/pipeline; ещё НЕ переехало: спека заведена, но это в ней не описано -->
|
||||
|
||||
Конвейер: `created` → `converted` → `transcribe` → `done` либо `failed`. Три
|
||||
воркера двигают по одному переходу, каждый опрашивает базу раз в секунду.
|
||||
|
||||
@@ -18,7 +18,11 @@ severity — в [CLAUDE.md](../../CLAUDE.md).
|
||||
Четыре записи перенесены из проекта jellybit — тот же Go, тот же автор, те же
|
||||
задачи. Код transcriber написан раньше и **части правил не следует**: ключи —
|
||||
UUID вместо ULID, время берётся `time.Now()` по месту, лог пишется на каждом
|
||||
шаге и дублируется воркером, доменные ошибки проверяются приведением типа.
|
||||
шаге и дублируется воркером.
|
||||
|
||||
Из этого перечня одно уже закрыто: доменные ошибки проверялись приведением типа
|
||||
до 2026-08-11, задача `errors-as-instead-of-typecast`. Приведение типа на этом
|
||||
месте больше не долг, а регрессия.
|
||||
|
||||
Пятая, `web-ui.md`, тоже пришла оттуда, но не прижилась: jellybit работает на
|
||||
htmx, а здесь решено делать SPA — и перенесённый текст снят целиком.
|
||||
@@ -51,7 +55,7 @@ htmx, а здесь решено делать SPA — и перенесённы
|
||||
| Правило | Где механизировано |
|
||||
| --- | --- |
|
||||
| Сравнение ошибок через `errors.Is` и `errors.As`, не `==` и не приведением типа | `.golangci.yml` → `errorlint` |
|
||||
| Непроверенное возвращаемое значение ошибки | `.golangci.yml` → `errcheck` (кроме `defer Close` и `send`) |
|
||||
| Непроверенное возвращаемое значение ошибки | `.golangci.yml` → `errcheck`, включая присваивание в `_` (`check-blank`). Отказ, который решено не проверять, объявляют в `exclude-functions` поимённо — там сегодня `defer Close` и `send` |
|
||||
| Форматирование исходников | `.golangci.yml` → `gofmt` |
|
||||
| Подозрительные конструкции языка | `.golangci.yml` → `govet`, `staticcheck`, `ineffassign`, `unused` |
|
||||
| Секреты в коммите | `lefthook.yml` → `gitleaks git --staged` |
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
|
||||
**Взято из проекта jellybit.** Расхождения с сегодняшним кодом названы по месту.
|
||||
Главное: единой точки отображения доменной ошибки в ответ нет, обработчики
|
||||
решают сами, а доменные ошибки проверяются приведением типа, а не `errors.As`.
|
||||
решают сами.
|
||||
|
||||
**Механизировано:** приведение типа и `err == ErrX` ловит `errorlint` в
|
||||
`.golangci.yml`. Запрета сторонних пакетов ошибок (`depguard`) нет — сторонних
|
||||
@@ -48,14 +48,13 @@ transcriber — **приложение, а не библиотека**: внеш
|
||||
`sql.ErrNoRows` превращается в доменную ошибку в слое репозитория, чтобы выше
|
||||
по коду не торчал `database/sql`.
|
||||
- Проверяем `errors.Is` и `errors.As`, а не сравнением и не приведением типа.
|
||||
|
||||
*Расхождение, и оно опасно:* `NoopJobError` и `JobNotFoundError` проверяются
|
||||
приведением типа — `err.(*contract.NoopJobError)` в
|
||||
`internal/controller/worker/worker.go` и `err.(*contract.JobNotFoundError)` в
|
||||
`internal/service/transcribe.go`. Работает это только потому, что на этом пути
|
||||
ошибку никто не оборачивает. Первый же `fmt.Errorf("…: %w")` между ними сломает
|
||||
проверку молча: воркер перестанет отличать «задач нет» от отказа и начнёт
|
||||
считать пустой прогон ошибкой раз в секунду.
|
||||
- **Признак домена читается только из ответа того шага, который его породил.**
|
||||
`errors.As` распознаёт признак на любой глубине цепочки, а не только сверху,
|
||||
— поэтому слой, придающий отказу собственный смысл, чужой признак в свою
|
||||
цепочку не сохраняет. Иначе воркер примет отказ, к которому признак
|
||||
примешался, за этот признак: зачтёт настоящий сбой пустым прогоном, и задача
|
||||
продолжит переопрашиваться без единой записи в журнале. Норма записана требованием
|
||||
[pipeline](../../openspec/specs/pipeline/spec.md).
|
||||
|
||||
## Sentinel и типизированные
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ SpeechKit, Yandex Object Storage и `ffmpeg`. Мерить нужно то, чт
|
||||
|
||||
| Дата | Запись | О чём |
|
||||
| --- | --- | --- |
|
||||
| 2026-08-11 | [gRPC-клиент SpeechKit: когда закрытие вообще может отказать](grpc-client-close.md) | Ленивое соединение и два исхода `Close` в grpc v1.74.2 |
|
||||
| 2026-08-11 | [Фреймворк приложения: Svelte, Vue и React на одном экране](spa-framework.md) | Размер собранной статики, цена шага сборки, что у трёх кандидатов одинаково |
|
||||
| 2026-08-11 | [Очередь задач: своя таблица против готовой библиотеки](job-queue.md) | Цена River и goqite в пакетах, захват одним запросом, чего нет для PocketBase |
|
||||
| 2026-08-11 | [PocketBase: что даёт панель администратора](pocketbase.md) | Записи, пользователи и файлы в панели версии 0.39.10 |
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
# gRPC-клиент SpeechKit: когда закрытие вообще может отказать
|
||||
|
||||
Отвечает на вопрос, возникший по ходу задачи `errors-as-instead-of-typecast`: что
|
||||
означает отказ `Close` у клиента SpeechKit и стоит ли писать его в журнал.
|
||||
Наблюдение понадобилось потому, что первая редакция кода и обоснования описывала
|
||||
этот отказ неверно — как признак недоступности Yandex.
|
||||
|
||||
## Как снималось
|
||||
|
||||
Не замером, а **чтением исходников** зависимости, зафиксированной в `go.mod`:
|
||||
`google.golang.org/grpc` версии **v1.74.2**. Смотрел два места в
|
||||
`clientconn.go` — конструктор клиента и метод `Close`. К Yandex ни разу не
|
||||
обратился: ни на живых ключах, ни на тестовых.
|
||||
|
||||
## Что выяснилось
|
||||
|
||||
- **`grpc.NewClient` соединения не открывает.** Клиент создаётся в состоянии
|
||||
ожидания, сеть трогается при первом вызове (`clientconn.go:145`). То есть на
|
||||
пути отказа конструктора — когда первый клиент создан, а второй нет — закрывать
|
||||
ещё нечего.
|
||||
- **`(*ClientConn).Close` возвращает ровно два исхода** (`clientconn.go:1142-1156`):
|
||||
`nil` либо `ErrClientConnClosing` — `codes.Canceled`, «grpc: the client
|
||||
connection is closing» (`clientconn.go:67`). Второй наступает **только при
|
||||
повторном закрытии** уже закрытого клиента.
|
||||
|
||||
## Что из этого следует для нас
|
||||
|
||||
Отказ `Close` в этом проекте означает **нашу ошибку — закрыли дважды**, а не сбой
|
||||
или недоступность Yandex. Поэтому запись в журнале при остановке процесса
|
||||
адресует владельца к нашему коду; так она и сформулирована.
|
||||
|
||||
Обработка отказа при этом оставлена в обоих местах, хотя сегодня он практически
|
||||
недостижим: она стоит одну строку и переживёт смену клиента, а её отсутствие
|
||||
пришлось бы обосновывать заново каждому читателю. Решение и его цена —
|
||||
[ADR](../adr/ADR-2026-08-11-errcheck-check-blank.md), обоснование целиком — в
|
||||
архивном
|
||||
[design.md](../../openspec/changes/archive/2026-08-11-errors-as-instead-of-typecast/design.md),
|
||||
Решение 2.
|
||||
|
||||
**Наблюдение привязано к версии.** Сменится мажорная версия `grpc` — перечень
|
||||
исходов `Close` надо перечитать, а не считать его прежним.
|
||||
+48
-6
@@ -63,15 +63,30 @@
|
||||
структуру, чьи теги и составляют проверяемый контракт, меняется вместе с ним
|
||||
и никогда не ловит поломку; такое судят по сырому виду ответа. Признак ищется
|
||||
мутацией: сломай проверяемое свойство и убедись, что тест краснеет (журнал,
|
||||
запись 2026-08-11).
|
||||
запись 2026-08-11);
|
||||
- **то же и об оракуле критерия приёмки, не только о тесте.** Критерий, чей
|
||||
единственный оракул — молчание линтера, годится ровно тогда, когда линтер
|
||||
краснеет на **всех** негодных реализациях; проверяется той же мутацией.
|
||||
Прецедент: «отказ `Close` не теряется молча» принимался молчанием `errcheck`,
|
||||
а тот пропускал `_ = conn.Close()` — реализацию, теряющую отказ целиком
|
||||
(журнал, запись 2026-08-11 про недостижимую норму; закрыто
|
||||
[решением](adr/ADR-2026-08-11-errcheck-check-blank.md));
|
||||
- **требование без сценария не имеет оракула** и потому не может быть нарушено
|
||||
заметно. Норма, которую нечем уронить, расходится с кодом молча — и расходится
|
||||
тем вернее, чем убедительнее написана (журнал, запись 2026-08-11).
|
||||
|
||||
### Типовые ложноположительные
|
||||
|
||||
- **«Воркер глотает ошибку `NoopJobError`».** Не дефект: этот тип означает «задач
|
||||
в этом состоянии нет», и `internal/controller/worker/worker.go` намеренно не
|
||||
логирует его и не считает в метрику. Настоящий дефект рядом другой — проверка
|
||||
идёт приведением типа и сломается при первой же обёртке; он уже записан в
|
||||
[conventions/errors.md](conventions/errors.md).
|
||||
логирует его и не считает в метрику. Норма записана требованием
|
||||
[pipeline](../openspec/specs/pipeline/spec.md).
|
||||
|
||||
**Оговорка, и она тут главная:** ложноположительным считается только само
|
||||
молчание воркера. Проверка **формы** узнавания ложноположительной не является:
|
||||
приведение типа на этом месте — настоящий дефект, закрытый 2026-08-11 задачей
|
||||
`errors-as-instead-of-typecast`. Появилось снова — это регрессия, и выбрасывать
|
||||
её как известную нельзя.
|
||||
- **«Захват задачи не в транзакции — гонка двух воркеров».** По построению её
|
||||
нет: три воркера читают три разных состояния, и одну строку они не делят.
|
||||
Механика захвата и её слабые места — [database.md](database.md),
|
||||
@@ -110,8 +125,9 @@
|
||||
`createTranscribeJob` — сегодня через него идут оба входа
|
||||
([architecture.md](architecture.md), «Единые точки проекта»).
|
||||
- `architecture`: не поехало ли поведение в `architecture.md` вместо спеки —
|
||||
заведена одна capability (`openspec/specs/intake`), поведение прочих узлов
|
||||
живёт в обзоре под маркерами долга, и соблазн дописать туда ещё максимальный.
|
||||
заведены две capability (`openspec/specs/intake` и `openspec/specs/pipeline`),
|
||||
и каждая описана частично. Поведение прочих узлов живёт в обзоре под маркерами
|
||||
долга, а соблазн дописать туда ещё — самый большой.
|
||||
- `conventions`: новая колонка правится во всех четырёх местах репозитория
|
||||
(CLAUDE.md, «Инварианты»).
|
||||
- `autotests`: покрыт ли изменённый шаг конвейера хоть одним тестом — сегодня
|
||||
@@ -181,6 +197,32 @@ API и имя не откатываются обратной правкой по
|
||||
поймать их было некому. У восстановленных нет поля «Чем воспроизведён», и
|
||||
выдумывать его задним числом нельзя.
|
||||
|
||||
## 2026-08-11 — норма требовала от сервиса недостижимого [пойман ревью]
|
||||
|
||||
- **Где:** дельта-спека `pipeline` задачи `errors-as-instead-of-typecast`, абзац
|
||||
об отказе шага
|
||||
- **Симптом:** требование гласило «отказ MUST быть записан **ровно один раз**
|
||||
единственной логирующей точкой». Сервис пишет дважды — сначала шаг конвейера,
|
||||
следом воркер, — то есть норма не выполнялась бы с первого дня, а после
|
||||
архивации стала бы посылкой для следующих задач
|
||||
- **Причина:** дефект родился при починке соседнего. Первая редакция назначала
|
||||
логирующей точкой воркера и фиксировала уровень `ERROR`, чем закрепляла
|
||||
контрактом долг `conventions/logging.md`. Правка по этой находке ушла в
|
||||
противоположную крайность: вместо «норма молчит о числе записей» получилось
|
||||
«норма требует одной». Двойная запись — записанный системный долг, и обе
|
||||
редакции с ним расходились, только в разные стороны
|
||||
- **Чем воспроизведён:** прогоном пробы через `go test -overlay`: один отказ
|
||||
хранилища даёт две записи — `Failed to find and acquire job` из шага и
|
||||
`Worker error` из воркера
|
||||
- **Почему не поймали раньше:** требование не имело сценария, а значит и оракула
|
||||
— упасть ему было нечем. Ревью дизайна абзац читало, но код с ним не сверяло:
|
||||
кода тогда не существовало. Поймал проход `specs` на ревью кода, направлением
|
||||
`code → spec`, и поймал прогоном, а не чтением
|
||||
- **Что меняем:** норма говорит только проверяемое сегодня — отказ виден
|
||||
владельцу и засчитан в счётчик; число записей и уровень названы долгом с
|
||||
адресом. В критерии приёмки добавлена строка: норма не объявляет обязательным
|
||||
недостижимое — ни в ту, ни в другую сторону
|
||||
|
||||
## 2026-08-11 — хвост имени отправителя уезжал на открытую страницу метрик [пойман ревью]
|
||||
|
||||
- **Где:** `internal/service/transcribe.go`, метки `file_extension` у размера
|
||||
|
||||
@@ -2,6 +2,7 @@ package yandex
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
@@ -52,8 +53,16 @@ func newSpeechKitService(cfg speechKitConfig) (*speechKitService, error) {
|
||||
// Создаем защищенное соединение для Operations API
|
||||
opConn, err := grpc.NewClient(OperationEndpoint, grpc.WithTransportCredentials(creds))
|
||||
if err != nil {
|
||||
sttConn.Close()
|
||||
return nil, fmt.Errorf("failed to connect to Operations API: %w", err)
|
||||
// Отказы независимы, и второй не теряется. На сегодняшнем клиенте он
|
||||
// почти наверняка не наступит: grpc.NewClient ленив, соединение к этому
|
||||
// моменту не открыто, и Close вернёт отказ только при повторном
|
||||
// закрытии — то есть сообщит о нашей ошибке, а не о Yandex. Сборка
|
||||
// оставлена как защита от смены реализации клиента; nil от закрытия
|
||||
// errors.Join отбрасывает, и форма ошибки в обычном случае не меняется.
|
||||
return nil, errors.Join(
|
||||
fmt.Errorf("failed to connect to Operations API: %w", err),
|
||||
sttConn.Close(),
|
||||
)
|
||||
}
|
||||
|
||||
sttClient := stt.NewAsyncRecognizerClient(sttConn)
|
||||
@@ -77,10 +86,10 @@ func (s *speechKitService) Close() error {
|
||||
if s.opConn != nil {
|
||||
err2 = s.opConn.Close()
|
||||
}
|
||||
if err1 != nil {
|
||||
return err1
|
||||
}
|
||||
return err2
|
||||
// Отказы двух соединений независимы, и вернуть только первый — значит
|
||||
// потерять половину причины: журнал пишется при остановке процесса, и
|
||||
// восстановить утраченное будет уже негде.
|
||||
return errors.Join(err1, err2)
|
||||
}
|
||||
|
||||
// recognizeFileFromS3 запускает асинхронное распознавание файла из S3
|
||||
|
||||
@@ -2,6 +2,7 @@ package worker
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"strconv"
|
||||
"time"
|
||||
@@ -48,7 +49,11 @@ func (w *CallbackWorker) Start(ctx context.Context) {
|
||||
return
|
||||
default:
|
||||
err := w.f()
|
||||
_, isNoop := err.(*contract.NoopJobError)
|
||||
// Признак узнаётся по смыслу, а не по точной форме значения:
|
||||
// приведение типа видело только вершину цепочки и сломалось бы от
|
||||
// первой же обёртки `%w`, которая в проекте — умолчание.
|
||||
var noop *contract.NoopJobError
|
||||
isNoop := errors.As(err, &noop)
|
||||
if !isNoop {
|
||||
metrics.WorkerJobCounter.WithLabelValues(w.Name(), strconv.FormatBool(err != nil)).Inc()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,204 @@
|
||||
package worker
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.vakhrushev.me/av/transcriber/internal/contract"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
)
|
||||
|
||||
// Проверки этого файла судят одну развилку воркера: пустой прогон против
|
||||
// отказа. Инвариант проекта — «NoopJobError не ошибка» — стоит ровно на ней, а
|
||||
// цена срабатывания отложенная: три воркера опрашивают базу раз в секунду, и
|
||||
// пустой прогон, принятый за отказ, даёт три записи в секунду и столько же
|
||||
// засчитанных сбоев, которых не было.
|
||||
|
||||
// journalBuffer собирает журнал прогона. Пишут в него из горутины воркера, а
|
||||
// читает проверка — отсюда мьютекс.
|
||||
type journalBuffer struct {
|
||||
mu sync.Mutex
|
||||
text strings.Builder
|
||||
}
|
||||
|
||||
func (b *journalBuffer) Write(p []byte) (int, error) {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return b.text.Write(p)
|
||||
}
|
||||
|
||||
func (b *journalBuffer) String() string {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return b.text.String()
|
||||
}
|
||||
|
||||
// runOnce прогоняет воркер ровно один раз и возвращает журнал этого прогона.
|
||||
// Цикл воркера бесконечен и спит секунду между прогонами, поэтому контекст
|
||||
// отменяется сразу после первого вызова работы: ждать второго прогона нечего, а
|
||||
// секунда сна на проверку — цена ни за что.
|
||||
func runOnce(t *testing.T, name string, work func() error) string {
|
||||
t.Helper()
|
||||
|
||||
journal := &journalBuffer{}
|
||||
logger := slog.New(slog.NewTextHandler(journal, nil))
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
var once sync.Once
|
||||
done := make(chan struct{})
|
||||
|
||||
w := NewCallbackWorker(name, func() error {
|
||||
err := work()
|
||||
once.Do(func() {
|
||||
cancel()
|
||||
close(done)
|
||||
})
|
||||
return err
|
||||
}, logger)
|
||||
|
||||
finished := make(chan struct{})
|
||||
go func() {
|
||||
w.Start(ctx)
|
||||
close(finished)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("работа воркера не была вызвана")
|
||||
}
|
||||
|
||||
select {
|
||||
case <-finished:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("воркер не остановился по отмене контекста")
|
||||
}
|
||||
|
||||
return journal.String()
|
||||
}
|
||||
|
||||
// runRecords оставляет от журнала только записи об исходе прогона. Жизненный
|
||||
// цикл самого воркера — старт и остановка — по конвенции идёт на INFO и к
|
||||
// прогону не относится; требование говорит о том, что воркер пишет про свой
|
||||
// прогон, а не о том, что он молчит вообще.
|
||||
func runRecords(journal string) string {
|
||||
var kept []string
|
||||
for _, line := range strings.Split(strings.TrimSpace(journal), "\n") {
|
||||
if line == "" {
|
||||
continue
|
||||
}
|
||||
if strings.Contains(line, "msg=\"Worker started\"") ||
|
||||
strings.Contains(line, "msg=\"Worker received shutdown signal") {
|
||||
continue
|
||||
}
|
||||
kept = append(kept, line)
|
||||
}
|
||||
return strings.Join(kept, "\n")
|
||||
}
|
||||
|
||||
// jobCount читает счётчик работы воркера из общего реестра процесса. Судит
|
||||
// реестр, а не переменную пакета: метка, потерянная в точке употребления,
|
||||
// переменную не ломает, а на странице метрик видна.
|
||||
func jobCount(t *testing.T, worker, errLabel string) float64 {
|
||||
t.Helper()
|
||||
|
||||
families, err := prometheus.DefaultGatherer.Gather()
|
||||
if err != nil {
|
||||
t.Fatalf("не удалось собрать метрики: %v", err)
|
||||
}
|
||||
|
||||
for _, mf := range families {
|
||||
if mf.GetName() != "transcriber_worker_job_count" {
|
||||
continue
|
||||
}
|
||||
for _, m := range mf.GetMetric() {
|
||||
var gotWorker, gotErr string
|
||||
for _, label := range m.GetLabel() {
|
||||
switch label.GetName() {
|
||||
case "name":
|
||||
gotWorker = label.GetValue()
|
||||
case "error":
|
||||
gotErr = label.GetValue()
|
||||
}
|
||||
}
|
||||
if gotWorker == worker && gotErr == errLabel {
|
||||
return m.GetCounter().GetValue()
|
||||
}
|
||||
}
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
// Обёртка `%w` объявлена конвенцией проекта умолчанием, и до этой задачи первая
|
||||
// же обёртка на пути сломала бы распознавание молча. Оракул держит именно
|
||||
// обёрнутое значение: на голом признак узнавался и приведением типа, то есть
|
||||
// проверка прошла бы и на починенном, и на сломанном коде.
|
||||
func TestWrappedNoopIsNotAFailure(t *testing.T) {
|
||||
const name = "wrapped_noop_worker"
|
||||
|
||||
before := jobCount(t, name, "false")
|
||||
beforeErr := jobCount(t, name, "true")
|
||||
|
||||
journal := runOnce(t, name, func() error {
|
||||
return fmt.Errorf("find and acquire job: %w", &contract.NoopJobError{State: "created"})
|
||||
})
|
||||
|
||||
// Записи о старте и остановке воркера законны и к прогону не относятся —
|
||||
// проверяется отсутствие записи об исходе прогона.
|
||||
if got := runRecords(journal); got != "" {
|
||||
t.Errorf("пустой прогон попал в журнал: %q", got)
|
||||
}
|
||||
if got := jobCount(t, name, "false"); got != before {
|
||||
t.Errorf("счётчик успешных прогонов вырос на пустом прогоне: было %v, стало %v", before, got)
|
||||
}
|
||||
if got := jobCount(t, name, "true"); got != beforeErr {
|
||||
t.Errorf("пустой прогон засчитан отказом: было %v, стало %v", beforeErr, got)
|
||||
}
|
||||
}
|
||||
|
||||
// Без этой проверки оракул был бы зелен и на коде, который не считает отказом
|
||||
// вообще ничего.
|
||||
func TestFailureIsLoggedAndCounted(t *testing.T) {
|
||||
const name = "failing_worker"
|
||||
|
||||
before := jobCount(t, name, "true")
|
||||
|
||||
journal := runOnce(t, name, func() error {
|
||||
return errors.New("database is gone")
|
||||
})
|
||||
|
||||
if !strings.Contains(journal, "database is gone") {
|
||||
t.Errorf("отказ не виден владельцу: журнал %q", journal)
|
||||
}
|
||||
if got := jobCount(t, name, "true"); got != before+1 {
|
||||
t.Errorf("отказ не засчитан: было %v, стало %v", before, got)
|
||||
}
|
||||
}
|
||||
|
||||
// Счёт успешных прогонов — знаменатель доли отказов. Реализация, снявшая его,
|
||||
// проходит обе проверки выше, а владелец теряет способность отличить «три
|
||||
// прогона в секунду, все отказали» от «три отказа среди тысячи прогонов».
|
||||
func TestSuccessIsCounted(t *testing.T) {
|
||||
const name = "successful_worker"
|
||||
|
||||
before := jobCount(t, name, "false")
|
||||
|
||||
journal := runOnce(t, name, func() error {
|
||||
return nil
|
||||
})
|
||||
|
||||
if got := jobCount(t, name, "false"); got != before+1 {
|
||||
t.Errorf("успешный прогон не засчитан: было %v, стало %v", before, got)
|
||||
}
|
||||
if strings.Contains(journal, "Worker error") {
|
||||
t.Errorf("успешный прогон записан отказом: журнал %q", journal)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,78 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.vakhrushev.me/av/transcriber/internal/contract"
|
||||
"git.vakhrushev.me/av/transcriber/internal/entity"
|
||||
)
|
||||
|
||||
// Путь признака «работы нет» состоит из двух звеньев: репозиторий рождает
|
||||
// «подходящей задачи не нашлось», сервис переводит это в «работы нет», и уже
|
||||
// его читает воркер. Проверки воркера подменяют работу целиком и второе звено
|
||||
// не видят — без этого файла правку в сервисе принимал бы только линтер, а он
|
||||
// судит форму записи, а не то, узнаётся ли признак на самом деле.
|
||||
|
||||
// stubJobRepo отдаёт заданную ошибку на запрос задачи. Прочих методов запроса
|
||||
// задачи проверки этого файла не зовут.
|
||||
type stubJobRepo struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (r *stubJobRepo) Create(*entity.TranscribeJob) error { return nil }
|
||||
func (r *stubJobRepo) Save(*entity.TranscribeJob) error { return nil }
|
||||
|
||||
func (r *stubJobRepo) GetByID(string) (*entity.TranscribeJob, error) {
|
||||
return nil, errors.New("не зовётся этими проверками")
|
||||
}
|
||||
|
||||
func (r *stubJobRepo) FindAndAcquire(string, string, time.Time) (*entity.TranscribeJob, error) {
|
||||
return nil, r.err
|
||||
}
|
||||
|
||||
func serviceWithRepo(repo contract.TranscriptJobRepository) *TranscribeService {
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
return NewTranscribeService(repo, nil, nil, nil, nil, nil, "", logger)
|
||||
}
|
||||
|
||||
// Репозиторий вправе добавить своему отказу пояснение — соседние ветки того же
|
||||
// метода уже оборачивают ошибки `%w` подряд. Пока признак узнавался приведением
|
||||
// типа, первая такая обёртка превратила бы пустой прогон в отказ: воркер начал
|
||||
// бы писать в журнал раз в секунду на каждом из трёх воркеров.
|
||||
func TestFindJobTranslatesWrappedNotFoundToNoop(t *testing.T) {
|
||||
svc := serviceWithRepo(&stubJobRepo{
|
||||
err: fmt.Errorf("find and acquire job: %w",
|
||||
&contract.JobNotFoundError{State: "created", Message: "appropriate job not found"}),
|
||||
})
|
||||
|
||||
_, err := svc.findJob("created", time.Minute)
|
||||
|
||||
var noop *contract.NoopJobError
|
||||
if !errors.As(err, &noop) {
|
||||
t.Fatalf("обёрнутое «задачи нет» не переведено в пустой прогон: получено %v", err)
|
||||
}
|
||||
if noop.State != "created" {
|
||||
t.Errorf("состояние потеряно при переводе: %q", noop.State)
|
||||
}
|
||||
}
|
||||
|
||||
// Оборотная сторона: настоящий отказ хранилища пустым прогоном считаться не
|
||||
// должен, иначе задача молча крутилась бы в цикле без единой записи.
|
||||
func TestFindJobKeepsRealFailure(t *testing.T) {
|
||||
svc := serviceWithRepo(&stubJobRepo{err: errors.New("database is gone")})
|
||||
|
||||
_, err := svc.findJob("created", time.Minute)
|
||||
|
||||
var noop *contract.NoopJobError
|
||||
if errors.As(err, &noop) {
|
||||
t.Fatalf("отказ хранилища зачтён пустым прогоном: %v", err)
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatal("отказ хранилища потерян")
|
||||
}
|
||||
}
|
||||
@@ -389,7 +389,10 @@ func (s *TranscribeService) findJob(state string, expiration time.Duration) (job
|
||||
|
||||
job, err = s.jobRepo.FindAndAcquire(state, acquisitionId, rottingTime)
|
||||
if err != nil {
|
||||
if _, ok := err.(*contract.JobNotFoundError); ok {
|
||||
// Признак узнаётся по смыслу: репозиторий вправе обернуть свой отказ
|
||||
// пояснением, и приведение типа от этого сломалось бы молча.
|
||||
var notFound *contract.JobNotFoundError
|
||||
if errors.As(err, ¬Found) {
|
||||
return nil, &contract.NoopJobError{State: state}
|
||||
}
|
||||
s.logger.Error("Failed to find and acquire job", "state", state, "error", err)
|
||||
|
||||
@@ -121,7 +121,15 @@ func main() {
|
||||
logger.Error("failed to create audio recognizer", "error", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
defer recognizer.Close()
|
||||
// Отдавать отказ закрытия некому — процесс заканчивается, — поэтому он идёт
|
||||
// в журнал владельца. Что он означает: gRPC-клиент отдаёт здесь отказ лишь
|
||||
// при повторном закрытии, то есть запись говорит о нашей ошибке, а не о
|
||||
// недоступности Yandex.
|
||||
defer func() {
|
||||
if err := recognizer.Close(); err != nil {
|
||||
logger.Error("failed to close audio recognizer", "error", err)
|
||||
}
|
||||
}()
|
||||
|
||||
// Создаем сервисы
|
||||
transcribeService := service.NewTranscribeService(
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-08-11
|
||||
@@ -0,0 +1,218 @@
|
||||
## Context
|
||||
|
||||
Два признака конвейера — «работы в этом состоянии нет» и «подходящей задачи не
|
||||
нашлось» — сегодня узнаются приведением значения ошибки к точному типу:
|
||||
`err.(*contract.NoopJobError)` в `internal/controller/worker/worker.go:51` и
|
||||
`err.(*contract.JobNotFoundError)` в `internal/service/transcribe.go:392`.
|
||||
Приведение видит только само значение и слепо к пояснениям, добавленным
|
||||
обёрткой `fmt.Errorf("…: %w", err)`.
|
||||
|
||||
Путь сегодня короткий и обёрток на нём нет: `FindAndAcquire`
|
||||
(`internal/adapter/repo/sqlite/transcript_job_repo.go:177`) рождает
|
||||
`JobNotFoundError`, `findJob` переводит его в `NoopJobError`, воркер этот признак
|
||||
ловит. Поэтому дефект пока не проявился — он ждёт первой обёртки, а обёртка
|
||||
`%w` объявлена конвенцией `docs/conventions/errors.md` умолчанием проекта. То
|
||||
есть код написан против собственного умолчания, и цена срабатывания —
|
||||
три записи отказа в секунду и столько же засчитанных сбоев, которых не было.
|
||||
|
||||
Отдельно и мельче: отказ закрытия соединения теряется в двух местах —
|
||||
`sttConn.Close()` на пути отказа конструктора распознавателя
|
||||
(`internal/adapter/recognizer/yandex/speechkit.go:55`) и `defer
|
||||
recognizer.Close()` при остановке процесса (`main.go:124`).
|
||||
|
||||
Оба сюжета держат гейт проекта красным четырьмя замечаниями линтера; долг
|
||||
объявлен в `CLAUDE.md`, разделе «Гейт».
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
|
||||
- Признак пустого прогона переживает пояснение, добавленное на любом
|
||||
промежуточном шаге.
|
||||
- Отказ закрытия соединения не теряется молча.
|
||||
- Гейт зелёный целиком: замечаний `errorlint` и `errcheck` нет.
|
||||
|
||||
**Non-Goals:**
|
||||
|
||||
- Внешнее поведение не меняется: ни ответы пользователю, ни набор состояний
|
||||
задачи, ни формат метрик.
|
||||
- Единая точка перевода доменной ошибки в HTTP-статус — другое расхождение той
|
||||
же конвенции, записанное там же; этим изменением не трогается.
|
||||
- Обёртки `%w` по всему пути не расставляются: изменение делает проверку
|
||||
устойчивой к ним, а не вводит их.
|
||||
- `tg.EmptyBotTokenError` — третья типизированная ошибка без полей — не трогается:
|
||||
линтер на ней молчит, приведения типа у неё нет.
|
||||
|
||||
## Decisions
|
||||
|
||||
### Решение 1: признак узнаётся `errors.As`, типы остаются
|
||||
|
||||
Обе проверки переходят на `errors.As` с сохранением сегодняшних типов
|
||||
`contract.NoopJobError` и `contract.JobNotFoundError`.
|
||||
|
||||
Что человек увидит иначе: ничего — ровно в этом ценность. Владелец сервиса
|
||||
увидит разницу лишь в тот день, когда кто-то добавит пояснение к ошибке на этом
|
||||
пути: журнал останется тихим, вместо того чтобы наполниться отказами, которых
|
||||
не было.
|
||||
|
||||
**Рассмотрено и отвергнуто — sentinel вместо типов.** Конвенция
|
||||
(`docs/conventions/errors.md`, раздел «Sentinel и типизированные») говорит:
|
||||
типизированная ошибка нужна, когда вызывающему нужны **данные**, а данные обоих
|
||||
типов сегодня не читает никто — обе проверяются на факт. По этому доводу типы
|
||||
следовало бы заменить на `errors.New` и проверять `errors.Is`, а состояние
|
||||
задачи вносить обёрткой `fmt.Errorf("%s: %w", state, contract.ErrNoJob)`.
|
||||
|
||||
Отвергнуто по цене против цели: обе формы **одинаково** устойчивы к обёртке, то
|
||||
есть по цели изменения они неразличимы, а sentinel правит пять мест вместо двух
|
||||
и переписывает рождение ошибки в репозитории и в сервисе. Задача — снятие долга
|
||||
линтера, а не пересмотр номенклатуры ошибок. Довод конвенции при этом не
|
||||
исчезает: он остаётся верным и становится поводом отдельной работы в тот день,
|
||||
когда типы так и не начнут нести читаемых данных.
|
||||
|
||||
**Рассмотрено и отвергнуто — оставить приведение, заглушив линтер.** Правило
|
||||
`errorlint` выключается директивой на строке. Отвергнуто: дефект от этого не
|
||||
исчезает, а перестаёт быть видимым, и следующий читатель кода примет молчание
|
||||
линтера за проверенность.
|
||||
|
||||
### Решение 2: отказ закрытия — по месту, а не единым правилом
|
||||
|
||||
Два места разные по природе, и одинаково они не чинятся.
|
||||
|
||||
- **Путь отказа конструктора** (`speechkit.go:55`): клиент распознавания создан,
|
||||
а клиент операций создать не удалось. Оба отказа независимы, и конвенция для
|
||||
такого случая называет `errors.Join`. Ошибка конструктора получает вторую
|
||||
строку, если закрытие тоже отказало.
|
||||
- **Остановка процесса** (`main.go:124`): отдавать отказ некому — процесс
|
||||
заканчивается. Значит, он идёт в журнал владельца, а `defer` получает тело с
|
||||
проверкой. В запись идёт **значение ошибки как есть**: оно несёт состояние
|
||||
клиента и адрес узла, но не тело запроса и не ключ — ключ живёт в метаданных
|
||||
вызова, а не в соединении. Разворачивать первопричину на границе клиента, как
|
||||
требует `docs/conventions/logging.md` от ошибок транспорта, здесь нечего:
|
||||
обёрток на этом пути нет.
|
||||
|
||||
**Чем этот отказ является на самом деле — сказано прямо, чтобы обоснование не
|
||||
поехало дальше в неверном виде.** `grpc.NewClient` ленив и соединения не
|
||||
открывает, а `Close` у клиента возвращает отказ единственным образом — при
|
||||
повторном закрытии. Значит, на пути отказа конструктора второй операнд почти
|
||||
наверняка `nil`, а запись при остановке процесса говорит о **нашей** ошибке
|
||||
(закрыли дважды), а не о недоступности Yandex. Обработка обоих мест остаётся:
|
||||
она стоит одну строку и переживёт смену клиента, а её отсутствие каждый раз
|
||||
приходится заново обосновывать читателю.
|
||||
|
||||
**Оракул этого решения чинится машиной, а не обещанием.** Правило `errcheck`
|
||||
сегодня молчит на `_ = conn.Close()`: настройка `check-blank` не выставлена, а её
|
||||
умолчание — «пропускать». То есть реализация, выбрасывающая отказ в пустоту,
|
||||
удовлетворяет критерию приёмки «линтер не даёт замечаний `errcheck`», не
|
||||
удовлетворяя самому критерию — «отказ возвращается либо попадает в журнал».
|
||||
Поэтому изменение включает `errcheck.check-blank: true` в `.golangci.yml`.
|
||||
Проверено прогоном: на сегодняшнем коде правило замечаний не добавляет, то есть
|
||||
включается чисто и отдельного коммита приведения не требует.
|
||||
|
||||
Это правка политики линтера, и её цена названа: `_ =` перестаёт быть способом
|
||||
сказать «отказ здесь не важен» молча — теперь такое место придётся либо
|
||||
обрабатывать, либо вносить в `exclude-functions` поимённо, то есть заметно.
|
||||
|
||||
**Рассмотрено и отвергнуто — дописать оба типа в `exclude-functions`
|
||||
`.golangci.yml`.** Соблазн сильный: список исключений там уже есть, и в нём
|
||||
записана ровно эта политика — «закрытие через `defer` и лучшая-попытка уборки
|
||||
файла — осознанно без проверки». То есть замечание снимается одной строкой
|
||||
конфига, и она даже выглядит согласованной с прежним решением.
|
||||
|
||||
Отвергнуто: политика в конфиге относится к закрытию, у которого **отказ ничего
|
||||
не значит** — файл, читатель, соединение с базой на выходе. Здесь не так: в
|
||||
первом случае отказ закрытия сопровождает уже случившийся отказ и полезен для
|
||||
разбора, во втором — это единственный признак того, что соединение с оплачиваемым
|
||||
внешним сервисом закрылось неправильно. Записав их в исключения, мы бы
|
||||
расширили политику молча, самим фактом добавления строки, и потеряли бы оба
|
||||
сигнала навсегда. Критерий приёмки задачи требует именно «возвращается либо
|
||||
попадает в журнал», а не «линтер замолчал».
|
||||
|
||||
### Решение 3: узнавание по смыслу шире приведения, и граница ставится нормой
|
||||
|
||||
Приведение типа видело **только вершину** цепочки. `errors.As` видит признак на
|
||||
любой её глубине — в этом и цель, но у расширения есть встречная сторона: отказ,
|
||||
к которому признак пустого прогона примешался по дороге (обёрткой или
|
||||
`errors.Join`), воркер зачтёт пустым прогоном. Тогда задача останется в своём
|
||||
состоянии и будет переопрашиваться раз в секунду без единой записи — тот самый
|
||||
класс, от которого защищает инвариант «Принятая запись не теряется молча», только
|
||||
с обратным знаком относительно чинимого дефекта.
|
||||
|
||||
Прямого места, где это случается, сегодня нет: признак рождается ровно в двух
|
||||
местах и никем не оборачивается. Но `%w` объявлен умолчанием проекта, а
|
||||
`errors.Join` вводит в кодовую базу это же изменение — значит, дыра появится
|
||||
тихо и не сегодня.
|
||||
|
||||
Граница ставится **нормой, а не кодом**: спека требует, чтобы признак рождался
|
||||
только ответом хранилища на опрос этим же шагом, и запрещает слою сохранять чужой
|
||||
признак в цепочке своей ошибки.
|
||||
|
||||
**Рассмотрено и отвергнуто — научить воркер различать «признак на вершине» от
|
||||
«признака в глубине».** Отвергнуто по цене: `errors.As` такого различения не
|
||||
даёт вовсе, пришлось бы либо проверять вершину вручную (то есть вернуть
|
||||
приведение типа, которое чинится), либо заводить свой обход цепочки. Код
|
||||
усложняется ради случая, которого сегодня нет ни одного, а защита от него нужна
|
||||
на входе — при написании нового слоя, — где норма работает, а проверка в рантайме
|
||||
опоздала бы.
|
||||
|
||||
### Решение 4: спека заводится только на пустой прогон
|
||||
|
||||
Дельта заводит capability `pipeline` с одним требованием — о пустом прогоне
|
||||
воркера. Имя предвосхищено маркерами долга в `docs/architecture.md`.
|
||||
|
||||
Закрытие соединения требования **не получает**: домена оно не трогает, наружу не
|
||||
видно. Заводить под него норму значило бы нормировать внутреннюю гигиену —
|
||||
граница спек проекта проходит не здесь. Судьёй остаётся критерий приёмки, а его
|
||||
оракул сделан различающим включением `check-blank` (Решение 2), а не оставлен на
|
||||
слово.
|
||||
|
||||
Требование при этом **не закрепляет нормой известный долг журнала.**
|
||||
`docs/conventions/logging.md` держит расхождение: сбой фонового цикла
|
||||
записывается дважды — шагом конвейера и следом воркером, — и уровнем `ERROR`
|
||||
там, где конвенция просит `WARN` для повторяющегося сбоя. Спека говорит «ровно
|
||||
один раз единственной логирующей точкой» и уровня не называет: иначе следующий,
|
||||
кто возьмётся закрывать этот долг, обнаружил бы, что убрать вторую запись нельзя
|
||||
без правки спеки, — и долг стал бы контрактом молча.
|
||||
Остальное поведение конвейера (переходы состояний, захват, срок протухания)
|
||||
спекой тоже не описывается: оно этим изменением не трогается, а требование,
|
||||
написанное без проверки, — предположение, а не норма.
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- **`errors.As` требует указателя на указатель, и ошибка формы не ловится
|
||||
компилятором, а даёт панику в рантайме** → цель объявляется переменной нужного
|
||||
типа (`var noop *contract.NoopJobError`), а ветка покрывается тестом, который
|
||||
и есть оракул критерия 2.
|
||||
- **`defer` не сработает, если процесс уйдёт через `os.Exit`** → проверить, что
|
||||
между постановкой `defer` и штатным выходом `os.Exit` не вызывается; иначе
|
||||
запись о закрытии не появится, и это будет тихой потерей того же рода, что
|
||||
чинится.
|
||||
- **Типы остаются, и довод конвенции о sentinel остаётся неотработанным** →
|
||||
назван открытым вопросом, а не забыт; работа отдельная.
|
||||
- **`check-blank: true` меняет политику для всего проекта, а не для двух мест** →
|
||||
проверено прогоном: на сегодняшнем коде замечаний не добавляется. Цена в
|
||||
будущем — осознанное игнорирование отказа придётся объявлять в
|
||||
`exclude-functions`, а не писать `_ =` по месту.
|
||||
- **Молчание на пустом прогоне остаётся единственным наблюдаемым состоянием
|
||||
цикла**: воркер, остановившийся по отмене или не стартовавший вовсе, выглядит
|
||||
так же, как воркер без работы → изменением не чинится и в требование не
|
||||
закладывается запрет на будущий признак живости: спека запрещает лишь запись
|
||||
**на уровне владельца**, оставляя место и `DEBUG`, и отдельному счётчику
|
||||
прогонов. Работа отдельная, идёт в урожай.
|
||||
- **У пакета `internal/controller/worker` сегодня нет ни одного теста** → тест
|
||||
на пустой прогон заводится этим изменением и приносит с собой первую тестовую
|
||||
оснастку пакета: подменный журнал и чтение счётчика из реестра метрик. Оснастка
|
||||
рискует разойтись с той, что уже есть в `internal/service` и
|
||||
`internal/controller/http`; сверяется по ним, а не пишется с нуля.
|
||||
|
||||
## Migration Plan
|
||||
|
||||
Миграции нет: схема базы не трогается, данные не переносятся, формат файлов на
|
||||
диске не меняется. Откат — обратный коммит.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- **Переводить ли обе ошибки в sentinel** и убирать типы, которых никто не
|
||||
читает. Довод конвенции остаётся в силе; цена — правка пяти мест против двух.
|
||||
Решается отдельной работой, не этой.
|
||||
- **Единая точка перевода доменной ошибки в HTTP-статус** — расхождение записано
|
||||
в `docs/conventions/errors.md` и этим изменением не закрывается.
|
||||
@@ -0,0 +1,48 @@
|
||||
## Why
|
||||
|
||||
Воркер отличает «работы сейчас нет» от настоящего отказа хрупким способом: он
|
||||
смотрит на точный тип значения ошибки. Пока никто по дороге не добавил к ошибке
|
||||
пояснения, это работает. Первое же пояснение, добавленное в любом месте пути,
|
||||
сделает пустой прогон неотличимым от поломки — молча, без единого признака в
|
||||
коде. Сервис начнёт раз в секунду на каждый из трёх воркеров писать в журнал
|
||||
отказ, которого не было, и засчитывать несуществующие сбои в счётчик работы.
|
||||
Инвариант проекта «пустой прогон — не ошибка» стоит ровно на этой проверке.
|
||||
|
||||
Сегодня же на этих местах красен линтер, и вместе с двумя потерянными отказами
|
||||
закрытия соединения он держит гейт проекта красным целиком.
|
||||
|
||||
## What Changes
|
||||
|
||||
- Признак «работы нет» и признак «подходящей задачи не нашлось» перестают
|
||||
зависеть от точной формы значения ошибки: они узнаются по смыслу и переживают
|
||||
любые пояснения, добавленные по дороге.
|
||||
- Отказ при закрытии соединения с распознавателем перестаёт теряться: он либо
|
||||
доходит до вызывающего, либо попадает в журнал владельца.
|
||||
- Поведение снаружи не меняется: пользователь, внешняя программа и набор
|
||||
состояний задачи остаются прежними.
|
||||
- Гейт проекта становится зелёным целиком — снимается объявленный долг из
|
||||
четырёх замечаний линтера.
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
|
||||
- `pipeline`: конвейер расшифровки — как задача переходит между состояниями, что
|
||||
делает воркер, когда работы нет, и что считается отказом шага. Имя предвосхищено
|
||||
маркерами долга в `docs/architecture.md`; этим изменением заводится **только**
|
||||
требование о пустом прогоне, остальное поведение конвейера дописывает задача,
|
||||
которая его тронет.
|
||||
|
||||
### Modified Capabilities
|
||||
|
||||
Нет. Требования `intake` изменение не трогает.
|
||||
|
||||
## Impact
|
||||
|
||||
- `internal/contract` — форма признаков «задачи нет» и «задача не найдена»;
|
||||
- `internal/controller/worker` — проверка пустого прогона, журнал и счётчик
|
||||
работы; у пакета сегодня нет ни одного теста, изменение заводит первый;
|
||||
- `internal/service` — перевод «задача не найдена» в «работы нет»;
|
||||
- `internal/adapter/repo/sqlite` — рождение признака «задача не найдена»;
|
||||
- `internal/adapter/recognizer/yandex` и `main.go` — отказ закрытия соединения;
|
||||
- гейт проекта и запись долга в `docs/conventions/errors.md` и `CLAUDE.md`.
|
||||
@@ -0,0 +1,477 @@
|
||||
# Триаж ревью: errors-as-instead-of-typecast
|
||||
|
||||
## Сводка
|
||||
|
||||
- **Размер / сложность / метка:** среднее × знакомое → **medium**. Режим — по графу.
|
||||
База диффа `origin/master` непригодна (удалённая ветка отстала на десятки
|
||||
коммитов и даёт 9136 строк шума); реальный вход — рабочее дерево: 5 файлов кода
|
||||
и конфига, 3 документа, 2 новых теста, каталог `openspec/changes/`.
|
||||
- **Состояние гейта:** зелёный. Проверено проходом `autotests` дважды (exit 0) и
|
||||
переспрошено триажем поимённо: `golangci-lint run` → `0 issues`,
|
||||
`go test ./...` → все пакеты `ok`. Объявленных долгов у гейта после этого
|
||||
изменения нет — заявление `CLAUDE.md` подтверждено выводом инструментов, а не
|
||||
декларацией.
|
||||
- **Находок на входе:** 12 сырых от четырёх проходов → **9 различных причин**
|
||||
после дедупликации (`Close`/`err2` пришёл трижды, MUST «ровно один раз» —
|
||||
дважды) → **5 в первых двух секциях**, 3 в гипотезах, 1 отсеяна.
|
||||
- **Потолки не срабатывали:** 2 + 3 = 5 при допустимых 3 + 4. Ничего не срезано,
|
||||
ничего не выброшено молча.
|
||||
|
||||
### Сигнал о заниженной метке
|
||||
|
||||
**Пришёл, от одного прохода — `review-code`.** Основания: четыре узла правки,
|
||||
изменение политики линтера на весь проект (`errcheck.check-blank`), переписанный
|
||||
раздел «Гейт» в `CLAUDE.md`, введение новой capability `pipeline`. С меткой
|
||||
`large` запускались бы отдельные проходы `architecture` и `operations` вместо
|
||||
разбора этих тем внутри `basics`.
|
||||
|
||||
`review-basics` запускался и сигнала о метке не подал. Согласие/несогласие
|
||||
проходов приоритет меняет, `confidence` — нет: метку выбирал `review-scope`.
|
||||
|
||||
**Ретроспективно сигнал подтверждается исходом:** три из пяти оставшихся находок
|
||||
— про документы и норму (`architecture.md`, `conventions/README.md`,
|
||||
`docs/review.md`, delta-спека), то есть ровно про тот слой, который на метке
|
||||
`medium` разбирается наименее глубоко.
|
||||
|
||||
### План разметки с исходом по каждой теме
|
||||
|
||||
| Тема | Дом | Глубина | Кто закрывает | Исход |
|
||||
|---|---|---|---|---|
|
||||
| requirements | дельта `specs/pipeline/spec.md` | разбор | `specs` | **закрыта**, 2 находки (1 дошла до блокирующей) |
|
||||
| autotests | `CLAUDE.md` «Гейт» | — | `autotests` | **закрыта**, 1 находка (понижена в гипотезы) |
|
||||
| conventions | `docs/conventions/` (errors.md, logging.md, README.md) | разбор | `code` | **закрыта**, 4 находки (1 блокирующая, 1 отсеяна) |
|
||||
| architecture | `docs/architecture.md` + `passport.md` | разбор | `basics` | **закрыта**, 1 находка |
|
||||
| security | `docs/security.md` | разбор | `basics` | **закрыта**, 0 находок (все три вопроса неприменимы) |
|
||||
| operations | `docs/architecture.md` «Эксплуатация» + `database.md` | разбор | `basics` | **закрыта**, 1 находка (понижена: изменением не создана) |
|
||||
|
||||
Своих тем проекта нет. **Тем без отчёта нет** — все шесть вернули вывод.
|
||||
|
||||
---
|
||||
|
||||
## Блокирует мердж
|
||||
|
||||
### 1. Дельта-спека закрепляет контрактом поведение, которого у системы нет, и после архивации станет ложной посылкой для следующей задачи
|
||||
|
||||
- Файл: `openspec/changes/errors-as-instead-of-typecast/specs/pipeline/spec.md:36-40`
|
||||
- Severity: major (`critical` не ставится: построенного пути к отказу или потере
|
||||
данных нет — вред отложенный и через следующего автора)
|
||||
- Confidence: high
|
||||
- Найдено проходами: `specs` (находка 1), `basics` («дешевле переделать», п. 1) —
|
||||
две независимые формулировки одной причины; оракула у них не было, оракул добыт
|
||||
триажем.
|
||||
- **Оракул (мой прогон, не на слово):**
|
||||
|
||||
```
|
||||
go test -overlay=<scratchpad>/overlay.json -run TestTriageOneFailureOneRecord -v ./internal/service/
|
||||
```
|
||||
|
||||
Тест поднимает `findJob` с репозиторием, отдающим `errors.New("database is gone")`,
|
||||
и логирует возвращённое так, как это делает `worker.go:59`. Вывод:
|
||||
|
||||
```
|
||||
level=ERROR msg="Failed to find and acquire job" state=created error="database is gone"
|
||||
level=ERROR msg="Worker error" worker=probe error="failed find and acquire job: created, database is gone"
|
||||
--- FAIL: один отказ дал 2 записей уровня ERROR, требование спеки — ровно одна
|
||||
```
|
||||
|
||||
Второй оракул — дословный критерий приёмки этого же изменения
|
||||
(`tasks.md`, «Добавлено разбором дизайна»): «**Норма не закрепляет контрактом
|
||||
то, что конвенции помечают строкой «Расхождение»: место записи и уровень
|
||||
журнала остаются долгом `docs/conventions/logging.md`**». Спека нормирует
|
||||
именно место записи. Долг записан в `docs/conventions/logging.md:159-162`.
|
||||
|
||||
- Последствие: требование `MUST быть записан ровно один раз единственной
|
||||
логирующей точкой` не имеет ни сценария (проверить его нечем — покраснеть оно
|
||||
не может), ни срока, ни владельца, ни маркера долга. Оговорка «сегодняшнее
|
||||
расхождение этим требованием нормой не объявляется» снимает обратное прочтение,
|
||||
но не снимает сам MUST. После `archive` абзац переезжает в
|
||||
`openspec/specs/pipeline/spec.md` и читается следующим автором как
|
||||
действующая норма: он либо «починит» вторую точку записи, не приняв
|
||||
сознательно решение об уровне (`ERROR` против `WARN` — дизайн его намеренно
|
||||
не принимал, `logging.md` требует `WARN` для повторяющегося сбоя фонового
|
||||
цикла), либо норма сгниёт. По `CLAUDE.md` («что такое сделана»: гейт зелёный
|
||||
**и критерии приёмки проверены поимённо») изменение сейчас не выполнено по
|
||||
своему собственному критерию.
|
||||
- Предложение: см. варианты в развилке.
|
||||
- **Действие: развилка.**
|
||||
|
||||
> В дельта-спеке `pipeline` абзац «Отказ шага MUST быть записан ровно один раз
|
||||
> единственной логирующей точкой» нормирует поведение, которого у системы нет
|
||||
> (оракул: один отказ хранилища даёт две записи ERROR), и нарушает
|
||||
> собственный критерий приёмки задачи. Как поступаем?
|
||||
>
|
||||
> **(а) Ослабить норму до проверяемого сегодня.** Оставить в требовании только
|
||||
> «отказ шага MUST быть виден владельцу записью в журнале и засчитан в счётчик
|
||||
> с пометкой отказа» — это покрыто сценарием «Шаг отказал» и проверкой
|
||||
> `TestFailureIsLoggedAndCounted`. Единственность логирующей точки вынести
|
||||
> отдельной строкой долга со ссылкой на `docs/conventions/logging.md:159-162` и
|
||||
> завести задачу в `tasks/items/`. Цена: правка дельта-спеки → **одобрение
|
||||
> дизайна отменяется, нужен возврат на чекпоинт**; кода не касается.
|
||||
>
|
||||
> **(б) Убрать вторую точку записи в этом же изменении.** Снять
|
||||
> `s.logger.Error("Failed to find and acquire job", …)` из
|
||||
> `internal/service/transcribe.go:398`, оставив запись воркеру. Цена: это чужой
|
||||
> долг и рост scope; требует сознательного решения об уровне записи, которое
|
||||
> дизайн не принимал; нужен новый сценарий и новая проверка на «ровно один
|
||||
> раз». Дельта-спека при этом всё равно правится (добавляется сценарий).
|
||||
>
|
||||
> **(в) Оставить как есть.** Цена: в актуальную спеку уезжает MUST, который
|
||||
> система нарушает с момента `apply` и который никогда не покраснеет.
|
||||
|
||||
**Прямой ответ на заданный вопрос: да, принятие этой находки меняет
|
||||
дельта-спеку — в любом из вариантов (а) и (б).** По правилу скилла `resolve`
|
||||
это отменяет одобрение дизайна и требует возврата на чекпоинт к человеку.
|
||||
Вариант (в) чекпоинта не требует, но оставляет дефект.
|
||||
|
||||
### 2. Закрытый долг остался записан в трёх местах, и одно из них — раздел, которым триаж отсеивает ложноположительные: следующая настоящая находка того же класса будет молча выброшена
|
||||
|
||||
- Файлы: `docs/conventions/README.md:21`, `docs/conventions/README.md:54`,
|
||||
`docs/review.md:69-74`
|
||||
- Severity: major
|
||||
- Confidence: high
|
||||
- Найдено проходом: `code` (находка B); третье место (`docs/review.md`) добавлено
|
||||
триажем — это тот же дефект в третьем доме.
|
||||
- **Оракул — дословные строки, живые на момент триажа:**
|
||||
- `docs/conventions/README.md:21`: «…лог пишется на каждом шаге и дублируется
|
||||
воркером, **доменные ошибки проверяются приведением типа**.» — расхождения
|
||||
больше нет, изменение его закрыло.
|
||||
- Правило самого же README (строки 27-29): «Каждое такое место названо в своей
|
||||
записи строкой «*Расхождение:*». … **Проходу ревью строка «Расхождение»
|
||||
говорит, что находка на этом месте уже известна и новой не считается.**»
|
||||
- `docs/review.md:69-74`, раздел «Типовые ложноположительные»: «Настоящий
|
||||
дефект рядом другой — **проверка идёт приведением типа и сломается при первой
|
||||
же обёртке; он уже записан в
|
||||
[conventions/errors.md](conventions/errors.md)**» — ссылка ведёт в файл, из
|
||||
которого этот текст изменением удалён (`git diff docs/conventions/errors.md`,
|
||||
строки 51-57 сняты).
|
||||
- `docs/conventions/README.md:54`: «Непроверенное возвращаемое значение ошибки |
|
||||
`.golangci.yml` → `errcheck` (кроме `defer Close` и `send`)» — таблица не
|
||||
знает о включённом `check-blank`, то есть о том, что `_ = x.Close()` теперь
|
||||
краснеет.
|
||||
- Задача сама называет ровно две правки (`tasks.md`, 4.2): «В
|
||||
`docs/conventions/errors.md` снять …; **прочие расхождения того файла** не
|
||||
трогать». Про соседний `README.md` и про `docs/review.md` там нет ничего —
|
||||
места просто пропущены, а не оставлены сознательно.
|
||||
- Последствие: класс «молчание». `docs/review.md`, «Типовые ложноположительные»
|
||||
— единственный проектный вход в шаг отсева триажа. Пока строка жива, следующий
|
||||
прогон ревью, увидев приведение типа в новом коде, обязан отнести находку к
|
||||
известным и выбросить её — то есть регрессия ровно того дефекта, который
|
||||
чинило это изменение, пройдёт молча. `docs.py check` этого не ловит: ссылки не
|
||||
битые, битым стало утверждение внутри документа.
|
||||
- Предложение: снять фразу «доменные ошибки проверяются приведением типа» из
|
||||
`README.md:21`; снять или переписать первый пункт «Типовых ложноположительных»
|
||||
в `docs/review.md` — оговорка про настоящий дефект рядом больше неверна, сам
|
||||
же пункт про `NoopJobError` остаётся верным; в строке таблицы «Механизировано»
|
||||
заменить «(кроме `defer Close` и `send`)» на актуальный перечень
|
||||
`exclude-functions` и упомянуть `check-blank: true`.
|
||||
- **Действие: инлайн.** Три текстовые правки, решение однозначно, инвариантов не
|
||||
трогает.
|
||||
|
||||
---
|
||||
|
||||
## Стоит исправить сейчас
|
||||
|
||||
### 3. `Close()` адаптера SpeechKit теряет отказ закрытия второго соединения — ровно тот дефект, который изменение объявило закрытым своим критерием приёмки
|
||||
|
||||
- Файл: `internal/adapter/recognizer/yandex/speechkit.go:79-91`
|
||||
- Severity: minor (ущерб мал — см. находку 4: на этом пути оба `Close` в
|
||||
сегодняшней реализации возвращают `nil`; вес держится критерием приёмки, а не
|
||||
последствием)
|
||||
- Confidence: high
|
||||
- Найдено проходами: `specs` (находка 2), `code` (находка D), `basics` (находка
|
||||
E) — **три независимых попадания, оракула ни у одного нет**. Совпадение
|
||||
повышает приоритет (значит, бросается в глаза), но не `confidence`: под всеми
|
||||
проходами одна модель.
|
||||
- Оракул — дословный критерий приёмки этого же изменения (`tasks.md`, раздел
|
||||
«Критерии приёмки»): «**Отказ `Close` не теряется молча: он либо возвращается
|
||||
вызывающему, либо попадает в лог.**» Код:
|
||||
|
||||
```go
|
||||
if err1 != nil {
|
||||
return err1
|
||||
}
|
||||
return err2 // err2 при ненулевом err1 теряется молча
|
||||
```
|
||||
|
||||
`errcheck` этого не видит: значение присвоено переменной. То есть механизация,
|
||||
ради которой изменение включило `check-blank`, здесь мимо.
|
||||
- Последствие: при остановке процесса, если оба gRPC-соединения отказали в
|
||||
закрытии, владелец увидит один отказ из двух и будет разбирать половину
|
||||
картины. Вероятность низкая, стоимость правки — одна строка.
|
||||
- Предложение: `return errors.Join(err1, err2)` — тот же приём, который изменение
|
||||
уже применило в конструкторе восемью строками выше, и `nil` из него отбрасывается.
|
||||
- **Действие: инлайн.**
|
||||
|
||||
### 4. Комментарии и `design.md` описывают поведение gRPC, которого нет: журнал отправит владельца разбирать недоступность Yandex по ошибке, которая может возникнуть только от нашего двойного закрытия
|
||||
|
||||
- Файлы: `internal/adapter/recognizer/yandex/speechkit.go:56-59`, `main.go:124-129`,
|
||||
`openspec/changes/errors-as-instead-of-typecast/design.md:81-92`
|
||||
- Severity: minor
|
||||
- Confidence: high
|
||||
- Найдено проходом: `code` (находка A). Оракул проверен триажем поимённо.
|
||||
- **Оракул — исходники `google.golang.org/grpc@v1.74.2`:**
|
||||
- `clientconn.go:145` — `func NewClient(...)`: конструктор ленив, соединения не
|
||||
открывает (устанавливает его первый RPC либо явный `Connect()`);
|
||||
- `clientconn.go:1142-1156` — `Close()` возвращает **только** `nil` либо
|
||||
`ErrClientConnClosing`, и только при повторном закрытии (`if cc.conns == nil`);
|
||||
- `clientconn.go:67` — `ErrClientConnClosing = status.Error(codes.Canceled,
|
||||
"grpc: the client connection is closing")`: статическая константа.
|
||||
- Последствие, по местам:
|
||||
- `speechkit.go:56-59` — комментарий обещает «**уже открытое** соединение могло
|
||||
не закрыться»; на деле второй операнд `errors.Join` на этом пути **всегда
|
||||
`nil`**. Конструкция безвредна и защищает от смены реализации, но читатель
|
||||
выведет из комментария неверную модель API;
|
||||
- `main.go:124-129` — комментарий «недоступность внешнего сервиса разбирает он»
|
||||
и уровень `ERROR` (по `logging.md` — класс «сбой БД, диска, недоступность
|
||||
внешнего сервиса»). Запись достижима только двойным закрытием, то есть нашим
|
||||
дефектом. Владелец, увидев её, пойдёт разбирать Yandex вместо своего кода;
|
||||
- `design.md:81-92` — тот же неверный образ записан двумя утверждениями
|
||||
(«соединение с распознаванием уже открыто»; ошибка «несёт состояние
|
||||
соединения и адрес узла» — `ErrClientConnClosing` не несёт ни того, ни
|
||||
другого) и после архивации поедет дальше как обоснование.
|
||||
- Предложение: код не трогать. Привести к действительности три текста: в
|
||||
`speechkit.go` — «`grpc.NewClient` ленив, закрытие здесь почти всегда `nil`;
|
||||
`errors.Join` стоит на случай смены реализации»; в `main.go` — «единственный
|
||||
достижимый отказ здесь — повторное закрытие, то есть дефект наш, а не
|
||||
внешнего сервиса»; в `design.md` — снять оба неверных утверждения.
|
||||
- **Действие: инлайн.** Правка `design.md` фактическая, а не решенческая:
|
||||
решение («отказ закрытия — по месту, а не единым правилом») остаётся тем же,
|
||||
меняется только неверное описание чужого API. Дельта-спеку не трогает,
|
||||
чекпоинта не требует.
|
||||
|
||||
### 5. Преамбула `docs/architecture.md` описывает состояние после архивации: сегодня она утверждает существование спеки, до которой нет пути
|
||||
|
||||
- Файл: `docs/architecture.md:11-23`
|
||||
- Severity: minor
|
||||
- Confidence: high
|
||||
- Найдено проходом: `basics` («дешевле переделать», п. 2)
|
||||
- Оракул — состояние дерева на момент триажа: `ls openspec/specs/` даёт **только**
|
||||
`intake`; в новой преамбуле у пункта `intake` ссылка есть
|
||||
(`../openspec/specs/intake/spec.md`), у пункта `pipeline` ссылки **нет** — она
|
||||
снята, потому что `docs.py check` краснел битой ссылкой. Задача предписала эту
|
||||
правку сама (`tasks.md`, 4.4).
|
||||
- Последствие: пока изменение не заархивировано, документ утверждает «Заведены
|
||||
две capability», а найти вторую читателю негде — единственная форма, в которой
|
||||
она существует, лежит в `openspec/changes/`. Если изменение уедет без
|
||||
`archive` (а `archive` — отдельный шаг и отдельная команда), расхождение
|
||||
останется постоянным, и поймать его нечем: гейт зелёный именно потому, что
|
||||
ссылку сняли.
|
||||
- Предложение: у пункта `pipeline` дописать оговорку о том, где спека лежит
|
||||
сейчас и когда переедет — «дельта в
|
||||
`openspec/changes/errors-as-instead-of-typecast/specs/pipeline/spec.md`,
|
||||
переезжает в `openspec/specs/` при архивации задачи». Одно предложение, ссылка
|
||||
на существующий файл, гейт остаётся зелёным.
|
||||
- **Действие: инлайн.**
|
||||
|
||||
---
|
||||
|
||||
## Гипотезы без доказательства
|
||||
|
||||
### Новая ветка `errors.Join` в `newSpeechKitService` не покрыта ни одним тестом
|
||||
|
||||
Понижено с **major/high** до гипотезы и, по существу, до строки в границах
|
||||
покрытия. Пришло от прохода `autotests` с настоящим оракулом
|
||||
(`go tool cover -func` → `newSpeechKitService 0.0%`; триаж перепроверил:
|
||||
`go test -coverprofile` даёт `internal/adapter/recognizer/yandex — coverage:
|
||||
0.0% of statements`, тестовых файлов в пакете нет вовсе).
|
||||
|
||||
Почему понижено: находка 4 показывает, что ветка практически недостижима.
|
||||
`grpc.NewClient` с константным корректным адресом (`operation.api.cloud.yandex.net:443`)
|
||||
и валидными TLS-credentials отказывает только на разборе target'а, а второй
|
||||
операнд `errors.Join` на этом пути всегда `nil`. То есть непокрыт код, который
|
||||
и выполняться-то не будет. **Предложенное самим проходом «вынести создание
|
||||
клиента за шов» триаж не рекомендует**: это разросшаяся абстракция в адаптере
|
||||
ради единственной недостижимой ветки, и заказывать её на основании процента
|
||||
покрытия — ровно та правка, от которой защищает потолок. Принят второй вариант
|
||||
самого же прохода: занести в границы покрытия (сделано ниже).
|
||||
|
||||
### Признака живости воркера нет: сутки тишины одинаково означают «записей не слали» и «все три воркера висят»
|
||||
|
||||
Пришло от `basics` (находка F), понижено: **изменением не создано**. Спека
|
||||
нормирует молчание пустого прогона, но само молчание стоит на инварианте
|
||||
`CLAUDE.md` «`NoopJobError` — не ошибка», который старше этой задачи.
|
||||
Оракула на «воркер висит» нет — поднять SpeechKit в тесте нечем (см. «Недоступно
|
||||
проверке»). Более того, свойство **уже заведено задачей**:
|
||||
`tasks/items/service-observability.md`, критерий завершения 1 — «По метрикам
|
||||
видно, что конвейер встал: задача висит в состоянии дольше обычного, и это
|
||||
отличимо от «работы нет»». Новой находкой не считается; чинить в этом изменении
|
||||
нечего, иначе это рост scope на целую тему наблюдаемости.
|
||||
|
||||
### Норма спеки пересказана прозой в `docs/conventions/errors.md` — второй дом факта
|
||||
|
||||
Пришло от `basics` («дешевле переделать», п. 3). Оракула нет: `openspec/config.yaml:36`
|
||||
действительно предупреждает, что «второй дом факта расходится с первым молча»,
|
||||
но новый абзац `errors.md:51-57` заканчивается словами «**Норма записана
|
||||
требованием capability `pipeline`**», то есть первый дом назван явно. Разделение
|
||||
здесь защитимо: спека говорит, что делает система, конвенция — как это пишут в
|
||||
коде. Доказательства предстоящего расхождения у меня нет, а превентивная правка
|
||||
свелась бы к спору о вкусе. Оставлено гипотезой; если расхождение когда-нибудь
|
||||
случится, эта запись — след.
|
||||
|
||||
---
|
||||
|
||||
## Promote candidates
|
||||
|
||||
- **Форма `msg` в журнале.** `logger.Error("failed to close audio recognizer", …)`
|
||||
(`main.go:126`) — предложение, а не короткая категория, вопреки
|
||||
`docs/conventions/logging.md:32-37`. Пришло от `code` (находка C) и **отсеяно
|
||||
как находка**: `logging.md:44` уже несёт строку «*Расхождение:* сегодня `msg` —
|
||||
предложение вида `Starting conversion job`», и весь корпус журнала написан в
|
||||
этой форме; правка одной строки сделает журнал неоднороднее, а не однороднее.
|
||||
Это претензия на правило, а не на этот код: либо сканер формы `msg`, либо
|
||||
отдельная задача на разовую миграцию всего корпуса.
|
||||
- **Устаревание утверждения внутри документа не механизировано.** `docs.py check`
|
||||
ловит битые ссылки и раскладку, но не ловит ситуацию находки 2: файл на месте,
|
||||
ссылка цела, неверным стало утверждение о его содержимом. Кандидат:
|
||||
проверка «строка «*Расхождение:*» и упоминание расхождения в чужом файле
|
||||
живут парой» либо явные якоря вместо ссылок на файл целиком.
|
||||
- **Покрытие изменённых строк не считается ничем** (`CLAUDE.md`, «Гейт», сказано
|
||||
прямо). Именно поэтому проход `autotests` вынужден был звать `go tool cover`
|
||||
руками, а решение «покрывать или занести в границы» принималось на глаз.
|
||||
Кандидат в шаг гейта, а не в находку ревью.
|
||||
|
||||
---
|
||||
|
||||
## Границы покрытия
|
||||
|
||||
### План: темы, их дома и глубины
|
||||
|
||||
Все шесть тем плана (`requirements`, `autotests`, `conventions`, `architecture`,
|
||||
`security`, `operations`) имели дом и вернули отчёт — см. таблицу в сводке. Тем
|
||||
без дома в плане нет. Своих тем проекта нет.
|
||||
|
||||
### Какие проходы запускались и в каком режиме
|
||||
|
||||
Метка `medium`, режим «по графу». Запущены: `review-autotests`, `review-specs`,
|
||||
`review-code`, `review-basics`, `review-triage`. Состав соответствует плану
|
||||
`review-scope`.
|
||||
|
||||
### Какие проходы не запускались и почему
|
||||
|
||||
- Отдельные проходы **`architecture` и `operations`** — по метке: на `medium`
|
||||
эти темы разбираются внутри `review-basics` меньшей глубиной. `review-code`
|
||||
подал сигнал, что метка, вероятно, занижена (см. сводку); при `large` эти два
|
||||
прохода шли бы отдельно и глубже.
|
||||
- Прохода **идиоматичности** в конвейере нет — упразднён.
|
||||
- Прохода **независимой реализации** в конвейере нет — снят по стоимости.
|
||||
|
||||
### Что каждый запущенный проход не мог проверить в принципе
|
||||
|
||||
- `autotests` — судит оракулы и их способность краснеть, но не судит, верна ли
|
||||
сама норма, которую они проверяют; поведение под реальным потоком не
|
||||
воспроизводит.
|
||||
- `specs` — судит соответствие нормы и кода, но не судит, нужна ли норма и не
|
||||
дорога ли она; альтернативной формулировки требования не строит.
|
||||
- `code` — читает дифф; поведения системы целиком, в сборе и под нагрузкой, не
|
||||
наблюдает.
|
||||
- `basics` — тремя темами на малой глубине; ни одну из них до дна не доводит по
|
||||
построению.
|
||||
- `triage` (я) — **ничего нового не нахожу по определению**: я не читаю код в
|
||||
поисках дефектов, я работаю с чужими выводами. Пропуск любого прохода — мой
|
||||
пропуск тоже, и единственное, что я могу с этим сделать, — назвать его
|
||||
поимённо, что и сделано выше.
|
||||
|
||||
### Что осталось целиком на человеке
|
||||
|
||||
**Не проверит ни один проход** (`docs/review.md:159-166`):
|
||||
|
||||
- `operations`: поведение внешних сервисов под нагрузкой и на границах —
|
||||
SpeechKit и Object Storage поднять в тесте нечем;
|
||||
- `operations`: реальный профиль нагрузки. Проект работает на единицах записей в
|
||||
день, и утверждения о росте остаются условиями, а не замерами;
|
||||
- `security`: стойкость `ffmpeg` к вредоносному входу — разбор чужого формата
|
||||
отдан внешней программе, и она вне нашей границы.
|
||||
|
||||
**Перестали проверять сознательно** (`docs/review.md:168-175`):
|
||||
|
||||
- `autotests`: разбор вывода настоящего `ffprobe`. Проверки приёма звали его до
|
||||
2026-08-11 — правда, звали так, что он всегда отказывал, — а теперь получают
|
||||
длительность от подставного источника. Своего теста у
|
||||
`adapter/metaviewer/ffmpeg` нет; решение и его цена — в
|
||||
`docs/adr/ADR-2026-08-11-stub-adapters-in-tests.md`.
|
||||
|
||||
**Плюс этим прогоном:**
|
||||
|
||||
- `internal/adapter/recognizer/yandex` — покрытие **0.0 %**, тестовых файлов в
|
||||
пакете нет вовсе; новая ветка `errors.Join` в `newSpeechKitService:53-63` не
|
||||
исполняется ни одной проверкой. Заносится сюда сознательно вместо заведения
|
||||
шва (см. гипотезы);
|
||||
- `main.go` — пакет `main` в проекте никогда не тестировался, шва нет; новый
|
||||
`defer` с логированием отказа закрытия проверен только чтением (`tasks.md`,
|
||||
2.4: между постановкой `defer` и завершением `main` нет `os.Exit`);
|
||||
- реальный путь `NoopJobError` от SQLite-репозитория (не от подставного) —
|
||||
проверки обоих звеньев работают на заглушках.
|
||||
|
||||
**Общее, что не проверяет никто:** история инцидентов; поведение под реальным
|
||||
потоком; поведение внешних систем в их версиях (утверждения о gRPC в находке 4
|
||||
сняты с исходников `v1.74.2` — на другой версии их надо перепроверять);
|
||||
завязка потребителей на текущее поведение; вопрос «а нужна ли эта
|
||||
функциональность вообще».
|
||||
|
||||
### Каких документов проекта не хватило
|
||||
|
||||
- **`docs/adr/` по теме этого изменения — записи нет.** Решение «`NoopJobError`
|
||||
и `JobNotFoundError` остаются типизированными, а не становятся sentinel»
|
||||
принято в `design.md` (Решение 1) и живёт только там, в документе изменения,
|
||||
который после архивации уедет в `openspec/changes/archive/`. Через полгода
|
||||
обоснование придётся выводить заново.
|
||||
- **Строка «*Расхождение:*» не имеет владельца и срока по построению**
|
||||
(`docs/conventions/README.md:27-29`). Из-за этого долг «одна запись отказа —
|
||||
две строки журнала» нельзя ни просрочить, ни закрыть — что и породило находку 1.
|
||||
- Прочих пробелов проходы не заявили. `docs/security.md`, `docs/database.md`,
|
||||
`docs/passport.md`, `docs/conventions/*` на месте и периметр называют;
|
||||
инварианты в `CLAUDE.md` есть и снабжены severity — деградации по этому
|
||||
разряду на этом прогоне не было.
|
||||
|
||||
### Сработавшие потолки — по строке на проход
|
||||
|
||||
- `review-basics` — потолок **2/4**, показано 2 находки + 3 пункта «дешевле
|
||||
переделать до мерджа». Потолок не срабатывал, за срезом ничего не осталось.
|
||||
Проход сообщил это сам.
|
||||
- `review-autotests` — **о своём потолке не сообщил**; показана 1 находка и 1
|
||||
отклонённая гипотеза. Это находка о прогоне: сколько осталось за срезом,
|
||||
установить нечем.
|
||||
- `review-specs` — **о своём потолке не сообщил**; показано 2 находки. То же.
|
||||
- `review-code` — **о своём потолке не сообщил**; показано 4 находки при
|
||||
раздельных потолках половин `conventions` и `техника`. То же.
|
||||
- `review-triage` (я) — потолки 3 и 4, занято 2 и 3. **Потолок не срабатывал,
|
||||
ничего не срезано, ничего не выброшено молча.** Единственная отсеянная находка
|
||||
(форма `msg`) названа поимённо в `Promote candidates` с причиной отсева.
|
||||
|
||||
### Четыре строки триажа: чего в конвейере нет вовсе
|
||||
|
||||
1. **Решения проекта не сверялись.** `docs/adr/` — процессный документ, прогон
|
||||
его не открывает. Изменение вводит новую capability `pipeline` и меняет
|
||||
политику линтера на весь проект; расходится ли это с записанными ADR, ни один
|
||||
проход не проверял. Ловит такое сверка документации — скилл
|
||||
`av-dev-docs:healthcheck`, и звать его надо руками (`CLAUDE.md`, «Гейт»:
|
||||
«согласованность документов между собой и с кодом … звать его надо руками»).
|
||||
2. **Записанные наблюдения проекта не использовались.** `docs/research/` — тоже
|
||||
процессный. Каждое число в этом отчёте снято на этом прогоне приложенной
|
||||
командой: `0 issues` от `golangci-lint run`, `0.0 % of statements` от
|
||||
`go test -coverprofile`, «2 записи ERROR» от прогона с `-overlay`. Чисел без
|
||||
команды замера в отчёте нет.
|
||||
3. **Поимённая сверка с руководствами по стилю Go не задавалась ни одним
|
||||
проходом.** `errors.Join` в конструкторе, `errors.As` с указателем на
|
||||
указатель, `defer` с телом вместо голого вызова — все три конструкции судились
|
||||
по внутренним конвенциям проекта и по линтеру. Различение «идиоматично против
|
||||
просто распространено» не спрашивал никто с тех пор, как упразднён проход про
|
||||
идиоматичность.
|
||||
4. **Альтернативной реализации, с которой можно сдиффить решения, у конвейера
|
||||
нет.** Проход независимой реализации снят по стоимости, а не по замеру. Вопрос
|
||||
«а не решается ли задача «признак не ломается обёрткой» иначе — например,
|
||||
sentinel-значениями, как сам `design.md` рассматривает в Решении 1» никем
|
||||
независимо не проверялся: рассмотрел и отверг его автор дизайна, и ревью
|
||||
сверялось с его же рассуждением.
|
||||
|
||||
Метка `medium`, поэтому пятая строка про `small` не применяется: темы `security`,
|
||||
`operations` и `architecture` разбирались проходом `basics` по своим домам, а не
|
||||
только по инвариантам `CLAUDE.md`.
|
||||
|
||||
---
|
||||
|
||||
**Формулировка «критичных проблем не обнаружено» в этом отчёте не употребляется
|
||||
и не подразумевается.** `critical` не выставлен ни одной находке по конкретной
|
||||
причине: построенного пути к потере данных, порче или утечке секрета ни один
|
||||
проход не предъявил, а `critical` без оракула или построенного пути не
|
||||
существует. Что именно осталось непроверенным — перечислено выше поимённо.
|
||||
+76
@@ -0,0 +1,76 @@
|
||||
## Purpose
|
||||
|
||||
Конвейер расшифровки: как задача движется по состояниям, что делает воркер,
|
||||
когда работы нет, и что считается отказом шага.
|
||||
|
||||
Описан пока **только пустой прогон воркера** — тот, что нормируют проверки
|
||||
пакета `internal/controller/worker` и перевод признака в `internal/service`.
|
||||
Сознательно не описаны переходы состояний и цепочка `created → converted →
|
||||
transcribe → done | failed`, захват задачи и срок его протухания, отмена
|
||||
контекста посреди шага, освобождение ресурсов внешних клиентов. Это не значит,
|
||||
что такого поведения нет: оно живёт в коде, а требования на него не написаны,
|
||||
потому что требование без проверки — предположение, а не норма. Первая задача,
|
||||
которая трогает любое из перечисленного, дописывает его сюда.
|
||||
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Пустой прогон воркера — не отказ
|
||||
|
||||
Воркер SHALL отличать «работы в этом состоянии сейчас нет» от отказа шага. На
|
||||
пустом прогоне он MUST не считать прогон отказом: не увеличивать счётчик работы
|
||||
и не писать о нём на уровне владельца сервиса. Признак пустого прогона MUST
|
||||
узнаваться по смыслу значения, а не по его точной форме, и MUST переживать
|
||||
пояснения, добавленные к этому значению на любом промежуточном шаге пути.
|
||||
|
||||
Требование стоит на инварианте проекта «`NoopJobError` — не ошибка»: три воркера
|
||||
опрашивают базу раз в секунду, и пустой прогон, принятый за отказ, даёт три
|
||||
записи отказа в секунду и столько же засчитанных сбоев, которых не было.
|
||||
|
||||
Признак пустого прогона MUST рождаться только ответом хранилища на опрос этим же
|
||||
шагом. Слой, придающий отказу собственный смысл, MUST не сохранять чужой признак
|
||||
в цепочке своей ошибки. Узнавание по смыслу видит признак на любой глубине, и
|
||||
отказ, к которому признак примешался, воркер зачёл бы пустым прогоном: задача
|
||||
осталась бы в своём состоянии и переопрашивалась раз в секунду без единой записи
|
||||
— ровно то, что запрещает инвариант «Принятая запись не теряется молча».
|
||||
|
||||
Отказ шага, наоборот, MUST быть виден владельцу сервиса записью в журнале и MUST
|
||||
быть засчитан в счётчик работы с пометкой отказа.
|
||||
|
||||
**Сколько раз он записывается и каким уровнем — это требование не нормирует, и
|
||||
умолчанием тут считать нечего.** Сегодня один отказ даёт две записи: пишет шаг
|
||||
конвейера и следом воркер, — а уровень стоит `ERROR` там, где конвенция просит
|
||||
`WARN` для повторяющегося сбоя фонового цикла. И то и другое записано долгом в
|
||||
`docs/conventions/logging.md`, раздел «Ошибки», строкой «Расхождение, и оно
|
||||
системное». Долгом оно и остаётся: требование, объявившее одиночную запись
|
||||
нормой, сделало бы недостижимое обязательным, а требование, объявившее нормой
|
||||
двойную, — закрыло бы долг контрактом. Задача, которая возьмётся за этот долг,
|
||||
дописывает норму сюда.
|
||||
|
||||
#### Scenario: Работы в состоянии нет
|
||||
|
||||
- **GIVEN** ни одной задачи в опрашиваемом состоянии нет
|
||||
- **WHEN** воркер делает свой прогон
|
||||
- **THEN** на уровне владельца сервиса об этом прогоне не пишется ничего
|
||||
- **AND** счётчик работы воркера не растёт
|
||||
|
||||
#### Scenario: Признак пустого прогона дошёл с пояснением
|
||||
|
||||
- **GIVEN** работы в опрашиваемом состоянии нет
|
||||
- **AND** промежуточный шаг добавил к этому признаку своё пояснение
|
||||
- **WHEN** воркер делает свой прогон
|
||||
- **THEN** прогон по-прежнему считается пустым: счётчик не растёт, записи на
|
||||
уровне владельца нет
|
||||
|
||||
#### Scenario: Шаг отказал
|
||||
|
||||
- **GIVEN** шаг конвейера вернул отказ
|
||||
- **WHEN** воркер завершает прогон
|
||||
- **THEN** отказ виден владельцу сервиса записью в журнале
|
||||
- **AND** счётчик работы воркера растёт с пометкой отказа
|
||||
|
||||
#### Scenario: Шаг сделал работу
|
||||
|
||||
- **GIVEN** шаг конвейера отработал задачу без отказа
|
||||
- **WHEN** воркер завершает прогон
|
||||
- **THEN** счётчик работы воркера растёт с пометкой успеха
|
||||
- **AND** записи об отказе в журнале нет
|
||||
@@ -0,0 +1,100 @@
|
||||
## 1. Признак узнаётся по смыслу
|
||||
|
||||
- [x] 1.1 В `internal/controller/worker/worker.go:51` заменить приведение
|
||||
`err.(*contract.NoopJobError)` на `errors.As` с целью типа
|
||||
`*contract.NoopJobError`; порядок ветвей (счётчик, затем журнал) сохранить.
|
||||
- [x] 1.2 В `internal/service/transcribe.go:392` заменить приведение
|
||||
`err.(*contract.JobNotFoundError)` на `errors.As`.
|
||||
- [x] 1.3 `golangci-lint run` не даёт замечаний `errorlint` — проверить прогоном.
|
||||
|
||||
## 2. Отказ закрытия не теряется
|
||||
|
||||
- [x] 2.1 В `.golangci.yml` включить `errcheck.check-blank: true`. Без этого
|
||||
`_ = conn.Close()` снимает замечание, и оракул раздела 4 не различает годную
|
||||
реализацию от негодной. Прогон обязан остаться на прежних 4 замечаниях — новых
|
||||
мест правило не открывает (проверено при разборе дизайна).
|
||||
- [x] 2.2 В `internal/adapter/recognizer/yandex/speechkit.go:55` собрать отказ
|
||||
закрытия `sttConn` с отказом соединения через `errors.Join`; `nil` от закрытия
|
||||
форму ошибки не меняет.
|
||||
- [x] 2.3 В `main.go:124` заменить `defer recognizer.Close()` на `defer` с телом,
|
||||
пишущим отказ закрытия в журнал.
|
||||
- [x] 2.4 Проверить, что между постановкой этого `defer` и штатным завершением
|
||||
`main` нет вызова `os.Exit`: иначе запись не появится (риск из `design.md`).
|
||||
- [x] 2.5 `golangci-lint run` не даёт замечаний `errcheck` — проверить прогоном.
|
||||
- [x] 2.6 Мутация оракула: временно заменить оба места на `_ = …Close()` —
|
||||
`golangci-lint run` обязан покраснеть обоими. Не покраснел — оракул критерия 4
|
||||
не работает, и шаг 2.1 сделан неверно. Восстановить код после проверки.
|
||||
|
||||
## 3. Оракул на пустой прогон — оба звена пути
|
||||
|
||||
- [x] 3.1 Завести первый тест пакета `internal/controller/worker`; оснастку
|
||||
(подменный журнал, чтение счётчика из реестра метрик) взять по образцу тестов
|
||||
`internal/service` и `internal/controller/http`, а не писать заново.
|
||||
- [x] 3.2 Тест: воркер, чья работа вернула `*contract.NoopJobError`, **обёрнутый**
|
||||
`fmt.Errorf("…: %w", err)`, не пишет в журнал ни одной записи.
|
||||
- [x] 3.3 Тот же тест: счётчик `transcriber_worker_job_count` для этого воркера
|
||||
не изменился — значение читается до и после прогона.
|
||||
- [x] 3.4 Тест отказа: обычный отказ шага даёт запись в журнал и рост счётчика с
|
||||
пометкой отказа — иначе оракул зелен на коде, который не считает отказом ничего.
|
||||
- [x] 3.5 Тест успеха: прогон без отказа растит счётчик с пометкой успеха и не
|
||||
пишет об отказе. Без него реализация, снявшая счёт успешных прогонов, проходит
|
||||
все проверки, а доля отказов перестаёт считаться.
|
||||
- [x] 3.6 **Второе звено пути**: тест в `internal/service` на `findJob` с
|
||||
подставным репозиторием, возвращающим `fmt.Errorf("…: %w",
|
||||
&contract.JobNotFoundError{…})` — ожидание `*contract.NoopJobError`. Без него
|
||||
правка 1.2 принимается только линтером, а линтер проверяет форму, не смысл.
|
||||
- [x] 3.7 Мутация, поимённо по обеим строкам: вернуть приведение типа в
|
||||
`worker.go` — краснеют 3.2 и 3.3; вернуть приведение (или подставить
|
||||
несовпадающую цель `errors.As`) в `transcribe.go` — краснеет 3.6. Обе мутации
|
||||
обязаны покраснеть по отдельности. Восстановить код после проверки.
|
||||
|
||||
## 4. Учёт
|
||||
|
||||
- [x] 4.1 `task gate` зелёный целиком; в `CLAUDE.md`, разделе «Гейт», снять
|
||||
запись об известном отказе `golangci-lint` и о задаче
|
||||
`errors-as-instead-of-typecast`.
|
||||
- [x] 4.2 В `docs/conventions/errors.md` снять пометку «*Расхождение, и оно
|
||||
опасно:*» о приведении типа и строку преамбулы «доменные ошибки проверяются
|
||||
приведением типа»; прочие расхождения того файла не трогать.
|
||||
- [x] 4.3 Дописать `## Purpose` в спеку `pipeline` — что это за capability и что
|
||||
сознательно **не** описано (переходы состояний, захват, срок протухания,
|
||||
отмена контекста, освобождение ресурсов). Без него следующий автор не отличит
|
||||
«остальное не нормировано» от «остального не бывает»; валидатор этого не ловит.
|
||||
- [x] 4.4 Поправить преамбулу `docs/architecture.md`: заведены две capability,
|
||||
`intake` и `pipeline`, и в `pipeline` описан только пустой прогон воркера.
|
||||
Маркеры `<!-- канон: поведение → openspec/specs/pipeline -->` на строках про
|
||||
идемпотентность и цепочку состояний оставить долгом — но так, чтобы их нельзя
|
||||
было прочесть как «уже переехало».
|
||||
- [x] 4.5 `openspec validate --strict errors-as-instead-of-typecast` проходит.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
Дословно из записи задачи `tasks/items/errors-as-instead-of-typecast.md`:
|
||||
|
||||
- Обе проверки идут через `errors.As` либо через `errors.Is` по sentinel.
|
||||
Оракул — `golangci-lint run` не даёт замечаний `errorlint`.
|
||||
- Обёртка `fmt.Errorf("…: %w", err)` в середине пути не ломает распознавание.
|
||||
Оракул — тест: обёрнутый `NoopJobError` воркер по-прежнему считает пустым
|
||||
прогоном и не пишет ни лога, ни метрики.
|
||||
- Метрика `transcriber_worker_job_count` на пустом прогоне не растёт. Оракул —
|
||||
тот же тест, проверка значения счётчика до и после.
|
||||
- Отказ `Close` не теряется молча: он либо возвращается вызывающему, либо
|
||||
попадает в лог. Оракул — `golangci-lint run` не даёт замечаний `errcheck`, и
|
||||
`task gate` зелёный целиком.
|
||||
|
||||
Добавлено разбором дизайна (рубрика прохода `rubric`, потолок пунктов не
|
||||
применялся):
|
||||
|
||||
- Каждый оракул способен покраснеть на негодной реализации. Проверяется
|
||||
мутацией — шаги 2.6 и 3.7; критерий, чей единственный оракул молчание линтера,
|
||||
годится только когда линтер краснеет на **всех** негодных реализациях.
|
||||
- Признак пустого прогона распознаётся на **обоих** звеньях пути, и каждое звено
|
||||
имеет свою падающую проверку.
|
||||
- Классификация отказа не зависит от текста ошибки: ни подстроки `Error()`, ни
|
||||
точной формы значения.
|
||||
- Норма не закрепляет контрактом то, что конвенции помечают строкой
|
||||
«Расхождение», и не объявляет обязательным недостижимое: число записей об
|
||||
отказе и уровень журнала остаются долгом `docs/conventions/logging.md`, а
|
||||
требование их не нормирует ни в ту, ни в другую сторону. Проверено ревью кода:
|
||||
первая редакция требовала «ровно один раз», чему код не соответствовал с
|
||||
первого дня.
|
||||
@@ -0,0 +1,78 @@
|
||||
# pipeline Specification
|
||||
|
||||
## Purpose
|
||||
|
||||
Конвейер расшифровки: как задача движется по состояниям, что делает воркер,
|
||||
когда работы нет, и что считается отказом шага.
|
||||
|
||||
Описан пока **только пустой прогон воркера** — тот, что нормируют проверки
|
||||
пакета `internal/controller/worker` и перевод признака в `internal/service`.
|
||||
Сознательно не описаны переходы состояний и цепочка `created → converted →
|
||||
transcribe → done | failed`, захват задачи и срок его протухания, отмена
|
||||
контекста посреди шага, освобождение ресурсов внешних клиентов. Это не значит,
|
||||
что такого поведения нет: оно живёт в коде, а требования на него не написаны,
|
||||
потому что требование без проверки — предположение, а не норма. Первая задача,
|
||||
которая трогает любое из перечисленного, дописывает его сюда.
|
||||
|
||||
## Requirements
|
||||
### Requirement: Пустой прогон воркера — не отказ
|
||||
|
||||
Воркер SHALL отличать «работы в этом состоянии сейчас нет» от отказа шага. На
|
||||
пустом прогоне он MUST не считать прогон отказом: не увеличивать счётчик работы
|
||||
и не писать о нём на уровне владельца сервиса. Признак пустого прогона MUST
|
||||
узнаваться по смыслу значения, а не по его точной форме, и MUST переживать
|
||||
пояснения, добавленные к этому значению на любом промежуточном шаге пути.
|
||||
|
||||
Требование стоит на инварианте проекта «`NoopJobError` — не ошибка»: три воркера
|
||||
опрашивают базу раз в секунду, и пустой прогон, принятый за отказ, даёт три
|
||||
записи отказа в секунду и столько же засчитанных сбоев, которых не было.
|
||||
|
||||
Признак пустого прогона MUST рождаться только ответом хранилища на опрос этим же
|
||||
шагом. Слой, придающий отказу собственный смысл, MUST не сохранять чужой признак
|
||||
в цепочке своей ошибки. Воркер узнаёт признак по смыслу на любой глубине, поэтому
|
||||
отказ, к которому признак примешался, тоже зачёл бы пустым прогоном: задача
|
||||
осталась бы в своём состоянии и переопрашивалась раз в секунду без единой записи
|
||||
— ровно то, что запрещает инвариант «Принятая запись не теряется молча».
|
||||
|
||||
Отказ шага, наоборот, MUST быть виден владельцу сервиса записью в журнале и MUST
|
||||
быть засчитан в счётчик работы с пометкой отказа.
|
||||
|
||||
**Сколько раз он записывается и каким уровнем — это требование не нормирует, и
|
||||
умолчанием тут считать нечего.** Сегодня один отказ даёт две записи: пишет шаг
|
||||
конвейера и следом воркер, — а уровень стоит `ERROR` там, где конвенция просит
|
||||
`WARN` для повторяющегося сбоя фонового цикла. И то и другое записано долгом в
|
||||
`docs/conventions/logging.md`, раздел «Ошибки», строкой «Расхождение, и оно
|
||||
системное». Долгом оно и остаётся: требование, объявившее одиночную запись
|
||||
нормой, сделало бы недостижимое обязательным, а требование, объявившее нормой
|
||||
двойную, — закрыло бы долг контрактом. Задача, которая возьмётся за этот долг,
|
||||
дописывает норму сюда.
|
||||
|
||||
#### Scenario: Работы в состоянии нет
|
||||
|
||||
- **GIVEN** ни одной задачи в опрашиваемом состоянии нет
|
||||
- **WHEN** воркер делает свой прогон
|
||||
- **THEN** на уровне владельца сервиса об этом прогоне не пишется ничего
|
||||
- **AND** счётчик работы воркера не растёт
|
||||
|
||||
#### Scenario: Признак пустого прогона дошёл с пояснением
|
||||
|
||||
- **GIVEN** работы в опрашиваемом состоянии нет
|
||||
- **AND** промежуточный шаг добавил к этому признаку своё пояснение
|
||||
- **WHEN** воркер делает свой прогон
|
||||
- **THEN** прогон по-прежнему считается пустым: счётчик не растёт, записи на
|
||||
уровне владельца нет
|
||||
|
||||
#### Scenario: Шаг отказал
|
||||
|
||||
- **GIVEN** шаг конвейера вернул отказ
|
||||
- **WHEN** воркер завершает прогон
|
||||
- **THEN** отказ виден владельцу сервиса записью в журнале
|
||||
- **AND** счётчик работы воркера растёт с пометкой отказа
|
||||
|
||||
#### Scenario: Шаг сделал работу
|
||||
|
||||
- **GIVEN** шаг конвейера отработал задачу без отказа
|
||||
- **WHEN** воркер завершает прогон
|
||||
- **THEN** счётчик работы воркера растёт с пометкой успеха
|
||||
- **AND** записи об отказе в журнале нет
|
||||
|
||||
Reference in New Issue
Block a user