fix(scraper-kit/proxy): бан на curl-пути не копит счётчик отказов узла (#3310)
Защита последнего узла в mark_banned бережёт узел от бан-строки и пишет «узел продолжит выдаваться», но curl_proxy_url на том же ProxyBanError вслед за баном звал mark_health(ok=False). Три ответа 403/капчи подряд — consecutive_fails = 3, и acquire отсекал узел для ВСЕХ источников: пул пуст при живом узле. Браузерный путь это исправил в #3288 (report_platform_ban), curl-путь — нет; через него ходят cian detail, ЖК-резолв, история цен Циана и оценщик. Теперь на ProxyBanError зовётся mark_banned вместо mark_health(False). Транспортные сбои по-прежнему засчитываются узлу. Тест на живом Postgres настоящим трактом (curl_proxy_url → RealProxyProvider → proxy_pool): единственный узел, три CianBlockedError → consecutive_fails == 0, acquire('cian') выдаёт узел; на старом коде «assert 3 == 0». Пять старых проверок ждали mark_health(False) на бане — они фиксировали дефект, поправлены. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
92295971fe
commit
611878df4c
6 changed files with 148 additions and 15 deletions
|
|
@ -103,7 +103,7 @@ async def test_403_bans_the_node_for_cian_only() -> None:
|
||||||
with pytest.raises(CianBlockedError):
|
with pytest.raises(CianBlockedError):
|
||||||
await _fetch(403, spy)
|
await _fetch(403, spy)
|
||||||
assert spy.mark_banned_calls == [(1, "cian")]
|
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 не течёт даже на бане
|
assert spy.release_calls == [1] # lease не течёт даже на бане
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -179,7 +179,7 @@ async def test_price_history_403_bans_the_node_for_cian() -> None:
|
||||||
pool = _SpyPool()
|
pool = _SpyPool()
|
||||||
result = await _run_price_history(pool, status_code=403)
|
result = await _run_price_history(pool, status_code=403)
|
||||||
assert pool.mark_banned_calls == [(9, "cian")]
|
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 # прогон честен: отказ посчитан
|
assert result.errors == 1 # прогон честен: отказ посчитан
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -335,7 +335,7 @@ async def test_zhk_resolve_403_reaches_the_pool() -> None:
|
||||||
with pytest.raises(CianBlockedError):
|
with pytest.raises(CianBlockedError):
|
||||||
await _resolve(403, spy)
|
await _resolve(403, spy)
|
||||||
assert spy.mark_banned_calls == [(9, "cian")]
|
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]
|
assert spy.release_calls == [9]
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
126
tradein-mvp/backend/tests/test_3310_curl_ban_keeps_last_node.py
Normal file
126
tradein-mvp/backend/tests/test_3310_curl_ban_keeps_last_node.py
Normal file
|
|
@ -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
|
||||||
|
|
@ -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 spy.mark_banned_calls == [(1, "cian")]
|
||||||
assert (1, True) not in spy.mark_health_calls, "узел с капчей записан здоровым"
|
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 не течёт
|
assert spy.release_calls == [1] # lease не течёт
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -127,10 +127,10 @@ def test_flag_on_exception_marks_fail_and_still_releases() -> None:
|
||||||
assert spy.release_calls == [7]
|
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) внутри блока —
|
"""Исключение — подкласс ProxyBanError (напр. AvitoBlockedError) внутри блока —
|
||||||
вызывает mark_banned(lease, source=provider) В ДОПОЛНЕНИЕ к mark_health(ok=False)
|
вызывает mark_banned(lease, source=provider) ВМЕСТО mark_health(ok=False)
|
||||||
(#2600 п.1 curl-путь). Zero изменений в вызывающем коде — сигнал детектируется
|
(#2600 п.1 curl-путь, #3310). Zero изменений в вызывающем коде — сигнал детектируется
|
||||||
по ТИПУ исключения, а не явным вызовом."""
|
по ТИПУ исключения, а не явным вызовом."""
|
||||||
|
|
||||||
class _FakeBlockedError(ProxyBanError):
|
class _FakeBlockedError(ProxyBanError):
|
||||||
|
|
@ -143,7 +143,8 @@ def test_ban_exception_calls_mark_banned_in_addition_to_mark_health() -> None:
|
||||||
assert url == _LEASE.url
|
assert url == _LEASE.url
|
||||||
raise _FakeBlockedError("firewall page detected")
|
raise _FakeBlockedError("firewall page detected")
|
||||||
assert spy.mark_banned_calls == [(7, "avito")]
|
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 всё равно освобождён
|
assert spy.release_calls == [7] # lease всё равно освобождён
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -18,11 +18,15 @@ release ВСЕГДА в finally — lease не должен течь, даже
|
||||||
|
|
||||||
Бан площадки (#2600 п.1): если исключение, поднятое ИЗНУТРИ `with curl_proxy_url(...) as
|
Бан площадки (#2600 п.1): если исключение, поднятое ИЗНУТРИ `with curl_proxy_url(...) as
|
||||||
url:`, — `isinstance` от `ProxyBanError` (mixin, который уже наследуют `AvitoBlockedError`/
|
url:`, — `isinstance` от `ProxyBanError` (mixin, который уже наследуют `AvitoBlockedError`/
|
||||||
`DomClickBlockedError` и т.п. — см. `proxy_errors.ProxyBanError`), это НЕ просто
|
`DomClickBlockedError` и т.п. — см. `proxy_errors.ProxyBanError`), это НЕ
|
||||||
`mark_health(ok=False)` (транзиентный сбой, инкремент consecutive_fails), а немедленный
|
`mark_health(ok=False)` (транзиентный сбой, инкремент consecutive_fails), а немедленный
|
||||||
`mark_banned` — узел сразу снимается с выдачи ЭТОМУ провайдеру (per-source бан, #2600 п.2;
|
`mark_banned` — узел сразу снимается с выдачи ЭТОМУ провайдеру (per-source бан, #2600 п.2;
|
||||||
для остальных источников остаётся в строю), кроме случая когда это последний узел,
|
для остальных источников остаётся в строю), кроме случая когда это последний узел,
|
||||||
достижимый для провайдера (защита в `app.services.proxy_pool.mark_banned`).
|
достижимый для провайдера (защита в `app.services.proxy_pool.mark_banned`).
|
||||||
|
`mark_health(ok=False)` на бане НЕ зовётся (#3310): узел исправен, его отбила площадка, а
|
||||||
|
глобальный счётчик после трёх банов выводил его из выдачи ВСЕМ источникам — в том числе
|
||||||
|
последний узел, который защита только что пообещала оставить. Тот же выбор, что у
|
||||||
|
`BrowserFetcher.report_platform_ban` (#3288).
|
||||||
Zero изменений для caller'а: любой provider, который уже поднимает свой Blocked-exception
|
Zero изменений для caller'а: любой provider, который уже поднимает свой Blocked-exception
|
||||||
ИЗНУТРИ блока, получает сигнал бесплатно — этот модуль намеренно НЕ импортирует
|
ИЗНУТРИ блока, получает сигнал бесплатно — этот модуль намеренно НЕ импортирует
|
||||||
avito_exceptions/domclick_exceptions (generic-прокси-слой не должен знать про конкретные
|
avito_exceptions/domclick_exceptions (generic-прокси-слой не должен знать про конкретные
|
||||||
|
|
@ -126,13 +130,15 @@ def curl_proxy_url(
|
||||||
raise
|
raise
|
||||||
finally:
|
finally:
|
||||||
# mark_health/mark_banned/release — best-effort: проблема пула не должна
|
# mark_health/mark_banned/release — best-effort: проблема пула не должна
|
||||||
# ронять сбор. mark_banned ПЕРЕД mark_health(ok=False) — оба независимы
|
# ронять сбор. Бан ВМЕСТО mark_health(ok=False), а не вдобавок (#3310): отказ
|
||||||
# (разные поля), но бан — более специфичный/сильный сигнал.
|
# площадки — свойство пары «узел × источник», глобальный счётчик здоровья он
|
||||||
|
# копить не должен (см. докстринг модуля).
|
||||||
if banned:
|
if banned:
|
||||||
try:
|
try:
|
||||||
proxy_provider.mark_banned(lease, source=provider)
|
proxy_provider.mark_banned(lease, source=provider)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.warning("proxy_pool: mark_banned failed for %s", provider, exc_info=True)
|
logger.warning("proxy_pool: mark_banned failed for %s", provider, exc_info=True)
|
||||||
|
else:
|
||||||
try:
|
try:
|
||||||
proxy_provider.mark_health(lease, ok)
|
proxy_provider.mark_health(lease, ok)
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue