From 8761602e9b36f8bee9a819f5ad85e32fd298e70c Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 6 Sep 2026 17:22:46 +0500 Subject: [PATCH 1/2] =?UTF-8?q?chore(tradein/proxy):=20=D0=BF=D0=BE=D1=81?= =?UTF-8?q?=D0=BB=D0=B5=D0=B4=D0=BD=D0=B8=D0=B5=20=D0=B4=D0=B2=D0=B5=20?= =?UTF-8?q?=D1=80=D1=83=D1=87=D0=BA=D0=B8=20admin.py=20=E2=80=94=20=D1=87?= =?UTF-8?q?=D0=B5=D1=80=D0=B5=D0=B7=20=D1=84=D0=B0=D0=B1=D1=80=D0=B8=D0=BA?= =?UTF-8?q?=D1=83=20=D1=84=D0=B5=D1=82=D1=87=D0=B5=D1=80=D0=B0;=20=D1=81?= =?UTF-8?q?=D0=BD=D1=8F=D1=82=20=D1=84=D0=BE=D1=80=D1=81=20pool-=D1=80?= =?UTF-8?q?=D0=B5=D0=B6=D0=B8=D0=BC=D0=B0=20=D1=83=20cian-history=20(#3197?= =?UTF-8?q?,=20#3386)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Два хвоста одной темы — проводка пула прокси в контейнере backend. #3197: `cian-login` и `domclick-detail-debug` были последними прямыми конструкциями `BrowserFetcher(source=, endpoint=)` мимо `build_browser_fetcher`. Без `proxy_provider`/`use_pool`/`environment` сайдкар брал свой env-узел `SCRAPER_PROXY_URL` (на проде выключенный узел 9: 407 → camoufox `InvalidIP`), а прод-отказ «пул пуст» (#2616) на этих путях был мёртв — он смотрит на `environment`, который до конструктора не доезжал. Соседи по эпику уже переведены (#3382 cian, #3389 yandex). Прямых конструкций без провайдера вне тестов больше не осталось: остальные (backfill-задачи, pipeline) пул получают своими kwargs, а `endpoint=None`-ветки providers — это документированный `config=None` для офлайн-тестов. #3386: `_PoolCurlConfig` в `cian_price_history` форсил `use_proxy_pool_curl=True`, потому что у контейнера `backend` не было переменной. #3387 задал `USE_PROXY_POOL_CURL: "true"` сервису `backend` в compose — зашитая константа стала лишней и делала рубильник неотключаемым ровно на этом пути (докстринг при этом описывал уже неверную причину). Теперь `RealScraperConfig()` напрямую. Тесты меряют значения, а не наличие kwarg'а: на откате исходников красные 4 параметризации нового `test_3197_admin_debug_browser_pool_wiring` (`assert None is not None` — провайдер не передан) и `test_price_history_honours_flag_off` (`assert ['cian'] == []` — пул дёргался при выключенном флаге). --- tradein-mvp/backend/app/api/v1/admin.py | 17 +- .../app/services/cian_price_history.py | 27 +--- .../tests/test_2830_pool_bypass_tails.py | 43 +++-- ...st_3197_admin_debug_browser_pool_wiring.py | 150 ++++++++++++++++++ .../test_admin_cian_session_endpoints.py | 10 +- .../src/scraper_kit/providers/_base.py | 10 +- 6 files changed, 212 insertions(+), 45 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py diff --git a/tradein-mvp/backend/app/api/v1/admin.py b/tradein-mvp/backend/app/api/v1/admin.py index dfc68eac..c1bae4cd 100644 --- a/tradein-mvp/backend/app/api/v1/admin.py +++ b/tradein-mvp/backend/app/api/v1/admin.py @@ -40,7 +40,6 @@ from pydantic import BaseModel, Field, field_validator # убрал legacy app.services.scheduler.scheduler_loop fallback) — тот же orchestrator, # что и debug-роуты этого файла. from scraper_kit.base import save_listings -from scraper_kit.browser_fetcher import BrowserFetcher from scraper_kit.orchestration.pipeline import ( DEFAULT_REGION_CODE, run_avito_city_sweep, @@ -49,6 +48,7 @@ from scraper_kit.orchestration.pipeline import ( run_yandex_city_sweep, run_yandex_full_load, ) +from scraper_kit.providers._base import build_browser_fetcher from scraper_kit.providers.avito.detail import fetch_detail, save_detail_enrichment from scraper_kit.providers.avito.houses import fetch_house_catalog, save_house_catalog_enrichment from scraper_kit.providers.avito.imv import ( @@ -508,8 +508,12 @@ async def cian_auto_login( ) try: - async with BrowserFetcher( - source="cian", endpoint=settings.browser_http_endpoint + # #3197 (хвост): через фабрику, а не BrowserFetcher(source=, endpoint=) напрямую — + # прямая конструкция не проставляла proxy_provider/use_pool/environment, и сайдкар + # логинился со своего env-узла SCRAPER_PROXY_URL мимо пула (на проде это выключенный + # узел 9: 407 → camoufox InvalidIP), а прод-отказ «пул пуст» (#2616) тут был мёртв. + async with build_browser_fetcher( + RealScraperConfig(), "cian", proxy_provider=_kit_proxy_provider() ) as fetcher: raw_cookies = await fetcher.login( url=settings.cian_login_url, @@ -699,7 +703,12 @@ async def debug_domclick_detail_fetch( # построен и verified live именно 2026-07-04 (815КБ __SSR_STATE__ через свежий IP). # Заменяет прежний source="cian" (docstring domclick/detail.py — устаревшая # рекомендация от 2026-06-27, до появления выделенного пула). - async with BrowserFetcher(source="domclick", endpoint=settings.browser_http_endpoint) as bf: + # #3197 (хвост): фабрика вместо прямой конструкции — она и есть то место, где + # proxy_provider/use_pool/environment попадают в тело POST /fetch. Без них узел + # выбирал сайдкар из своего env, а не пул с provider_affinity='domclick'. + async with build_browser_fetcher( + RealScraperConfig(), "domclick", proxy_provider=_kit_proxy_provider() + ) as bf: try: enrichment = await domclick_fetch_detail( body.card_url, browser_fetcher=bf, cookies=cookies diff --git a/tradein-mvp/backend/app/services/cian_price_history.py b/tradein-mvp/backend/app/services/cian_price_history.py index 0e23310e..21146957 100644 --- a/tradein-mvp/backend/app/services/cian_price_history.py +++ b/tradein-mvp/backend/app/services/cian_price_history.py @@ -34,25 +34,6 @@ from app.services.scraper_settings import get_scraper_delay logger = logging.getLogger(__name__) -class _PoolCurlConfig(RealScraperConfig): - """RealScraperConfig с принудительно включённым pool-режимом curl (#2830). - - `USE_PROXY_POOL_CURL` задан только контейнеру `scraper` (docker-compose.prod.yml - services.scraper.environment), а этот бэкфилл запускается ручкой - `POST /admin/scrape/cian-price-history` в контейнере `backend`, где переменной нет - → `settings.use_proxy_pool_curl` = False. С ней `providers/_proxy.py::curl_proxy_url` - ИГНОРИРУЕТ переданный `proxy_provider` и уходит на статичный `SCRAPER_PROXY_URL`: - один `proxy_provider=` был бы правкой без эффекта (зелёный тест, нулевой прод). - - Флаг — рубильник раскатки pool-режима для планировщика, а не решение «этому пути - пул не нужен»: инцидент 2026-08-10 (#2830) — ровно про то, что нужен именно ему. - """ - - @property - def use_proxy_pool_curl(self) -> bool: - return True - - @dataclass class CianPriceHistoryResult: checked: int = 0 @@ -84,7 +65,13 @@ async def backfill_cian_price_history( # Egress через пул с учётом `scrape_proxy_source_bans` (#2830): узел выбирает # `curl_proxy_url` внутри `fetch_detail`, он же на выходе возвращает вердикт # (mark_banned на CianBlockedError / mark_health / release). - scraper_config = _PoolCurlConfig() + # + # Флаг читается из окружения как у всех (#3386 хвост): до #3387 у контейнера + # `backend` не было `USE_PROXY_POOL_CURL`, и здесь стоял подкласс с зашитым + # `use_proxy_pool_curl = True` — иначе `curl_proxy_url` игнорировал бы + # `proxy_provider`. Теперь переменная задана и сервису `backend` (compose), а + # зашитая константа делала рубильник неотключаемым ровно на этом пути. + scraper_config = RealScraperConfig() proxy_provider = RealProxyProvider() if listing_id is not None: diff --git a/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py b/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py index 8384a28a..3c559fc8 100644 --- a/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py +++ b/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py @@ -14,9 +14,12 @@ kwarg'а в вызове. Красные на старом коде: * (1) `fetch_detail` вызывался без `proxy_provider` → lease не брался, 403 никому не - сообщался: `mark_banned_calls == []`. Плюс ловушка «правка без эффекта»: - `USE_PROXY_POOL_CURL` задан только контейнеру `scraper`, а ручка живёт в `backend`, - где флага нет — один `proxy_provider=` пул бы не включил (см. `_PoolCurlConfig`). + сообщался: `mark_banned_calls == []`. Ловушка «правка без эффекта» была в том, что + `USE_PROXY_POOL_CURL` задавался только контейнеру `scraper`, а ручка живёт в + `backend`, где флага не было — один `proxy_provider=` пул бы не включил. Отсюда взялся + подкласс конфига с зашитым `use_proxy_pool_curl = True`; #3387 задал переменную и + сервису `backend`, костыль снят (#3386 хвост), флаг снова управляет путём в обе + стороны — см. `test_price_history_honours_flag_off`. * (2) `_provider_proxy_url(source)` возвращал `settings.scraper_proxy_url` и не имел параметра `db` — вызов из теста падал бы на сигнатуре, а исход «пул исчерпан» выражения не имел вообще. @@ -123,7 +126,10 @@ def _price_history_db(n_listings: int) -> MagicMock: return db -async def _run_price_history(pool: _SpyPool, *, status_code: int, n: int = 1) -> Any: +async def _run_price_history( + pool: _SpyPool, *, status_code: int, n: int = 1, use_pool: bool = True +) -> Any: + from app.core.config import settings from app.services import scraper_adapters from app.services.cian_price_history import backfill_cian_price_history @@ -132,6 +138,7 @@ async def _run_price_history(pool: _SpyPool, *, status_code: int, n: int = 1) -> cian_detail, "build_curl_cffi_session", return_value=_session_returning(status_code) ), patch("app.services.cian_price_history.get_scraper_delay", return_value=0.0), + patch.object(settings, "use_proxy_pool_curl", use_pool), patch.object(scraper_adapters, "_proxy_pool", pool), patch.object(scraper_adapters, "_SessionLocal", MagicMock()), ): @@ -139,23 +146,33 @@ async def _run_price_history(pool: _SpyPool, *, status_code: int, n: int = 1) -> @pytest.mark.asyncio -async def test_price_history_takes_pool_node_despite_flag_off() -> None: - """Узел берётся из пула даже при выключенном USE_PROXY_POOL_CURL (контейнер backend). +async def test_price_history_takes_pool_node() -> None: + """Узел берётся из пула, вердикт возвращается, lease не течёт. - Красный на старом коде дважды: не было ни `proxy_provider=`, ни принудительного - pool-режима — `curl_proxy_url` уходил на статичный env-узел и `acquire` не звал. + Красный на коде до #2830: не было `proxy_provider=` — `curl_proxy_url` уходил на + статичный env-узел и `acquire` не звал. """ - from app.core.config import settings - - assert settings.use_proxy_pool_curl is False, ( - "тест обязан идти тем же путём, что прод-контейнер backend: без USE_PROXY_POOL_CURL" - ) pool = _SpyPool() await _run_price_history(pool, status_code=200) assert pool.acquire_calls == ["cian"] assert pool.release_calls == [9] # lease не течёт +@pytest.mark.asyncio +async def test_price_history_honours_flag_off() -> None: + """USE_PROXY_POOL_CURL=false → честный env-путь, а не пул через зашитую константу. + + #3386 (хвост): пока в сервисе жил подкласс `RealScraperConfig` с + `use_proxy_pool_curl = True`, рубильник на этом пути был неотключаем — тест красный + на main (`acquire_calls == ["cian"]`). После #3387 переменная задана контейнеру + `backend` в compose, костыль лишний, и флаг снова управляет обеими сторонами. + """ + pool = _SpyPool() + result = await _run_price_history(pool, status_code=200, use_pool=False) + assert pool.acquire_calls == [], "при выключенном флаге пул не трогаем" + assert result.checked == 1, "запрос всё равно идёт — просто env-прокси, как раньше" + + @pytest.mark.asyncio async def test_price_history_403_bans_the_node_for_cian() -> None: """403 от Циана снимает узел с выдачи ИМЕННО Циану. Красный: было `mark_banned` = [].""" diff --git a/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py b/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py new file mode 100644 index 00000000..09896500 --- /dev/null +++ b/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py @@ -0,0 +1,150 @@ +"""#3197 (хвост) — две служебные ручки admin.py строили BrowserFetcher мимо пула. + +`POST /admin/scrape/cian/auto-login` и `POST /admin/scrape/domclick/debug/detail-fetch` +конструировали `BrowserFetcher(source=..., endpoint=settings.browser_http_endpoint)` +напрямую — без `proxy_provider`/`use_pool`/`environment`, единственных трёх аргументов, +которые кладут "proxy" в тело POST /fetch сайдкара. Без них сайдкар берёт свой env-узел +(`SCRAPER_PROXY_URL`), на проде это выключенный узел 9 (407 → camoufox `InvalidIP`), а +прод-отказ «пул пуст → не ходить на env/direct» (#2616) на этих путях был мёртв: он +смотрит на `environment`, который до конструктора не доезжал. Соседние точки того же +эпика уже переведены: yandex newbuilding (#3389), cian history (#3197), domclick/avito. + +Тест меряет ЗНАЧЕНИЯ kwargs, а не то, из какого модуля взят класс: подделка ставится и на +`scraper_kit.providers._base.BrowserFetcher` (путь через фабрику), и на +`app.api.v1.admin.BrowserFetcher` (прямая конструкция, как было до правки, `create=True` — +после правки такого имени в модуле нет). Поэтому на откате красное читается как «в +конструктор не передан провайдер», а не как «мок не сработал». + +Сеть/БД/камуфокс замоканы; в сеть тест не ходит. +""" + +from __future__ import annotations + +import os +from contextlib import ExitStack +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 app.api.v1 import admin +from app.services import cian_session as cian_session_svc + +_TARGETS = ("scraper_kit.providers._base.BrowserFetcher", "app.api.v1.admin.BrowserFetcher") + + +class _CapturingFetcher: + """Собирает kwargs каждой конструкции; login() отдаёт валидный набор куки.""" + + captured: ClassVar[list[dict[str, Any]]] = [] + + def __init__(self, **kwargs: Any) -> None: + _CapturingFetcher.captured.append(kwargs) + + async def __aenter__(self) -> _CapturingFetcher: + return self + + async def __aexit__(self, *_: object) -> None: + return None + + async def login(self, **_kwargs: Any) -> dict[str, str]: + return {name: "v" for name in cian_session_svc.CIAN_REQUIRED_COOKIES} + + +@pytest.fixture +def _pool_on(monkeypatch: pytest.MonkeyPatch) -> None: + """Curl-флаг включён всегда: это он открывает `_kit_proxy_provider()` (гейт #2163). + + Провайдер обязан доезжать до конструктора при ЛЮБОМ значении browser-флага — + `use_pool=False` фетчер его просто игнорирует, но call-site у dev и прода один. + """ + from app.core.config import settings + + monkeypatch.setattr(settings, "use_proxy_pool_curl", True) + _CapturingFetcher.captured = [] + + +def _assert_wiring(source: str, *, use_pool: bool, environment: str) -> None: + assert len(_CapturingFetcher.captured) == 1, "ручка обязана построить ровно один фетчер" + kwargs = _CapturingFetcher.captured[0] + assert kwargs["source"] == source + # .get(), а не [] — красное должно читаться как «значение не то», а не KeyError. + assert kwargs.get("proxy_provider") is not None, "без провайдера пул не подключится" + assert kwargs.get("use_pool") is use_pool, "флаг пула должен доезжать из конфига" + # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв. + assert kwargs.get("environment") == environment + # endpoint не должен потеряться при переезде на фабрику (#2322: без него TypeError). + from app.core.config import settings + + assert kwargs.get("endpoint") == settings.browser_http_endpoint + + +@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) +@pytest.mark.usefixtures("_pool_on") +async def test_cian_auto_login_wires_proxy_pool( + monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str +) -> None: + """Логин-браузер ходит через узел пула, а не через env-прокси сайдкара.""" + from app.core.config import settings + + monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool) + monkeypatch.setattr(settings, "environment", environment) + monkeypatch.setattr(settings, "cookie_encryption_key", "k" * 32) + monkeypatch.setattr(settings, "cian_login_email", "a@b.c") + monkeypatch.setattr(settings, "cian_login_password", "pw") + + with ExitStack() as stack: + for target in _TARGETS: + stack.enter_context(patch(target, _CapturingFetcher, create=True)) + stack.enter_context( + patch.object( + cian_session_svc, "verify_session", AsyncMock(return_value={"user": {"userId": 7}}) + ) + ) + stack.enter_context(patch.object(cian_session_svc, "save_session", MagicMock())) + result = await admin.cian_auto_login(db=MagicMock(), body=None) + + assert result["ok"] is True and result["userId"] == 7 + _assert_wiring("cian", use_pool=use_pool, environment=environment) + + +@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) +@pytest.mark.usefixtures("_pool_on") +async def test_domclick_debug_detail_wires_proxy_pool( + monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str +) -> None: + """Debug-карточка DomClick берёт узел с provider_affinity='domclick', а не env.""" + from scraper_kit.providers.domclick import detail as domclick_detail + + from app.core.config import settings + + monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool) + monkeypatch.setattr(settings, "environment", environment) + + enrichment = SimpleNamespace( + item_id="1", + repair_state=None, + living_area_m2=None, + year_built=None, + price_changes=[], + raw_extra={}, + ) + body = admin.DomClickDebugDetailFetchRequest( + card_url="https://ekaterinburg.domclick.ru/card/sale__flat__1" + ) + with ExitStack() as stack: + for target in _TARGETS: + stack.enter_context(patch(target, _CapturingFetcher, create=True)) + stack.enter_context( + patch.object(domclick_detail, "fetch_detail", AsyncMock(return_value=enrichment)) + ) + stack.enter_context( + patch("app.services.domclick_session.load_session", MagicMock(return_value=None)) + ) + result = await admin.debug_domclick_detail_fetch(body=body, db=MagicMock()) + + assert result.ok is True + _assert_wiring("domclick", use_pool=use_pool, environment=environment) diff --git a/tradein-mvp/backend/tests/test_admin_cian_session_endpoints.py b/tradein-mvp/backend/tests/test_admin_cian_session_endpoints.py index 884a7c8a..e354d718 100644 --- a/tradein-mvp/backend/tests/test_admin_cian_session_endpoints.py +++ b/tradein-mvp/backend/tests/test_admin_cian_session_endpoints.py @@ -6,7 +6,7 @@ VERIFY_BAN_SENTINEL / VERIFY_SOURCE_UNAVAILABLE_SENTINEL / VERIFY_MARKUP_CHANGED (403) и недоступность источника (5xx) выглядели как "куки протухли" — человек в момент инцидента перезаливал заведомо валидные куки вместо починки egress/прокси. -Покрытие 3 эндпоинтов (db/verify_session/BrowserFetcher мокаются, NO live network/DB), +Покрытие 3 эндпоинтов (db/verify_session/build_browser_fetcher мокаются, NO live network/DB), зеркалит паттерн test_domclick_admin_apis.py (dependency_overrides[get_db] + TestClient): - POST /api/v1/admin/scrape/cian/upload-cookies - POST /api/v1/admin/scrape/cian/auto-login @@ -178,7 +178,9 @@ def test_auto_login_ban_returns_503(client: TestClient) -> None: patch("app.api.v1.admin.settings.cian_login_email", "user@example.com"), patch("app.api.v1.admin.settings.cian_login_password", "secret"), patch( - "app.api.v1.admin.BrowserFetcher", + # #3197 (хвост): ручка строит фетчер фабрикой, а не BrowserFetcher напрямую — + # только так до сайдкара доезжают proxy_provider/use_pool/environment. + "app.api.v1.admin.build_browser_fetcher", return_value=_mock_browser_fetcher(_RAW_COOKIES), ), patch( @@ -197,7 +199,9 @@ def test_auto_login_success_saves_and_returns_200(client: TestClient) -> None: patch("app.api.v1.admin.settings.cian_login_email", "user@example.com"), patch("app.api.v1.admin.settings.cian_login_password", "secret"), patch( - "app.api.v1.admin.BrowserFetcher", + # #3197 (хвост): ручка строит фетчер фабрикой, а не BrowserFetcher напрямую — + # только так до сайдкара доезжают proxy_provider/use_pool/environment. + "app.api.v1.admin.build_browser_fetcher", return_value=_mock_browser_fetcher(_RAW_COOKIES), ), patch( diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py index ffdbce5c..444e215f 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py @@ -209,11 +209,11 @@ def build_browser_fetcher( `proxy_provider=None` (дефолт) — валидно, но у providers больше не встречается: после #3382 (cian newbuilding) и #3389 (yandex newbuilding) провайдер передают ВСЕ call-site'ы providers (avito/cian/domclick/yandex, serp и newbuilding). - Без пула остались только прямые конструкции `BrowserFetcher(...)` мимо этой - фабрики — служебные ручки `app/api/v1/admin.py` (511 cian-login, 702 - domclick-detail-debug); прямые конструкции в `orchestration/pipeline.py` и в - backfill-задачах пул получают. `use_pool` при `proxy_provider is None` эффективно - игнорируется `BrowserFetcher` (env-fallback, см. `browser_fetcher.py::_pool_proxy`). + Служебные ручки `app/api/v1/admin.py` (cian-login, domclick-detail-debug) были + последними прямыми конструкциями мимо фабрики — переведены сюда же (#3197 хвост); + прямые конструкции в `orchestration/pipeline.py` и в backfill-задачах пул получают + своими kwargs. `use_pool` при `proxy_provider is None` эффективно игнорируется + `BrowserFetcher` (env-fallback, см. `browser_fetcher.py::_pool_proxy`). `fetch_timeout_s=None` (дефолт) → используется дефолт `BrowserFetcher` (120s). Явный таймаут передаёт ровно один call-site — `yandex/serp.py` (30s); -- 2.45.3 From f4d174283ac7266ff7238a2f695f29ea76322e74 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 6 Sep 2026 17:48:29 +0500 Subject: [PATCH 2/2] =?UTF-8?q?chore(#3197):=20cian-login=20=E2=80=94=20?= =?UTF-8?q?=D0=B1=D0=B5=D0=B7=20=D1=85=D0=BE=D0=BB=D0=BE=D1=81=D1=82=D0=BE?= =?UTF-8?q?=D0=B9=20=D0=B0=D1=80=D0=B5=D0=BD=D0=B4=D1=8B=20=D0=B8=20=D0=BB?= =?UTF-8?q?=D0=BE=D0=B6=D0=BD=D0=BE=D0=B3=D0=BE=20=D0=BE=D1=82=D0=BA=D0=B0?= =?UTF-8?q?=D0=B7=D0=B0=20(/login=20=D1=81=D0=B0=D0=B9=D0=B4=D0=BA=D0=B0?= =?UTF-8?q?=D1=80=D0=B0=20override=20=D0=BD=D0=B5=20=D0=B1=D0=B5=D1=80?= =?UTF-8?q?=D1=91=D1=82);=20=D1=87=D0=B5=D1=81=D1=82=D0=BD=D1=8B=D0=B5=20?= =?UTF-8?q?=D0=B4=D0=BE=D0=BA=D1=81=D1=82=D1=80=D0=B8=D0=BD=D0=B3=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review хвоста #3197: на `cian-login` аренда из пула не доходила до сайдкара. `BrowserFetcher._post_login` не кладёт `payload["proxy"]` (это делают только `fetch`/`fetch_json`), а на приёме `login_handler` (browser/server.py:2814-2817) зовёт `_no_live_proxy(provider, None)` и `_ensure_browser(provider)` без override — `/login` proxy-override не принимает вовсе. Lease брался в `__aenter__` и освобождался в `__aexit__` без пользы и без health-вердикта по узлу. Хуже холостого хода: при пустом пуле в production `_acquire_lease` поднимает `NoProxyAvailableError` ДО POST, `cian_auto_login` ловит любое `Exception` → `502 Browser login failed`. То есть единственная ручка ВОССТАНОВЛЕНИЯ cian-сессии отказывала ровно во время инцидента с пулом. Комментарий в admin.py при этом утверждал, что фикс закрывает InvalidIP на логине — неправда. Убран `proxy_provider=` (фабрика остаётся ради endpoint/environment из одного места). `use_pool` без провайдера фетчер игнорирует сам — `_acquire_lease`: `use_pool AND provider is not None` — поэтому ни аренды, ни прод-отказа. `domclick-detail-debug` не тронут: он ходит через `fetch`, где override реально кладётся в тело и читается сайдкаром — там #3197 остаётся настоящим фиксом. Тесты для cian обратные по значению и падают на HEAD ветки: `test_cian_auto_login_does_not_lease_from_pool` (провайдер не передан, `acquire` не вызван) и `test_cian_auto_login_survives_empty_pool_in_production` (пустой пул на проде не отдаёт 502). Стаб cian — подкласс настоящего `BrowserFetcher`, чтобы второе утверждение шло через реальный `_acquire_lease`, а не через заглушку. Follow-up (отдельной задачей): сайдкар `/login` не принимает proxy override → логин cian всегда с env-узла (сейчас выключенный узел 9). --- tradein-mvp/backend/app/api/v1/admin.py | 17 +- ...st_3197_admin_debug_browser_pool_wiring.py | 186 +++++++++++++----- .../src/scraper_kit/providers/_base.py | 9 +- 3 files changed, 159 insertions(+), 53 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/admin.py b/tradein-mvp/backend/app/api/v1/admin.py index c1bae4cd..32b8c47f 100644 --- a/tradein-mvp/backend/app/api/v1/admin.py +++ b/tradein-mvp/backend/app/api/v1/admin.py @@ -508,13 +508,16 @@ async def cian_auto_login( ) try: - # #3197 (хвост): через фабрику, а не BrowserFetcher(source=, endpoint=) напрямую — - # прямая конструкция не проставляла proxy_provider/use_pool/environment, и сайдкар - # логинился со своего env-узла SCRAPER_PROXY_URL мимо пула (на проде это выключенный - # узел 9: 407 → camoufox InvalidIP), а прод-отказ «пул пуст» (#2616) тут был мёртв. - async with build_browser_fetcher( - RealScraperConfig(), "cian", proxy_provider=_kit_proxy_provider() - ) as fetcher: + # #3197 (хвост): через фабрику (endpoint/environment из одного места), но + # НАМЕРЕННО без proxy_provider. `/login` сайдкара proxy-override не принимает + # (browser/server.py:2814-2817 — `_no_live_proxy(provider, None)`; и сам + # `_post_login` не кладёт payload["proxy"], это делают только fetch/fetch_json) — + # логин идёт с env-узла сайдкара. Аренда здесь была бы холостой и при пустом пуле + # блокировала бы ручку восстановления (`_acquire_lease` → NoProxyAvailableError → + # 502 ровно во время инцидента с пулом). Пул для логина — отдельная задача сайдкара. + # `use_pool` без провайдера фетчер игнорирует (`_acquire_lease`: use_pool AND + # provider is not None), поэтому передавать его тут безвредно, но и бесполезно. + async with build_browser_fetcher(RealScraperConfig(), "cian") as fetcher: raw_cookies = await fetcher.login( url=settings.cian_login_url, email=email, diff --git a/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py b/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py index 09896500..ba6f2586 100644 --- a/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py +++ b/tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py @@ -1,19 +1,34 @@ -"""#3197 (хвост) — две служебные ручки admin.py строили BrowserFetcher мимо пула. +"""#3197 (хвост) — две служебные ручки admin.py и пул прокси: одна чинится, вторая НЕТ. -`POST /admin/scrape/cian/auto-login` и `POST /admin/scrape/domclick/debug/detail-fetch` -конструировали `BrowserFetcher(source=..., endpoint=settings.browser_http_endpoint)` -напрямую — без `proxy_provider`/`use_pool`/`environment`, единственных трёх аргументов, -которые кладут "proxy" в тело POST /fetch сайдкара. Без них сайдкар берёт свой env-узел -(`SCRAPER_PROXY_URL`), на проде это выключенный узел 9 (407 → camoufox `InvalidIP`), а -прод-отказ «пул пуст → не ходить на env/direct» (#2616) на этих путях был мёртв: он -смотрит на `environment`, который до конструктора не доезжал. Соседние точки того же -эпика уже переведены: yandex newbuilding (#3389), cian history (#3197), domclick/avito. +`POST /admin/scrape/domclick/debug/detail-fetch` — **настоящий фикс**. Он ходит через +`BrowserFetcher.fetch`, а `fetch`/`fetch_json` — единственные методы, которые кладут +`payload["proxy"]` в тело POST /fetch сайдкара, и сайдкар этот override читает +(`_resolve_proxy_override` → `_ensure_browser(provider, proxy_override=...)`). Прямая +конструкция `BrowserFetcher(source=, endpoint=)` не проставляла +`proxy_provider`/`use_pool`/`environment` — без них сайдкар брал свой env-узел +(`SCRAPER_PROXY_URL`, на проде это выключенный узел 9: 407 → camoufox `InvalidIP`), а +прод-отказ «пул пуст → не ходить на мёртвый env» (#2616) тут был мёртв: он смотрит на +`environment`, который до конструктора не доезжал. Тест меряет ЗНАЧЕНИЯ kwargs. -Тест меряет ЗНАЧЕНИЯ kwargs, а не то, из какого модуля взят класс: подделка ставится и на -`scraper_kit.providers._base.BrowserFetcher` (путь через фабрику), и на -`app.api.v1.admin.BrowserFetcher` (прямая конструкция, как было до правки, `create=True` — -после правки такого имени в модуле нет). Поэтому на откате красное читается как «в -конструктор не передан провайдер», а не как «мок не сработал». +`POST /admin/scrape/cian/auto-login` — **намеренно без пула**, и это проверяется обратными +по значению утверждениями. Логин идёт не через `fetch`, а через `login` → `_post_login`, +который `payload["proxy"]` не кладёт вовсе; на приёме `login_handler` +(`browser/server.py:2814-2817`) зовёт `_no_live_proxy(provider, None)` и +`_ensure_browser(provider)` без override — то есть **сайдкар на `/login` proxy-override не +принимает** и логинится с env-узла при любом теле запроса. Аренда на этом пути была бы +холостой (взяли в `__aenter__`, отпустили в `__aexit__`, health-вердикта по узлу нет), а +при пустом пуле в production `_acquire_lease` поднимает `NoProxyAvailableError` ДО POST — +и единственная ручка ВОССТАНОВЛЕНИЯ сессии отдавала бы `502 Browser login failed` ровно во +время инцидента с пулом. Поэтому здесь `proxy_provider` не передаётся (фабрика остаётся +ради endpoint/environment из одного места); пул для логина — отдельная задача сайдкара. + +Подделка ставится и на `scraper_kit.providers._base.BrowserFetcher` (путь через фабрику), и +на `app.api.v1.admin.BrowserFetcher` (прямая конструкция, как было до #3197, `create=True` — +сейчас такого имени в модуле нет). Дубль нужен, чтобы файл был запускаем и против прежних +состояний исходников: красное тогда читается как «значение не то», а не «мок не сработал». +Стаб cian — ПОДКЛАСС настоящего `BrowserFetcher`: `__aenter__`/`_acquire_lease` исполняются +по-настоящему (иначе тест на «нет ложного 502» проверял бы заглушку), застаблен только +сетевой `login`. Сеть/БД/камуфокс замоканы; в сеть тест не ходит. """ @@ -29,6 +44,9 @@ from unittest.mock import AsyncMock, MagicMock, patch os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") import pytest +from fastapi import HTTPException +from scraper_kit.browser_fetcher import BrowserFetcher +from scraper_kit.contracts import ProxyLease from app.api.v1 import admin from app.services import cian_session as cian_session_svc @@ -54,6 +72,45 @@ class _CapturingFetcher: return {name: "v" for name in cian_session_svc.CIAN_REQUIRED_COOKIES} +class _RealLeaseFetcher(BrowserFetcher): + """Настоящий `BrowserFetcher` (реальные `__aenter__`/`_acquire_lease`) без сети. + + Наследование, а не заглушка: утверждение «пустой пул в production не роняет ручку» + обязано пройти через тот самый guard `_acquire_lease`, который поднимает + `NoProxyAvailableError`. Подменён только сетевой `login`. + """ + + captured: ClassVar[list[dict[str, Any]]] = [] + + def __init__(self, **kwargs: Any) -> None: + _RealLeaseFetcher.captured.append(kwargs) + super().__init__(**kwargs) + + async def login(self, **_kwargs: Any) -> dict[str, str]: + return {name: "v" for name in cian_session_svc.CIAN_REQUIRED_COOKIES} + + +class _SpyProvider: + """Пустой пул + спай: `acquire` пишет вызовы в ClassVar и всегда отдаёт None. + + ClassVar, а не поле инстанса: `_kit_proxy_provider()` конструирует провайдер сам, + и «ни разу не позвали» должно покрывать в том числе «даже не создали». + """ + + acquired: ClassVar[list[str]] = [] + released: ClassVar[list[int]] = [] + + def acquire(self, provider: str) -> ProxyLease | None: + _SpyProvider.acquired.append(provider) + return None + + def release(self, lease: ProxyLease) -> None: + _SpyProvider.released.append(lease.id) + + def mark_health(self, lease: ProxyLease, *, ok: bool, error: str | None = None) -> None: + return None + + @pytest.fixture def _pool_on(monkeypatch: pytest.MonkeyPatch) -> None: """Curl-флаг включён всегда: это он открывает `_kit_proxy_provider()` (гейт #2163). @@ -64,51 +121,81 @@ def _pool_on(monkeypatch: pytest.MonkeyPatch) -> None: from app.core.config import settings monkeypatch.setattr(settings, "use_proxy_pool_curl", True) + monkeypatch.setattr(admin, "RealProxyProvider", _SpyProvider) _CapturingFetcher.captured = [] + _RealLeaseFetcher.captured = [] + _SpyProvider.acquired = [] + _SpyProvider.released = [] -def _assert_wiring(source: str, *, use_pool: bool, environment: str) -> None: - assert len(_CapturingFetcher.captured) == 1, "ручка обязана построить ровно один фетчер" - kwargs = _CapturingFetcher.captured[0] - assert kwargs["source"] == source +def _cian_login_settings(monkeypatch: pytest.MonkeyPatch, *, use_pool: bool, env: str) -> None: + from app.core.config import settings + + monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool) + monkeypatch.setattr(settings, "environment", env) + monkeypatch.setattr(settings, "cookie_encryption_key", "k" * 32) + monkeypatch.setattr(settings, "cian_login_email", "a@b.c") + monkeypatch.setattr(settings, "cian_login_password", "pw") + + +def _patched_cian_login(stack: ExitStack, fetcher_cls: type) -> None: + for target in _TARGETS: + stack.enter_context(patch(target, fetcher_cls, create=True)) + stack.enter_context( + patch.object( + cian_session_svc, "verify_session", AsyncMock(return_value={"user": {"userId": 7}}) + ) + ) + stack.enter_context(patch.object(cian_session_svc, "save_session", MagicMock())) + + +@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) +@pytest.mark.usefixtures("_pool_on") +async def test_cian_auto_login_does_not_lease_from_pool( + monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str +) -> None: + """Логин пул НЕ арендует: сайдкар на `/login` proxy-override не берёт (см. докстринг).""" + _cian_login_settings(monkeypatch, use_pool=use_pool, env=environment) + + with ExitStack() as stack: + _patched_cian_login(stack, _RealLeaseFetcher) + result = await admin.cian_auto_login(db=MagicMock(), body=None) + + assert result["ok"] is True and result["userId"] == 7 + assert len(_RealLeaseFetcher.captured) == 1, "ручка обязана построить ровно один фетчер" + kwargs = _RealLeaseFetcher.captured[0] # .get(), а не [] — красное должно читаться как «значение не то», а не KeyError. - assert kwargs.get("proxy_provider") is not None, "без провайдера пул не подключится" - assert kwargs.get("use_pool") is use_pool, "флаг пула должен доезжать из конфига" - # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв. - assert kwargs.get("environment") == environment - # endpoint не должен потеряться при переезде на фабрику (#2322: без него TypeError). + assert kwargs.get("proxy_provider") is None, "аренда на /login холостая — провайдер не нужен" + assert _SpyProvider.acquired == [], "lease взят впустую (сайдкар его всё равно не увидит)" + # endpoint из фабрики не должен потеряться (#2322: без него TypeError). from app.core.config import settings assert kwargs.get("endpoint") == settings.browser_http_endpoint -@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) @pytest.mark.usefixtures("_pool_on") -async def test_cian_auto_login_wires_proxy_pool( - monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str +async def test_cian_auto_login_survives_empty_pool_in_production( + monkeypatch: pytest.MonkeyPatch, ) -> None: - """Логин-браузер ходит через узел пула, а не через env-прокси сайдкара.""" - from app.core.config import settings + """Пустой пул на проде НЕ ломает ручку восстановления сессии (нет ложного 502). - monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool) - monkeypatch.setattr(settings, "environment", environment) - monkeypatch.setattr(settings, "cookie_encryption_key", "k" * 32) - monkeypatch.setattr(settings, "cian_login_email", "a@b.c") - monkeypatch.setattr(settings, "cian_login_password", "pw") + Ровно тот сценарий, ради которого пул отсюда убран: `_acquire_lease` при + `use_pool + provider + production + пустой пул` поднимает `NoProxyAvailableError`, + `cian_auto_login` ловит любое `Exception` и отдаёт `502 Browser login failed` — то + есть инцидент с пулом закрывал бы единственный способ переполучить cian-сессию, + хотя логину пул не нужен (сайдкар proxy-override на `/login` не принимает). + """ + _cian_login_settings(monkeypatch, use_pool=True, env="production") with ExitStack() as stack: - for target in _TARGETS: - stack.enter_context(patch(target, _CapturingFetcher, create=True)) - stack.enter_context( - patch.object( - cian_session_svc, "verify_session", AsyncMock(return_value={"user": {"userId": 7}}) - ) - ) - stack.enter_context(patch.object(cian_session_svc, "save_session", MagicMock())) - result = await admin.cian_auto_login(db=MagicMock(), body=None) + _patched_cian_login(stack, _RealLeaseFetcher) + try: + result = await admin.cian_auto_login(db=MagicMock(), body=None) + except HTTPException as exc: + # pytest.fail, а не re-raise: красное должно называть статус и detail. + pytest.fail(f"пустой пул уронил ручку восстановления: {exc.status_code} {exc.detail}") assert result["ok"] is True and result["userId"] == 7 - _assert_wiring("cian", use_pool=use_pool, environment=environment) @pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")]) @@ -147,4 +234,15 @@ async def test_domclick_debug_detail_wires_proxy_pool( result = await admin.debug_domclick_detail_fetch(body=body, db=MagicMock()) assert result.ok is True - _assert_wiring("domclick", use_pool=use_pool, environment=environment) + assert len(_CapturingFetcher.captured) == 1, "ручка обязана построить ровно один фетчер" + kwargs = _CapturingFetcher.captured[0] + assert kwargs["source"] == "domclick" + # .get(), а не [] — красное должно читаться как «значение не то», а не KeyError. + assert kwargs.get("proxy_provider") is not None, "без провайдера пул не подключится" + assert kwargs.get("use_pool") is use_pool, "флаг пула должен доезжать из конфига" + # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв. + assert kwargs.get("environment") == environment + # endpoint не должен потеряться при переезде на фабрику (#2322: без него TypeError). + from app.core.config import settings + + assert kwargs.get("endpoint") == settings.browser_http_endpoint diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py index 444e215f..f78c0bdb 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py @@ -212,8 +212,13 @@ def build_browser_fetcher( Служебные ручки `app/api/v1/admin.py` (cian-login, domclick-detail-debug) были последними прямыми конструкциями мимо фабрики — переведены сюда же (#3197 хвост); прямые конструкции в `orchestration/pipeline.py` и в backfill-задачах пул получают - своими kwargs. `use_pool` при `proxy_provider is None` эффективно игнорируется - `BrowserFetcher` (env-fallback, см. `browser_fetcher.py::_pool_proxy`). + своими kwargs. Живой `proxy_provider=None` остался ровно один — cian-login: сайдкар + на `/login` proxy-override не берёт (`browser/server.py::login_handler` → + `_no_live_proxy(provider, None)`, а `_post_login` не кладёт `payload["proxy"]`), так + что аренда там была бы холостой, а на проде при пустом пуле роняла бы ручку + восстановления в 502. `use_pool` при `proxy_provider is None` игнорируется + `BrowserFetcher` (`_acquire_lease`: `use_pool AND provider is not None`) — ни аренды, + ни прод-отказа, поведение как до фабрики. `fetch_timeout_s=None` (дефолт) → используется дефолт `BrowserFetcher` (120s). Явный таймаут передаёт ровно один call-site — `yandex/serp.py` (30s); -- 2.45.3