diff --git a/tradein-mvp/backend/app/services/scheduler.py b/tradein-mvp/backend/app/services/scheduler.py index b5cc645c..f44fc756 100644 --- a/tradein-mvp/backend/app/services/scheduler.py +++ b/tradein-mvp/backend/app/services/scheduler.py @@ -161,6 +161,25 @@ async def _execute_cian_backfill( # #3196: отказ detail-фетча теперь несёт диагноз (HTTP-статус последнего ответа # сайдкара). В 'banned' переводим ТОЛЬКО прогон, который отказы видел и не # обогатил НИЧЕГО, — частичный успех остаётся 'done', как и был. + if result.no_proxy_stop: + # #3197 (как #3288 у avito / #3283 у домклика): остановка из-за пустого пула — + # НЕ блок, поэтому и не mark_banned: иначе прогон уйдёт в 'banned' и запись + # будет утверждать про площадку то, чего не было. Это отказ нашей стороны. + counters["no_proxy_stop"] = 1 + runs_mod.mark_failed( + db, run_id, "пул прокси пуст — к площадке не ходили (#3197)", counters + ) + logger.error( + "scheduler: cian_history_backfill run_id=%d СТОП (пул пуст) — " + "listings=%d/%d houses=%d/%d %.1fs", + run_id, + result.listings_succeeded, + result.listings_total, + result.houses_succeeded, + result.houses_total, + result.duration_sec, + ) + return if result.ban_kinds and (result.listings_succeeded + result.houses_succeeded) == 0: counters["blocked"] = result.listings_blocked # Полная перепись диагнозов, а не только доминирующий вид (#3196) — иначе diff --git a/tradein-mvp/backend/app/tasks/cian_history_backfill.py b/tradein-mvp/backend/app/tasks/cian_history_backfill.py index 94762ae0..41c8f3f0 100644 --- a/tradein-mvp/backend/app/tasks/cian_history_backfill.py +++ b/tradein-mvp/backend/app/tasks/cian_history_backfill.py @@ -40,6 +40,7 @@ from dataclasses import dataclass, field from scraper_kit.browser_fetcher import BrowserFetcher, ban_kind_from_status from scraper_kit.providers.cian.detail import fetch_detail, save_detail_enrichment from scraper_kit.providers.cian.valuation import estimate_via_cian_valuation +from scraper_kit.proxy_errors import caused_by_no_proxy from sqlalchemy import text from sqlalchemy.orm import Session @@ -77,6 +78,10 @@ class CianBackfillResult: # у него не проставлялся вовсе. listings_blocked: int = 0 ban_kinds: Counter[str] = field(default_factory=Counter) + # #3197: прогон оборван, потому что пул прокси пуст — к площадке не ходили вовсе. + # Не блок и не отказ Циана: caller (scheduler) обязан пометить прогон failed, а не + # banned, иначе запись утверждает про площадку то, чего не было. + no_proxy_stop: bool = False @property def ban_kind(self) -> str: @@ -203,7 +208,20 @@ async def backfill_cian_history( else: # One BrowserFetcher instance shared across all listings in this batch. # priceChanges requires JS rendering — curl_cffi returns empty list (#1574). - async with BrowserFetcher(source="cian", endpoint=settings.browser_http_endpoint) as bf: + # proxy_provider/use_pool/environment (#3197, шаг A2): без этих трёх сайдкар + # берёт свой env-прокси (SCRAPER_PROXY_URL) — прогон шёл мимо пула из 4 узлов + # целиком (ни выбора узла, ни scrape_proxy_source_bans, ни ротации), а + # прод-отказ «пул пуст → не ходить на env/direct» (#2616) на этом пути был + # мёртв: он смотрит на environment, который сюда не доезжал. Образец — + # domclick_detail_backfill.py:403 и house_imv_backfill.py:749 (#2698/#3197). + _cfg = RealScraperConfig() + async with BrowserFetcher( + source="cian", + endpoint=settings.browser_http_endpoint, + proxy_provider=RealProxyProvider(), + use_pool=_cfg.use_proxy_pool_browser, + environment=_cfg.environment, + ) as bf: for row in rows: listing_id: int = row["id"] source_url: str = row["source_url"] @@ -215,6 +233,21 @@ async def backfill_cian_history( try: enrichment = await fetch_detail(source_url, browser_fetcher=bf) except Exception as exc: + # #3197: пустой пул — не отказ площадки: запрос не уходил вовсе, + # следующее объявление упрётся ровно в то же самое (иначе батч + # крутит впустую весь список). Опора — тип в цепочке причин, а не + # текст: fetch_detail заворачивает сбой фетча в своё исключение. + if caused_by_no_proxy(exc): + result.no_proxy_stop = True + result.listings_failed_fetch += 1 + logger.error( + "cian_history_backfill: СТОП — пул прокси пуст, к площадке " + "не ходили. listing_id=%s processed=%d succeeded=%d", + listing_id, + result.listings_processed, + result.listings_succeeded, + ) + break kind = _note_refusal(result, bf.last_response_status) logger.warning( "cian_detail fetch failed for listing_id=%s url=%s: %s " @@ -272,7 +305,9 @@ async def backfill_cian_history( await asyncio.sleep(delay) # ── 2. Houses: missing houses_price_dynamics ────────────────────────────── - if do_houses: + # no_proxy_stop (#3197): пул пуст — дома идут через тот же пул (fetch_newbuilding с + # RealProxyProvider ниже), крутить их незачем. + if do_houses and not result.no_proxy_stop: # Kit's fetch_newbuilding() now accepts config= (issue #2322 fixed) — pass # RealScraperConfig() at the call site below so BrowserFetcher gets a real # endpoint instead of degrading to endpoint=None (#2397 Part D2). @@ -363,7 +398,7 @@ async def backfill_cian_history( await asyncio.sleep(delay) # ── 3. Cian listings без external_valuations (price prediction backfill) ── - if do_valuations: + if do_valuations and not result.no_proxy_stop: # #3197: пул пуст — см. блок домов rows = ( db.execute( text(""" diff --git a/tradein-mvp/backend/tests/test_3197_cian_history_proxy_pool_wiring.py b/tradein-mvp/backend/tests/test_3197_cian_history_proxy_pool_wiring.py new file mode 100644 index 00000000..19a7b1dd --- /dev/null +++ b/tradein-mvp/backend/tests/test_3197_cian_history_proxy_pool_wiring.py @@ -0,0 +1,153 @@ +"""#3197 (часть 1, Циан) — суточный бэкфилл ходил в сайдкар мимо прокси-пула. + +`BrowserFetcher(source="cian", endpoint=...)` конструировался БЕЗ +`proxy_provider`/`use_pool`/`environment` — единственных трёх аргументов, которые +кладут "proxy" в тело POST /fetch (см. `scraper_kit.browser_fetcher`). Без них сайдкар +брал свой env-прокси (`SCRAPER_PROXY_URL`): ни выбора узла из пула, ни +`scrape_proxy_source_bans`, ни ротации, — а прод-отказ «пул пуст → не ходить на +env/direct» (#2616) на этом пути был мёртв, потому что смотрит на `environment`, который +до конструктора не доезжал. Соседи уже починены: domclick (#3197 ч.1, см. +test_3197_domclick_proxy_pool_wiring.py) и house_imv/avito (#2698). + +Второй тест — про то, чем оживший отказ оборачивается в прогоне: пустой пул +поднимается ДО запроса, поэтому следующее объявление упрётся ровно в то же самое, и +батч обязан оборваться на первом, а не крутить весь список. + +Сеть/БД/камуфокс замоканы; в сеть тест не ходит. +""" + +from __future__ import annotations + +import os +from types import SimpleNamespace +from typing import Any, ClassVar +from unittest.mock import AsyncMock, MagicMock, patch + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import pytest +from scraper_kit.proxy_errors import NoProxyAvailableError + +from app.tasks import cian_history_backfill as chb + +_FETCH = "app.tasks.cian_history_backfill.fetch_detail" +_SAVE = "app.tasks.cian_history_backfill.save_detail_enrichment" +_SLEEP = "app.tasks.cian_history_backfill.asyncio.sleep" +_DELAY = "app.tasks.cian_history_backfill.get_scraper_delay" + + +class _CapturingFetcher: + """Зеркалит _CapturingFetcher из test_3197_domclick_proxy_pool_wiring.py.""" + + captured: ClassVar[dict[str, Any]] = {} + + def __init__(self, **kwargs: Any) -> None: + _CapturingFetcher.captured = kwargs + self.last_response_status: int | None = None + + async def __aenter__(self) -> _CapturingFetcher: + return self + + async def __aexit__(self, *_: object) -> None: + return None + + +def _mock_db(n_rows: int) -> MagicMock: + rows = [ + {"id": i + 1, "source_url": f"https://ekb.cian.ru/sale/flat/{i + 1}/"} + for i in range(n_rows) + ] + db = MagicMock() + sel = MagicMock() + sel.mappings.return_value.all.return_value = rows + db.execute.return_value = sel + return db + + +async def _run_listings(db: MagicMock, fetch: Any) -> chb.CianBackfillResult: + with ( + patch.object(chb, "BrowserFetcher", _CapturingFetcher), + patch(_FETCH, fetch), + patch(_SAVE, return_value=True), + patch(_SLEEP, new_callable=AsyncMock), + patch(_DELAY, return_value=0.0), + ): + return await chb.backfill_cian_history( + db, batch_size=10, do_listings=True, do_houses=False, do_valuations=False + ) + + +@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) +async def test_browser_fetcher_gets_proxy_pool_wiring( + monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str +) -> None: + """use_pool/proxy_provider/environment доезжают до BrowserFetcher ИЗ КОНФИГА. + + Оба значения флага проверяются одним телом: `use_pool` обязан следовать конфигу, а не + быть зашитой константой, а `proxy_provider` передаётся в любом случае — при + `use_pool=False` он игнорируется фетчером, но call-site у dev и прода один и тот же. + """ + _CapturingFetcher.captured = {} + monkeypatch.setattr(chb.settings, "use_proxy_pool_browser", use_pool) + monkeypatch.setattr(chb.settings, "environment", environment) + + fetch = AsyncMock(return_value=SimpleNamespace(price_changes=[])) + await _run_listings(_mock_db(1), fetch) + + captured = _CapturingFetcher.captured + assert captured["source"] == "cian" + assert captured["endpoint"] == chb.settings.browser_http_endpoint + # .get(), а не [] — красное должно читаться как «значение не то», а не как KeyError. + assert captured.get("proxy_provider") is not None, "без провайдера пул не подключится" + assert captured.get("use_pool") is use_pool, "флаг пула должен доезжать до фетчера из конфига" + # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв. + assert captured.get("environment") == environment + + +async def test_empty_pool_stops_the_batch_on_first_listing() -> None: + """«Пул пуст» на первом объявлении обрывает батч, а не крутит весь список.""" + _CapturingFetcher.captured = {} + + def _raise_wrapped(*_a: object, **_kw: object) -> None: + # Ровно как в проде: провайдер заворачивает сбой фетча в своё исключение, и + # «пул пуст» приезжает наверх под видом отказа площадки. + try: + raise NoProxyAvailableError("cian") + except NoProxyAvailableError as exc: + raise RuntimeError("cian detail fetch failed") from exc + + fetch = AsyncMock(side_effect=_raise_wrapped) + result = await _run_listings(_mock_db(3), fetch) + + # Сначала измеримое поведение (сколько раз пошли), потом флаг: красное на откате + # должно означать «прошли 3 строки вместо 1», а не «поля нет». + assert fetch.await_count == 1, "к площадке ходили только один раз — пул пуст с первого" + assert result.listings_processed == 1, "батч обязан оборваться, а не пройти все 3 строки" + assert getattr(result, "no_proxy_stop", False) is True + # Отказ НАШЕЙ стороны не должен маскироваться под бан площадки (иначе прогон уйдёт + # в 'banned' и запись соврёт про Циан). + assert result.ban_kinds == {} + assert result.listings_blocked == 0 + + +async def test_run_marked_failed_with_no_proxy_stop_counter() -> None: + """Прогон с пустым пулом финализируется как failed + counters.no_proxy_stop=1.""" + from app.services import scheduler as sched + + runs = MagicMock() + result = chb.CianBackfillResult( + listings_total=3, listings_processed=1, listings_failed_fetch=1, no_proxy_stop=True + ) + with ( + patch.object(sched, "runs_mod", runs), + patch.object(chb, "backfill_cian_history", AsyncMock(return_value=result)), + ): + await sched._execute_cian_backfill(MagicMock(), run_id=3197, params={"batch_size": 3}) + + runs.mark_done.assert_not_called() + runs.mark_banned.assert_not_called() + runs.mark_failed.assert_called_once() + _db, run_id, reason, counters = runs.mark_failed.call_args.args + assert run_id == 3197 + assert "пул" in reason + assert counters["no_proxy_stop"] == 1 diff --git a/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py b/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py index 43cbe845..58c79a0c 100644 --- a/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py +++ b/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py @@ -221,7 +221,17 @@ async def test_cian_history_backfill_browser_fetcher_uses_settings_endpoint() -> db, do_listings=True, do_houses=False, do_valuations=False ) - assert captured == {"source": "cian", "endpoint": settings.browser_http_endpoint} + # #3197: к endpoint= добавилась проводка пула — без неё сайдкар брал env-прокси, и + # суточный прогон шёл мимо пула из 4 узлов (детали — test_3197_cian_history_proxy_ + # pool_wiring.py). + assert captured == { + "source": "cian", + "endpoint": settings.browser_http_endpoint, + "proxy_provider": captured.get("proxy_provider"), + "use_pool": settings.use_proxy_pool_browser, + "environment": settings.environment, + } + assert captured["proxy_provider"] is not None # ── config= threading — houses / valuations blocks (#2397 Part D2) ─────────────── diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/proxy_errors.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/proxy_errors.py index 851bda08..e8f3178f 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/proxy_errors.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/proxy_errors.py @@ -65,4 +65,26 @@ class ProxyBanError(Exception): """ -__all__ = ["NoProxyAvailableError", "ProxyBanError"] +def caused_by_no_proxy(exc: BaseException) -> bool: + """Прячется ли за этим исключением пустой пул прокси (#3197). + + Опора — ТИП в цепочке `__cause__`/`__context__`, а не подстрока «no proxy available» + в тексте: провайдеры заворачивают любой сбой фетча в свои Blocked/Unavailable- + исключения, и «пул пуст» приезжает наверх под видом блокировки площадки, хотя + запрос не уходил вовсе. По тексту такое уже один раз объявили баном чужую строку + (#3272), поэтому здесь только isinstance. + + Те же две копии живут приватно в `app/tasks/avito_detail_backfill.py` (#3288) и + `domclick_detail_backfill.py` (#3283); их схлопывание сюда — отдельная правка. + """ + seen: set[int] = set() + cur: BaseException | None = exc + while cur is not None and id(cur) not in seen: + if isinstance(cur, NoProxyAvailableError): + return True + seen.add(id(cur)) + cur = cur.__cause__ or cur.__context__ + return False + + +__all__ = ["NoProxyAvailableError", "ProxyBanError", "caused_by_no_proxy"]