diff --git a/tradein-mvp/backend/tests/test_2700_cian_detail_403_node.py b/tradein-mvp/backend/tests/test_2700_cian_detail_403_node.py index d6637486..c4da8e96 100644 --- a/tradein-mvp/backend/tests/test_2700_cian_detail_403_node.py +++ b/tradein-mvp/backend/tests/test_2700_cian_detail_403_node.py @@ -103,7 +103,7 @@ async def test_403_bans_the_node_for_cian_only() -> None: with pytest.raises(CianBlockedError): await _fetch(403, spy) assert spy.mark_banned_calls == [(1, "cian")] - assert spy.mark_health_calls == [(1, False)] + assert spy.mark_health_calls == [] # #3310: отказ площадки не копит consecutive_fails assert spy.release_calls == [1] # lease не течёт даже на бане 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 3c559fc8..27aa1d92 100644 --- a/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py +++ b/tradein-mvp/backend/tests/test_2830_pool_bypass_tails.py @@ -179,7 +179,7 @@ async def test_price_history_403_bans_the_node_for_cian() -> None: pool = _SpyPool() result = await _run_price_history(pool, status_code=403) assert pool.mark_banned_calls == [(9, "cian")] - assert pool.mark_health_calls == [(9, False)] + assert pool.mark_health_calls == [] # #3310: бан вместо mark_health(False) assert result.errors == 1 # прогон честен: отказ посчитан @@ -335,7 +335,7 @@ async def test_zhk_resolve_403_reaches_the_pool() -> None: with pytest.raises(CianBlockedError): await _resolve(403, spy) assert spy.mark_banned_calls == [(9, "cian")] - assert spy.mark_health_calls == [(9, False)] + assert spy.mark_health_calls == [] # #3310: бан вместо mark_health(False) assert spy.release_calls == [9] diff --git a/tradein-mvp/backend/tests/test_3310_curl_ban_keeps_last_node.py b/tradein-mvp/backend/tests/test_3310_curl_ban_keeps_last_node.py new file mode 100644 index 00000000..40a75a4d --- /dev/null +++ b/tradein-mvp/backend/tests/test_3310_curl_ban_keeps_last_node.py @@ -0,0 +1,126 @@ +"""Защита последнего узла держит слово и на curl-пути (#3310). + +`mark_banned` бережёт узел от бан-строки, если он последний для источника, и пишет +«узел продолжит выдаваться». Но `curl_proxy_url` на том же `ProxyBanError` вслед за +баном звал `mark_health(ok=False)`: после трёх таких ответов `consecutive_fails = 3`, и +`acquire` отсекал узел для ВСЕХ источников — пул объявлял себя пустым при живом узле. +Браузерный путь это уже не делает (#3288, `report_platform_ban`), curl-путь — делал. + +Проверка по значению на живом Postgres, настоящим трактом: `curl_proxy_url` → +`RealProxyProvider` → `proxy_pool`. Сессии адаптера привязаны к одной внешней +транзакции с откатом (commit внутри пула = RELEASE savepoint). Без БД — skip. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from collections.abc import Iterator +from dataclasses import dataclass +from typing import Any + +import pytest +from scraper_kit.cian_exceptions import CianBlockedError +from scraper_kit.orchestration.run_context import current_run_id +from scraper_kit.providers._proxy import curl_proxy_url +from sqlalchemy import create_engine, text +from sqlalchemy.orm import Session + +from app.services import proxy_pool, scraper_adapters + + +def _live_engine() -> Any | None: + dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "") + if not dsn or "localhost:5432/test" in dsn: + return None + try: + engine = create_engine(dsn, future=True) + with engine.connect() as conn: + conn.execute(text("SELECT 1 FROM scrape_proxies LIMIT 1")) + return engine + except Exception: + return None + + +_ENGINE = _live_engine() +pytestmark = pytest.mark.skipif(_ENGINE is None, reason="no reachable Postgres test DB") + + +@dataclass +class _Config: + use_proxy_pool_curl: bool = True + environment: str = "production" + + +@pytest.fixture +def node(monkeypatch: pytest.MonkeyPatch) -> Iterator[tuple[int, Session]]: + """Единственный включённый узел пула ('any'); адаптер ходит в ту же транзакцию.""" + assert _ENGINE is not None + conn = _ENGINE.connect() + outer = conn.begin() + + def _session() -> Session: + return Session(bind=conn, join_transaction_mode="create_savepoint") + + db = _session() + db.execute(text("UPDATE scrape_proxies SET enabled = false")) + proxy_id = db.execute( + text( + "INSERT INTO scrape_proxies (url, provider_affinity) " + "VALUES ('http://t3310-' || gen_random_uuid(), 'any') RETURNING id" + ) + ).scalar_one() + db.commit() + monkeypatch.setattr(scraper_adapters, "_SessionLocal", _session) + token = current_run_id.set(None) + try: + yield int(proxy_id), db + finally: + current_run_id.reset(token) + db.close() + outer.rollback() + conn.close() + + +def _fails(db: Session, proxy_id: int) -> int: + return int( + db.execute( + text("SELECT consecutive_fails FROM scrape_proxies WHERE id = :id"), {"id": proxy_id} + ).scalar_one() + ) + + +def test_three_bans_on_last_node_keep_it_in_the_pool(node: tuple[int, Session]) -> None: + """Приёмка issue: единственный узел + три бана подряд → acquire не пуст. + + На старом коде: consecutive_fails == 3 и acquire('cian') → None.""" + proxy_id, db = node + provider = scraper_adapters.RealProxyProvider() + + for _ in range(3): + with pytest.raises(CianBlockedError): + with curl_proxy_url(_Config(), provider, "cian", env_fallback_url=None): + raise CianBlockedError("HTTP 403") + + assert _fails(db, proxy_id) == 0 + ban_rows = db.execute( + text("SELECT count(*) FROM scrape_proxy_source_bans WHERE proxy_id = :id"), + {"id": proxy_id}, + ).scalar_one() + assert ban_rows == 0, "защита последнего узла не сработала — тест проверял бы не то" + lease = proxy_pool.acquire(db, "cian", run_id=None) + assert lease is not None and lease.id == proxy_id, f"пул пуст при живом узле: {lease}" + + +def test_transport_failure_still_counts_against_node_health(node: tuple[int, Session]) -> None: + """Обратная сторона: сетевой сбой — не бан, узлу по-прежнему засчитывается отказ.""" + proxy_id, db = node + provider = scraper_adapters.RealProxyProvider() + + with pytest.raises(OSError): + with curl_proxy_url(_Config(), provider, "cian", env_fallback_url=None): + raise OSError("proxy 407") + + assert _fails(db, proxy_id) == 1 diff --git a/tradein-mvp/backend/tests/test_3402_cian_captcha_http200.py b/tradein-mvp/backend/tests/test_3402_cian_captcha_http200.py index 78cd03b9..f7b77441 100644 --- a/tradein-mvp/backend/tests/test_3402_cian_captcha_http200.py +++ b/tradein-mvp/backend/tests/test_3402_cian_captcha_http200.py @@ -255,7 +255,7 @@ async def test_captcha_on_curl_path_bans_the_node_for_cian() -> None: assert spy.mark_banned_calls == [(1, "cian")] assert (1, True) not in spy.mark_health_calls, "узел с капчей записан здоровым" - assert spy.mark_health_calls == [(1, False)] + assert spy.mark_health_calls == [] # #3310: бан вместо mark_health(False) assert spy.release_calls == [1] # lease не течёт diff --git a/tradein-mvp/backend/tests/test_proxy_pool_curl_paths.py b/tradein-mvp/backend/tests/test_proxy_pool_curl_paths.py index fe94b232..5e06ee9a 100644 --- a/tradein-mvp/backend/tests/test_proxy_pool_curl_paths.py +++ b/tradein-mvp/backend/tests/test_proxy_pool_curl_paths.py @@ -127,10 +127,10 @@ def test_flag_on_exception_marks_fail_and_still_releases() -> None: assert spy.release_calls == [7] -def test_ban_exception_calls_mark_banned_in_addition_to_mark_health() -> None: +def test_ban_exception_calls_mark_banned_instead_of_mark_health() -> None: """Исключение — подкласс ProxyBanError (напр. AvitoBlockedError) внутри блока — - вызывает mark_banned(lease, source=provider) В ДОПОЛНЕНИЕ к mark_health(ok=False) - (#2600 п.1 curl-путь). Zero изменений в вызывающем коде — сигнал детектируется + вызывает mark_banned(lease, source=provider) ВМЕСТО mark_health(ok=False) + (#2600 п.1 curl-путь, #3310). Zero изменений в вызывающем коде — сигнал детектируется по ТИПУ исключения, а не явным вызовом.""" class _FakeBlockedError(ProxyBanError): @@ -143,7 +143,8 @@ def test_ban_exception_calls_mark_banned_in_addition_to_mark_health() -> None: assert url == _LEASE.url raise _FakeBlockedError("firewall page detected") assert spy.mark_banned_calls == [(7, "avito")] - assert spy.mark_health_calls == [(7, False)] # оба сигнала, не взаимоисключающие + # #3310: бан ВМЕСТО mark_health(False) — иначе три бана выводили узел из выдачи всем + assert spy.mark_health_calls == [] assert spy.release_calls == [7] # lease всё равно освобождён diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_proxy.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_proxy.py index a6f3e539..fe12fe12 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_proxy.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_proxy.py @@ -18,11 +18,15 @@ release ВСЕГДА в finally — lease не должен течь, даже Бан площадки (#2600 п.1): если исключение, поднятое ИЗНУТРИ `with curl_proxy_url(...) as url:`, — `isinstance` от `ProxyBanError` (mixin, который уже наследуют `AvitoBlockedError`/ -`DomClickBlockedError` и т.п. — см. `proxy_errors.ProxyBanError`), это НЕ просто +`DomClickBlockedError` и т.п. — см. `proxy_errors.ProxyBanError`), это НЕ `mark_health(ok=False)` (транзиентный сбой, инкремент consecutive_fails), а немедленный `mark_banned` — узел сразу снимается с выдачи ЭТОМУ провайдеру (per-source бан, #2600 п.2; для остальных источников остаётся в строю), кроме случая когда это последний узел, достижимый для провайдера (защита в `app.services.proxy_pool.mark_banned`). +`mark_health(ok=False)` на бане НЕ зовётся (#3310): узел исправен, его отбила площадка, а +глобальный счётчик после трёх банов выводил его из выдачи ВСЕМ источникам — в том числе +последний узел, который защита только что пообещала оставить. Тот же выбор, что у +`BrowserFetcher.report_platform_ban` (#3288). Zero изменений для caller'а: любой provider, который уже поднимает свой Blocked-exception ИЗНУТРИ блока, получает сигнал бесплатно — этот модуль намеренно НЕ импортирует avito_exceptions/domclick_exceptions (generic-прокси-слой не должен знать про конкретные @@ -126,17 +130,19 @@ def curl_proxy_url( raise finally: # mark_health/mark_banned/release — best-effort: проблема пула не должна - # ронять сбор. mark_banned ПЕРЕД mark_health(ok=False) — оба независимы - # (разные поля), но бан — более специфичный/сильный сигнал. + # ронять сбор. Бан ВМЕСТО mark_health(ok=False), а не вдобавок (#3310): отказ + # площадки — свойство пары «узел × источник», глобальный счётчик здоровья он + # копить не должен (см. докстринг модуля). if banned: try: proxy_provider.mark_banned(lease, source=provider) except Exception: logger.warning("proxy_pool: mark_banned failed for %s", provider, exc_info=True) - try: - proxy_provider.mark_health(lease, ok) - except Exception: - logger.warning("proxy_pool: mark_health failed for %s", provider, exc_info=True) + else: + try: + proxy_provider.mark_health(lease, ok) + except Exception: + logger.warning("proxy_pool: mark_health failed for %s", provider, exc_info=True) try: proxy_provider.release(lease) # ОБЯЗАТЕЛЬНО — lease не течёт except Exception: