From 3658434c03e46ca7f1f82233cf4b80f09990eed0 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 5 Sep 2026 22:47:08 +0500 Subject: [PATCH] =?UTF-8?q?fix(tradein/backfill):=20save-False=20=D1=83=20?= =?UTF-8?q?yandex/avito=20=D0=B1=D0=BE=D0=BB=D1=8C=D1=88=D0=B5=20=D0=BD?= =?UTF-8?q?=D0=B5=20=D1=82=D0=B5=D1=80=D1=8F=D0=B5=D1=82=20=D0=BF=D0=BE?= =?UTF-8?q?=D0=BF=D1=8B=D1=82=D0=BA=D1=83=20=E2=80=94=20attempted=20=3D=3D?= =?UTF-8?q?=20=D1=81=D1=83=D0=BC=D0=BC=D0=B0=20=D0=B8=D1=81=D1=85=D0=BE?= =?UTF-8?q?=D0=B4=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Тот же дефект, что #3332 у domclick (#3335), у обоих соседей: `if save(...): enriched += 1` без else. UPDATE, не задевший строку (объявление удалено между снимком и записью), оставлял попытку без исхода — attempted переставал сходиться с суммой исходов, и расхождение читается как потерянный отказ площадки. Разбор ВСЕХ точек выхода из цикла попыток показал, что у соседей это единственная дыра: обрыва «пул прокси пуст» у них нет (resolve_proxy_url бросает ProxyPoolExhaustedError ДО цикла), а budget/SIGTERM-брейки стоят до `attempted += 1`. Исход — failed с отдельным warning про ненайденную строку. Формы тождества разные и это не описка: у yandex blocked ⊆ failed (#3196), у avito blocked/gone/failed — непересекающиеся корзины. Тесты — по значению (числа, не «не бросило»), плюс контроль на противоположную ошибку (исход не начисляется дважды). В test_3332 добавлен запрошенный ревью кейс: транспортный сбой → пустой пул даёт attempted=2 failed=2, а не 3. Closes #3338 --- .../app/tasks/avito_detail_backfill.py | 16 + .../app/tasks/yandex_detail_backfill.py | 16 + .../test_3332_domclick_counter_identity.py | 28 ++ .../test_3338_backfill_counter_identity.py | 280 ++++++++++++++++++ 4 files changed, 340 insertions(+) create mode 100644 tradein-mvp/backend/tests/test_3338_backfill_counter_identity.py diff --git a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py index 40c041c7..2ccb60ee 100644 --- a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py +++ b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py @@ -684,6 +684,22 @@ async def run_avito_detail_backfill( ) if save_detail_enrichment(db, enrichment): counters.enriched += 1 + else: + # #3338 (та же дыра, что #3332 у domclick): карточка взята и + # разобрана, а UPDATE не задел ни одной строки — объявление + # удалено/деактивировано между снимком и записью. Попытка была, + # исхода не было: attempted переставал сходиться с + # enriched + blocked + gone + failed, и расхождение читается как + # потерянный отказ площадки. Исход failed: непрошедший UPDATE — не + # успех, не блок и не gone (снятие метит is_active=FALSE сам, по 404). + counters.failed += 1 + logger.warning( + "avito_detail_backfill: run_id=%d listing %s -- карточка " + "разобрана, но UPDATE не нашёл строку id=%s", + run_id, + source_url, + row["id"], + ) if use_curl: items_since_warm += 1 breaker.record_success() diff --git a/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py b/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py index 49454c09..05842817 100644 --- a/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py +++ b/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py @@ -458,6 +458,22 @@ async def run_yandex_detail_backfill( consecutive_blocks = 0 if save_detail_enrichment(db, listing_id, enrichment): counters.enriched += 1 + else: + # #3338 (та же дыра, что #3332 у domclick): страница взята и + # разобрана, а UPDATE не задел ни одной строки — объявление + # удалено/деактивировано между снимком и записью. Попытка была, + # исхода не было: attempted переставал сходиться с enriched + + # failed, и расхождение читается как потерянный отказ площадки. + # Исход failed: непрошедший UPDATE — не успех и не блок. + counters.failed += 1 + logger.warning( + "yandex_detail_backfill: run_id=%d listing_id=%d source_url=%s " + "-- карточка разобрана, но UPDATE не нашёл строку id=%d", + run_id, + listing_id, + source_url, + listing_id, + ) except Exception as exc: counters.failed += 1 diff --git a/tradein-mvp/backend/tests/test_3332_domclick_counter_identity.py b/tradein-mvp/backend/tests/test_3332_domclick_counter_identity.py index feb35609..e4e14923 100644 --- a/tradein-mvp/backend/tests/test_3332_domclick_counter_identity.py +++ b/tradein-mvp/backend/tests/test_3332_domclick_counter_identity.py @@ -31,6 +31,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost: _wp_mock = MagicMock() sys.modules.setdefault("weasyprint", _wp_mock) +import httpx # noqa: E402 import pytest # noqa: E402 from scraper_kit.domclick_exceptions import DomClickBlockedError # noqa: E402 from scraper_kit.proxy_errors import NoProxyAvailableError # noqa: E402 @@ -92,6 +93,14 @@ def _empty_pool_block() -> DomClickBlockedError: return blocked +def _transport_block() -> DomClickBlockedError: + """Сбой НАШЕЙ стороны (таймаут/5xx сайдкара): httpx-ошибка в __cause__ — + ровно то, что читает `_is_transport_failure` (#3283).""" + blocked = DomClickBlockedError("browser fetch failed") + blocked.__cause__ = httpx.ConnectTimeout("sidecar timed out") + return blocked + + async def _run( fetch: AsyncMock, *, snapshot: int, save_ok: bool = True ) -> tuple[DomClickDetailBackfillResult, MagicMock]: @@ -160,6 +169,25 @@ async def test_missing_row_on_save_keeps_identity() -> None: ) +@pytest.mark.asyncio +async def test_transport_failure_then_empty_pool_counts_each_once() -> None: + """Контроль двойного начисления на самом пути пула (#3338, просьба ревью). + + Транспортный сбой уже начисляет failed и идёт `continue`; следующая попытка + упирается в пустой пул и начисляет failed повторно — но СВОЙ, за СВОЮ + попытку. attempted=2 и failed=2, а не 3: правка #3332 не должна начислять + исход второй раз за ту же попытку. + """ + fetch = AsyncMock(side_effect=[_transport_block(), _empty_pool_block()]) + counters, _ = await _run(fetch, snapshot=5) + + _assert_identity(counters, expected_attempted=2) + assert (counters.failed, counters.blocked, counters.enriched) == (2, 0, 0), ( + f"failed={counters.failed} blocked={counters.blocked} enriched={counters.enriched}, " + "ожидали 2/0/0 — по одному отказу нашей стороны на каждую из двух попыток" + ) + + @pytest.mark.asyncio async def test_blocked_and_enriched_counted_once() -> None: """Контроль на противоположную ошибку: блоки/успехи по-прежнему по одному разу.""" diff --git a/tradein-mvp/backend/tests/test_3338_backfill_counter_identity.py b/tradein-mvp/backend/tests/test_3338_backfill_counter_identity.py new file mode 100644 index 00000000..555dadf3 --- /dev/null +++ b/tradein-mvp/backend/tests/test_3338_backfill_counter_identity.py @@ -0,0 +1,280 @@ +"""Тождество счётчиков у соседей domclick: yandex/avito detail-backfill (#3338). + +Тот же дефект, что #3332 (см. tests/test_3332_domclick_counter_identity.py): +`counters.attempted` инкрементируется ДО попытки, исход дописывается при разборе +результата — а ветка `save_detail_enrichment(...) -> False` исхода не дописывала +вовсе (`if save(...): enriched += 1` без else). Страницу взяли, разобрали, а +UPDATE не задел ни одной строки (объявление удалено/деактивировано между +снимком и записью) — попытка была, исхода не было. Расхождение +`attempted - сумма исходов` читается как потерянный отказ площадки. + +Разбор ВСЕХ точек выхода из цикла попыток показал, что это единственная дыра у +обоих (у domclick второй был обрыв «пул прокси пуст» — у соседей такого пути +нет: `resolve_proxy_url` бросает ProxyPoolExhaustedError ДО цикла): + + yandex — fetch-исключение → failed; non-200 → blocked+failed; parse→None → + failed; save→False → БЫЛА ДЫРА; общий except → failed. + avito — save→False → БЫЛА ДЫРА; AvitoListingGoneError → gone; + Blocked/RateLimited → blocked; TimeoutError → failed; + общий except → failed. Обрывы по budget/SIGTERM стоят ДО + `attempted += 1`, они попытку не создают. + +Формы тождества у файлов РАЗНЫЕ, и это не описка: + * yandex: `blocked` документирован как ПОДМНОЖЕСТВО `failed` (dataclass, + #3196) — non-200 инкрементирует оба, поэтому сумма исходов = enriched + failed; + * avito: `blocked`/`gone`/`failed` — непересекающиеся корзины, сумма исходов = + enriched + blocked + gone + failed. + +Проверка ПО ЗНАЧЕНИЮ: сравниваются числа, а не «не бросило исключение». Вторым +кейсом на каждый файл идёт контроль на противоположную ошибку — что правка не +начисляет исход дважды. Харнессы зеркалят tests/test_3196_yandex_ban_kind.py и +tests/test_3283g_rotate_on_platform_ban.py. +""" + +from __future__ import annotations + +import os +import sys +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) + +import pytest # noqa: E402 +from scraper_kit.avito_exceptions import AvitoBlockedError # noqa: E402 + +from app.core import shutdown as _sd # noqa: E402 +from app.tasks.avito_detail_backfill import ( # noqa: E402 + AvitoDetailBackfillResult, + run_avito_detail_backfill, +) +from app.tasks.yandex_detail_backfill import ( # noqa: E402 + YandexDetailBackfillResult, + run_yandex_detail_backfill, +) + +# ── yandex ──────────────────────────────────────────────────────────────────── +_Y_ASYNC_SESSION = "app.tasks.yandex_detail_backfill.AsyncSession" +_Y_PARSE = "app.tasks.yandex_detail_backfill.YandexDetailScraper.parse" +_Y_SAVE = "app.tasks.yandex_detail_backfill.save_detail_enrichment" +_Y_RUNS = "app.tasks.yandex_detail_backfill.runs_mod" +_Y_SLEEP = "app.tasks.yandex_detail_backfill.asyncio.sleep" +_Y_RESOLVE_PROXY_URL = "app.tasks.yandex_detail_backfill.resolve_proxy_url" + +# ── avito ───────────────────────────────────────────────────────────────────── +_A_FETCH = "app.tasks.avito_detail_backfill.fetch_detail" +_A_SAVE = "app.tasks.avito_detail_backfill.save_detail_enrichment" +_A_RUNS = "app.tasks.avito_detail_backfill.runs_mod" +_A_SLEEP = "app.tasks.avito_detail_backfill.asyncio.sleep" +_A_SETTINGS = "app.tasks.avito_detail_backfill.settings" +_A_SESSION = "app.tasks.avito_detail_backfill.AsyncSession" +_A_SCRAPER = "app.tasks.avito_detail_backfill.AvitoScraper" +_A_BROWSER_FETCHER = "app.tasks.avito_detail_backfill.BrowserFetcher" +_A_ROTATE_PROXY = "app.tasks.avito_detail_backfill.rotate_proxy" + + +@pytest.fixture(autouse=True) +def _reset_shutdown() -> None: + _sd.reset_shutdown() + yield + _sd.reset_shutdown() + + +def _assert_identity( + attempted: int, outcomes: int, *, expected_attempted: int, detail: str +) -> None: + assert attempted == expected_attempted, ( + f"attempted={attempted}, ожидали {expected_attempted} попыток" + ) + assert attempted == outcomes, ( + f"тождество нарушено: attempted={attempted}, сумма исходов={outcomes} ({detail}), " + f"потеряно {attempted - outcomes} попыток без исхода" + ) + + +def _assert_yandex_identity(c: YandexDetailBackfillResult, *, expected_attempted: int) -> None: + # blocked ⊆ failed (см. докстринг модуля) — в сумму входит только failed. + _assert_identity( + c.attempted, + c.enriched + c.failed, + expected_attempted=expected_attempted, + detail=f"enriched={c.enriched} failed={c.failed} (blocked={c.blocked} ⊆ failed)", + ) + + +def _assert_avito_identity(c: AvitoDetailBackfillResult, *, expected_attempted: int) -> None: + _assert_identity( + c.attempted, + c.enriched + c.blocked + c.gone + c.failed, + expected_attempted=expected_attempted, + detail=(f"enriched={c.enriched} blocked={c.blocked} gone={c.gone} failed={c.failed}"), + ) + + +def _mock_yandex_db(n: int) -> MagicMock: + snapshot = [ + {"id": i + 1, "source_url": f"https://realty.yandex.ru/offer/{i + 1}/"} for i in range(n) + ] + db = MagicMock() + sel = MagicMock() + sel.mappings.return_value.all.return_value = snapshot + sel.one.return_value = SimpleNamespace(url_from_offer_id=0, unenrichable_pending=0) + db.execute.return_value = sel + return db + + +def _resp(status: int) -> MagicMock: + resp = MagicMock() + resp.status_code = status + resp.text = "ok" + return resp + + +def _yandex_session_cls(responses: list[MagicMock]) -> MagicMock: + session = AsyncMock() + session.get = AsyncMock(side_effect=responses) + ctx = MagicMock() + ctx.__aenter__ = AsyncMock(return_value=session) + ctx.__aexit__ = AsyncMock(return_value=None) + return MagicMock(return_value=ctx) + + +async def _run_yandex( + responses: list[MagicMock], *, save_ok: bool, parse_result: Any = None +) -> YandexDetailBackfillResult: + count = len(responses) + db = _mock_yandex_db(count) + with ( + patch(_Y_ASYNC_SESSION, _yandex_session_cls(responses)), + patch(_Y_PARSE, return_value=parse_result or MagicMock()), + patch(_Y_SAVE, return_value=save_ok), + patch(_Y_RUNS, MagicMock()), + patch(_Y_SLEEP, new_callable=AsyncMock), + patch(_Y_RESOLVE_PROXY_URL, MagicMock(return_value="http://proxy:3128")), + ): + return await run_yandex_detail_backfill( + db, + run_id=3338, + params={"batch_size": count, "budget_sec": 3600, "max_consecutive_blocks": 10}, + ) + + +@pytest.mark.asyncio +async def test_yandex_missing_row_on_save_keeps_identity() -> None: + """save_detail_enrichment вернул False (строки уже нет) → попытка не теряется.""" + counters = await _run_yandex([_resp(200), _resp(200)], save_ok=False) + + _assert_yandex_identity(counters, expected_attempted=2) + assert (counters.enriched, counters.failed, counters.blocked) == (0, 2, 0), ( + f"enriched={counters.enriched} failed={counters.failed} blocked={counters.blocked}: " + "непрошедший UPDATE — не успех и не блок площадки" + ) + + +@pytest.mark.asyncio +async def test_yandex_outcomes_counted_once() -> None: + """Контроль на противоположную ошибку: успех и non-200 — по одному разу. + + non-200 инкрементирует и blocked, и failed НАРОЧНО (blocked ⊆ failed, #3196); + правка #3338 не должна добавлять там третий инкремент. + """ + counters = await _run_yandex([_resp(200), _resp(403)], save_ok=True) + + _assert_yandex_identity(counters, expected_attempted=2) + assert (counters.enriched, counters.failed, counters.blocked) == (1, 1, 1), ( + f"enriched={counters.enriched} failed={counters.failed} blocked={counters.blocked}, " + "ожидали 1/1/1 — исход начислен дважды" + ) + + +def _fake_avito_settings() -> MagicMock: + return MagicMock( + scraper_fetch_mode="browser", + avito_detail_backfill_use_curl=False, + detail_backfill_block_ratio_window=20, + detail_backfill_block_ratio_threshold=0.7, + browser_http_endpoint="http://browser:9000", + avito_detail_backfill_rotate_after_attempts=15, + avito_detail_backfill_rotate_on_ban_max=0, + avito_detail_backfill_rotate_on_ban_min_gap=10, + ) + + +def _mock_avito_db(n: int) -> MagicMock: + snapshot = [ + { + "id": i + 1, + "source_url": f"https://www.avito.ru/ekaterinburg/kvartiry/1-k._kvartira_{i + 1}", + } + for i in range(n) + ] + db = MagicMock() + sel = MagicMock() + sel.mappings.return_value.all.return_value = snapshot + db.execute.return_value = sel + return db + + +def _mock_avito_browser_fetcher_cls() -> MagicMock: + instance = AsyncMock() + instance.__aenter__ = AsyncMock(return_value=instance) + instance.__aexit__ = AsyncMock(return_value=False) + instance.request_context_reset = MagicMock() + instance.lease_id = 42 + return MagicMock(return_value=instance) + + +async def _run_avito(fetch_results: list[Any], *, save_ok: bool) -> AvitoDetailBackfillResult: + count = len(fetch_results) + db = _mock_avito_db(count) + with ( + patch(_A_SETTINGS, _fake_avito_settings()), + patch(_A_SESSION), + patch(_A_SCRAPER), + patch(_A_RUNS, MagicMock()), + patch(_A_BROWSER_FETCHER, _mock_avito_browser_fetcher_cls()), + patch(_A_FETCH, AsyncMock(side_effect=fetch_results)), + patch(_A_SAVE, return_value=save_ok), + patch(_A_ROTATE_PROXY, AsyncMock()), + patch(_A_SLEEP, new_callable=AsyncMock), + ): + return await run_avito_detail_backfill( + db, + run_id=3338, + params={"batch_size": count, "budget_sec": 3600, "max_consecutive_blocks": 10}, + ) + + +@pytest.mark.asyncio +async def test_avito_missing_row_on_save_keeps_identity() -> None: + """save_detail_enrichment вернул False (строки уже нет) → попытка не теряется.""" + counters = await _run_avito([MagicMock(), MagicMock()], save_ok=False) + + _assert_avito_identity(counters, expected_attempted=2) + assert (counters.enriched, counters.failed) == (0, 2), ( + f"enriched={counters.enriched} failed={counters.failed}: непрошедший UPDATE — не успех" + ) + assert (counters.blocked, counters.gone) == (0, 0), ( + f"blocked={counters.blocked} gone={counters.gone}: площадка ответила и ничего " + "не снимала — исход отказа НАШЕЙ стороны, чужие корзины трогать нельзя" + ) + + +@pytest.mark.asyncio +async def test_avito_outcomes_counted_once() -> None: + """Контроль на противоположную ошибку: успех/блок/таймаут — по одному разу.""" + counters = await _run_avito( + [MagicMock(), AvitoBlockedError("firewall/soft-block"), TimeoutError("fetch stalled")], + save_ok=True, + ) + + _assert_avito_identity(counters, expected_attempted=3) + assert (counters.enriched, counters.blocked, counters.failed, counters.gone) == (1, 1, 1, 0), ( + f"enriched={counters.enriched} blocked={counters.blocked} " + f"failed={counters.failed} gone={counters.gone}, " + "ожидали 1/1/1/0 — правка #3338 не должна начислять исход дважды" + )