fix(ptica): job_settings не отравляет чужую сессию при сбое БД (#2464 кластер A) #2937

Merged

bot-backend merged 1 commit from fix/2464a-job-settings-savepoint into main

2026-08-19 15:42:40 +00:00

bot-backend commented

2026-08-19 15:17:51 +00:00

Collaborator

Дефект

get_all / get_one глотают ошибку db.execute и возвращают fallback — поведение задумано как graceful. Но сессию им отдаёт вызывающий: admin-ручки (admin_jobs.py), beat_schedule.py:71, и get_setting_value из cadastre_fetch.py / nspd_geo.py.

На Postgres упавший запрос оставляет транзакцию в aborted-состоянии. Все последующие запросы этой же сессии падают с current transaction is aborted, commands ignored until end of transaction block — то есть падает не тот, кто виноват, а следующий блок кода.

Голый db.rollback() здесь запрещён: он снёс бы незакоммиченную работу вызывающего. Средство — SAVEPOINT вокруг самого execute, ровно как в уже закрытом пункте того же кластера (developer_attribution.py:152, где это записано в комментарии).

Мок воспроизводит Postgres — и это не педантизм

Тест моделирует настоящую семантику: упавший запрос переводит сессию в aborted, дальнейшие падают, откат SAVEPOINT восстанавливает.

Пришлось так, потому что существующий образец в репозитории проверки не даёт. В tests/test_saturation.py мок begin_nested — пустой контекст-менеджер, aborted-состояния у него нет, поэтому второй execute проходит независимо от того, есть SAVEPOINT в коде или нет.

Проверил экспериментом, а не рассуждением: временно снял with db.begin_nested(): из saturation.py:247 —

19 passed          КОД ВОЗВРАТА = 0

Все тесты файла зелёные при снятой защите. Тест, который не может покраснеть, когда убираешь то, что он охраняет, охраны не проверяет.

Поэтому здесь добавлен ещё и контроль на сам мок — test_mock_actually_poisons_without_a_savepoint. Без него две главные проверки были бы зелёными по построению.

Проверка

тест	`origin/main`	с правкой
`test_get_all_leaves_the_caller_session_usable`	красный: `AbortedTransactionError: current transaction is aborted, commands ignored until end of transaction block`	зелёный
`test_get_one_leaves_the_caller_session_usable`	красный, тот же текст	зелёный
`test_mock_actually_poisons_without_a_savepoint`	зелёный	зелёный
`test_healthy_session_is_not_disturbed`	зелёный	зелёный

Последний контроль важен отдельно: он требует, чтобы при исправной БД возвращались данные из неё, а не fallback — иначе «починка» могла бы свестись к тому, что fallback отдаётся всегда.

pytest tests/services: 3063 passed, 14 skipped, rc=0
pytest tests/api/v1/test_admin_jobs_settings.py tests/test_saturation.py: 21 passed, rc=0

Побочно: беззубые тесты кластера A

Находка про test_saturation.py касается не только его. Пять пунктов кластера A отмечены закрытыми, и если их проверяли моками того же устройства, отметки говорят о наличии кода, а не о работе защиты. Это стоит перепроверить тем же приёмом — снять SAVEPOINT и посмотреть, покраснеет ли тест. Отдельной задачей заводить не стал, напишу в эпик.

Refs #2464

## Дефект `get_all` / `get_one` глотают ошибку `db.execute` и возвращают fallback — поведение задумано как graceful. Но сессию им отдаёт **вызывающий**: admin-ручки (`admin_jobs.py`), `beat_schedule.py:71`, и `get_setting_value` из `cadastre_fetch.py` / `nspd_geo.py`. На Postgres упавший запрос оставляет транзакцию в aborted-состоянии. Все последующие запросы **этой же сессии** падают с `current transaction is aborted, commands ignored until end of transaction block` — то есть падает не тот, кто виноват, а следующий блок кода. Голый `db.rollback()` здесь запрещён: он снёс бы незакоммиченную работу вызывающего. Средство — SAVEPOINT вокруг самого `execute`, ровно как в уже закрытом пункте того же кластера (`developer_attribution.py:152`, где это записано в комментарии). ## Мок воспроизводит Postgres — и это не педантизм Тест моделирует настоящую семантику: упавший запрос переводит сессию в `aborted`, дальнейшие падают, откат SAVEPOINT восстанавливает. Пришлось так, потому что **существующий образец в репозитории проверки не даёт**. В `tests/test_saturation.py` мок `begin_nested` — пустой контекст-менеджер, aborted-состояния у него нет, поэтому второй `execute` проходит независимо от того, есть SAVEPOINT в коде или нет. Проверил экспериментом, а не рассуждением: временно снял `with db.begin_nested():` из `saturation.py:247` — ``` 19 passed КОД ВОЗВРАТА = 0 ``` Все тесты файла зелёные при снятой защите. Тест, который не может покраснеть, когда убираешь то, что он охраняет, охраны не проверяет. Поэтому здесь добавлен ещё и **контроль на сам мок** — `test_mock_actually_poisons_without_a_savepoint`. Без него две главные проверки были бы зелёными по построению. ## Проверка | тест | `origin/main` | с правкой | |---|---|---| | `test_get_all_leaves_the_caller_session_usable` | **красный**: `AbortedTransactionError: current transaction is aborted, commands ignored until end of transaction block` | зелёный | | `test_get_one_leaves_the_caller_session_usable` | **красный**, тот же текст | зелёный | | `test_mock_actually_poisons_without_a_savepoint` | зелёный | зелёный | | `test_healthy_session_is_not_disturbed` | зелёный | зелёный | Последний контроль важен отдельно: он требует, чтобы при исправной БД возвращались данные **из неё**, а не fallback — иначе «починка» могла бы свестись к тому, что fallback отдаётся всегда. `pytest tests/services`: **3063 passed, 14 skipped, rc=0** `pytest tests/api/v1/test_admin_jobs_settings.py tests/test_saturation.py`: **21 passed, rc=0** ## Побочно: беззубые тесты кластера A Находка про `test_saturation.py` касается не только его. Пять пунктов кластера A отмечены закрытыми, и если их проверяли моками того же устройства, отметки говорят о наличии кода, а не о работе защиты. Это стоит перепроверить тем же приёмом — снять SAVEPOINT и посмотреть, покраснеет ли тест. Отдельной задачей заводить не стал, напишу в эпик. Refs #2464

bot-backend added 1 commit 2026-08-19 15:17:52 +00:00

fix(ptica): job_settings не отравляет чужую сессию при сбое БД (#2464 кластер A)

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI / changes (pull_request) Successful in 10s

Details

CI Trade-In / backend-tests (pull_request) Has been skipped

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Successful in 2m10s

Details

CI / backend-tests (pull_request) Successful in 17m14s

Details

4c0abb8a0d

get_all/get_one глотают ошибку db.execute и возвращают fallback, но сессию им
отдаёт вызывающий: admin-ручки, beat_schedule, и get_setting_value из
cadastre_fetch/nspd_geo. На Postgres упавший execute оставляет транзакцию в
aborted-состоянии — все последующие запросы ЭТОЙ ЖЕ сессии падают с «current
transaction is aborted», то есть падает не тот, кто виноват.

Голый db.rollback() здесь запрещён: он снёс бы незакоммиченную работу
вызывающего. Средство — SAVEPOINT вокруг самого execute, как в уже закрытом
пункте того же кластера (developer_attribution.py:152).

Мок в тесте ВОСПРОИЗВОДИТ семантику Postgres: упавший запрос переводит сессию в
aborted, дальнейшие падают, откат SAVEPOINT восстанавливает. Это не педантизм.
Существующий образец в tests/test_saturation.py устроен иначе — begin_nested там
пустой контекст-менеджер без aborted-состояния, — и проверка на отравление
проходит независимо от наличия защиты. Проверено экспериментом: со СНЯТЫМ
SAVEPOINT в saturation.py все 19 тестов файла зелёные. Тест, который не может
покраснеть при снятии охраняемого, охраны не проверяет.

Поэтому здесь есть ещё и контроль на сам мок (test_mock_actually_poisons_
without_a_savepoint): без него проверки были бы зелёными по построению.

Тесты: 2 красных на origin/main с настоящим текстом ошибки Postgres
(«current transaction is aborted, commands ignored until end of transaction
block»), 2 контроля зелёные с обеих сторон.

pytest tests/services: 3063 passed, 14 skipped, rc=0