(high) Sber pull: пойманный сбой UPSERT отравляет Session — нет db.rollback(), все оставшиеся серии каскадно падают #1345

Closed
opened 2026-06-14 18:08:47 +00:00 by bot-backend · 1 comment
Collaborator

Описание

pull_sber_indices итерирует по парам (ref_area × dashboard), делая per-row db.execute(...) UPSERT и per-series db.commit(). Если любой db.execute бросит исключение (constraint/type-ошибка, либо DB-уровневая ошибка соединения) — оно перехватывается bare except Exception (строка 439), который только логирует и инкрементирует counters["errors"], но не вызывает db.rollback(). PostgreSQL переводит соединение в aborted-transaction state, поэтому каждый последующий db.execute на том же Session (все оставшиеся города/дашборды) падает с InFailedSqlTransaction и так же молча проглатывается. Одна плохая строка стирает весь pull-run вместо пропуска одной серии.

Корень

Общий Session конфигурируется с autocommit=False (SQLAlchemy 2.x autobegin). После неоткатанного DB-исключения соединение в aborted-state; без db.rollback() сессия остаётся непригодной для всех следующих итераций. Дополнительно нет per-row begin_nested() savepoint, поэтому даже одна плохая строка прерывает commit всей серии. HTTP-обработчики (httpx.HTTPStatusError строка 423, httpx.RequestError строка 431) ловят сетевые ошибки ДО db.execute — им rollback не нужен; пробел именно в bare except Exception (439), покрывающем post-execute DB-сбои.

Evidence

sber_index.py:439-445:

except Exception:
    logger.exception(
        "sber_index: unexpected error for dashboard=%s ref_area=%s — skip",
        dashboard.slug,
        ref_area,
    )
    counters["errors"] += 1   # ← нет db.rollback(); сессия остаётся aborted

Для контраста — аналогичные except-блоки в кодовой базе вызывают rollback: scrape_pipeline.py:866 (Yandex), scrape_pipeline.py:1383, 1415 (CIAN). sber_index.py уникально его опускает.

Suggested fix

Добавить db.rollback() в начало каждого except-блока, следующего за db.execute (как минимум в bare except Exception на строке 439), чтобы сбросить сессию перед следующей серией. Опционально обернуть per-row UPSERT в with db.begin_nested():, чтобы одна плохая строка не прерывала commit всей серии.

Severity-обоснование

high: одна плохая строка/серия каскадит в ноль успешных записей для всех оставшихся серий всего pull-run, молча (только counters). Снаружи run выглядит частично-успешным, хотя все post-failure серии — фактически no-op. Самый широкий blast-radius из подтверждённых находок этого класса (стирает весь прогон, а не 2 индекса из 6 или enrichment одного якоря).


week ревью 1 · multi-agent аудит 2026-06-14 · severity=high · category=error-handling

Затронутые файлы:

  • /Users/anton/Птица/gendesign/tradein-mvp/backend/app/services/sber_index.py
## Описание `pull_sber_indices` итерирует по парам (ref_area × dashboard), делая per-row `db.execute(...)` UPSERT и per-series `db.commit()`. Если любой `db.execute` бросит исключение (constraint/type-ошибка, либо DB-уровневая ошибка соединения) — оно перехватывается bare `except Exception` (строка 439), который только логирует и инкрементирует `counters["errors"]`, но **не вызывает `db.rollback()`**. PostgreSQL переводит соединение в aborted-transaction state, поэтому каждый последующий `db.execute` на том же Session (все оставшиеся города/дашборды) падает с `InFailedSqlTransaction` и так же молча проглатывается. Одна плохая строка стирает весь pull-run вместо пропуска одной серии. ## Корень Общий Session конфигурируется с `autocommit=False` (SQLAlchemy 2.x autobegin). После неоткатанного DB-исключения соединение в aborted-state; без `db.rollback()` сессия остаётся непригодной для всех следующих итераций. Дополнительно нет per-row `begin_nested()` savepoint, поэтому даже одна плохая строка прерывает commit всей серии. HTTP-обработчики (`httpx.HTTPStatusError` строка 423, `httpx.RequestError` строка 431) ловят сетевые ошибки ДО `db.execute` — им rollback не нужен; пробел именно в bare `except Exception` (439), покрывающем post-execute DB-сбои. ## Evidence `sber_index.py:439-445`: ```python except Exception: logger.exception( "sber_index: unexpected error for dashboard=%s ref_area=%s — skip", dashboard.slug, ref_area, ) counters["errors"] += 1 # ← нет db.rollback(); сессия остаётся aborted ``` Для контраста — аналогичные except-блоки в кодовой базе вызывают rollback: `scrape_pipeline.py:866` (Yandex), `scrape_pipeline.py:1383, 1415` (CIAN). `sber_index.py` уникально его опускает. ## Suggested fix Добавить `db.rollback()` в начало каждого except-блока, следующего за `db.execute` (как минимум в bare `except Exception` на строке 439), чтобы сбросить сессию перед следующей серией. Опционально обернуть per-row UPSERT в `with db.begin_nested():`, чтобы одна плохая строка не прерывала commit всей серии. ## Severity-обоснование **high**: одна плохая строка/серия каскадит в ноль успешных записей для всех оставшихся серий всего pull-run, молча (только counters). Снаружи run выглядит частично-успешным, хотя все post-failure серии — фактически no-op. Самый широкий blast-radius из подтверждённых находок этого класса (стирает весь прогон, а не 2 индекса из 6 или enrichment одного якоря). --- _week ревью 1 · multi-agent аудит 2026-06-14 · severity=**high** · category=`error-handling`_ Затронутые файлы: - `/Users/anton/Птица/gendesign/tradein-mvp/backend/app/services/sber_index.py`
bot-backend added the
week ревью 1
label 2026-06-14 18:08:47 +00:00
bot-backend added the
scope/backend
label 2026-06-15 11:01:50 +00:00
Owner

🔍 week ревью 1 — верификация (ПОДТВЕРЖДЕНО)

Текущее состояние кода: tradein-mvp/backend/app/services/sber_index.py:439-445 — bare except Exception логирует + counters["errors"] += 1, без db.rollback(). Per-row UPSERT db.execute(...) на :338-367, per-series db.commit() на :370.

Корень: После неоткатанного DB-исключения соединение в aborted-state (autobegin SQLAlchemy 2.x). Каждый последующий db.execute на той же Session (оставшиеся ref_area × dashboard) падает с InFailedSqlTransaction и снова молча проглатывается → весь оставшийся pull = no-op при счётчике errors.

Что править:

  • Добавить db.rollback() первой строкой в except Exception :439 (и желательно best-effort try: db.rollback() except: pass).
  • Опционально обернуть per-row UPSERT в with db.begin_nested(): (:338), чтобы одна плохая строка не валила commit всей серии — паттерн уже применён в matching (#849).
  • Дополнительно: benchmark-блок reconciliation на :394-421 делает db.execute(...SELECT...) внутри try/except Exception :418 — на уже отравленной сессии он тоже упадёт; rollback в основном except это закрывает.

Что посмотреть:

  • Код: контраст-эталоны с rollback — scrape_pipeline.py:866 (Yandex), :1383/:1415 (CIAN); финализаторы scrape_runs.py:mark_failed:167/mark_banned:199 (имеют defensive rollback). Тест: симулировать IntegrityError на одной серии → проверить, что последующие серии всё ещё пишутся.
  • Vault: inbox/2026-05-31-1612-resolver-794-sberindex-api.md, inbox/_merged/2026-05-31-2139-...-sberindex-per-dashboard-filter-bug...md (sberindex контекст; не про rollback напрямую).
**🔍 week ревью 1 — верификация (ПОДТВЕРЖДЕНО)** **Текущее состояние кода:** `tradein-mvp/backend/app/services/sber_index.py:439-445` — bare `except Exception` логирует + `counters["errors"] += 1`, без `db.rollback()`. Per-row UPSERT `db.execute(...)` на `:338-367`, per-series `db.commit()` на `:370`. **Корень:** После неоткатанного DB-исключения соединение в aborted-state (autobegin SQLAlchemy 2.x). Каждый последующий `db.execute` на той же Session (оставшиеся ref_area × dashboard) падает с `InFailedSqlTransaction` и снова молча проглатывается → весь оставшийся pull = no-op при счётчике `errors`. **Что править:** - Добавить `db.rollback()` первой строкой в `except Exception` `:439` (и желательно best-effort `try: db.rollback() except: pass`). - Опционально обернуть per-row UPSERT в `with db.begin_nested():` (`:338`), чтобы одна плохая строка не валила commit всей серии — паттерн уже применён в matching (#849). - Дополнительно: benchmark-блок reconciliation на `:394-421` делает `db.execute(...SELECT...)` внутри `try/except Exception` `:418` — на уже отравленной сессии он тоже упадёт; rollback в основном except это закрывает. **Что посмотреть:** - Код: контраст-эталоны с rollback — `scrape_pipeline.py:866` (Yandex), `:1383`/`:1415` (CIAN); финализаторы `scrape_runs.py:mark_failed:167`/`mark_banned:199` (имеют defensive rollback). Тест: симулировать `IntegrityError` на одной серии → проверить, что последующие серии всё ещё пишутся. - Vault: `inbox/2026-05-31-1612-resolver-794-sberindex-api.md`, `inbox/_merged/2026-05-31-2139-...-sberindex-per-dashboard-filter-bug...md` (sberindex контекст; не про rollback напрямую).
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#1345
No description provided.