diff --git a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py index 92346cf6..b96b9036 100644 --- a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py +++ b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py @@ -463,13 +463,18 @@ def _sidecar_ban_page_error(upstream_status: int | None = 401) -> SidecarBanPage @pytest.mark.asyncio -async def test_fetch_detail_reports_ban_on_sidecar_ban_page() -> None: +async def test_fetch_detail_does_not_report_ban_again_on_sidecar_ban_page() -> None: + """Sidecar-бан рапортует сам фетчер (`_report_platform_ban`, #3288) — здесь уже нет. + + Повтор отсюда приходит ПОСЛЕ возможной ротации lease по fail-streak и банил бы + свежий узел. Ветка `parse_detail_html` ниже — другой случай: тот детект наш, + фетчер его не видит, и там report_ban остаётся (см. тест следом за этим блоком). + """ bf = MagicMock() bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error()) with pytest.raises(DomClickBlockedError): await fetch_detail(_CARD_URL, browser_fetcher=bf) - bf.report_ban.assert_called_once() - assert _CARD_URL in bf.report_ban.call_args.args[0] + bf.report_ban.assert_not_called() @pytest.mark.asyncio diff --git a/tradein-mvp/backend/tests/test_3283_avito_sidecar_ban_is_platform_ban.py b/tradein-mvp/backend/tests/test_3283_avito_sidecar_ban_is_platform_ban.py index 71f3441b..7f32bf37 100644 --- a/tradein-mvp/backend/tests/test_3283_avito_sidecar_ban_is_platform_ban.py +++ b/tradein-mvp/backend/tests/test_3283_avito_sidecar_ban_is_platform_ban.py @@ -67,17 +67,23 @@ async def test_fetch_detail_sidecar_ban_page_raises_platform_block_not_infra() - @pytest.mark.asyncio -async def test_fetch_detail_sidecar_ban_page_reports_ban() -> None: - """Подтверждённый маркер-бан обязан попасть в scrape_proxy_source_bans через - report_ban — иначе узел не ротируется и продолжает выдаваться в аренду.""" +async def test_fetch_detail_sidecar_ban_page_does_not_report_ban_again() -> None: + """Бан рапортует ФЕТЧЕР (`_report_platform_ban`), провайдер — уже нет (#3288). + + До #3288 здесь стоял `assert_called_once()`, и это было верно, пока фетчер о + бане не знал. Теперь рапорт идёт из `_post_fetch` — РАНЬШЕ и по правильному + lease; повтор отсюда приходит после возможной ротации по fail-streak и банит + свежий узел, который к площадке не ходил. Что бан всё-таки доезжает до пула — + пин по значению в tests/test_3288_avito_ban_per_source.py (там настоящий + BrowserFetcher с фейковым пулом, а не MagicMock). + """ bf = MagicMock() bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error()) with pytest.raises(AvitoBlockedError): await fetch_detail(_ITEM_URL, browser_fetcher=bf) - bf.report_ban.assert_called_once() - assert _ITEM_URL in bf.report_ban.call_args.args[0] + bf.report_ban.assert_not_called() @pytest.mark.asyncio diff --git a/tradein-mvp/backend/tests/test_3288_avito_ban_per_source.py b/tradein-mvp/backend/tests/test_3288_avito_ban_per_source.py index 2defb071..ecb74cea 100644 --- a/tradein-mvp/backend/tests/test_3288_avito_ban_per_source.py +++ b/tradein-mvp/backend/tests/test_3288_avito_ban_per_source.py @@ -19,7 +19,13 @@ через обёртку `fetch_detail` он остаётся распознаваемым ПО ТИПУ в цепочке причин (приём `_iter_causes`), а не по подстроке «no proxy available» (#3272); (г) порядок веток `except`: ban-ветка стоит ДО общего `except Exception`. Пин по - значению — тот же HTTP 500 с маркером и без него разводит судьбу узла. + значению — тот же HTTP 500 с маркером и без него разводит судьбу узла; + (д) при РОТАЦИИ после бана свежий узел не наказывается за чужой отказ: дедуп по + `_banned_lease_id` сбрасывается взятием нового lease, поэтому единственный + допустимый рапорт — из самого фетчера, до ротации. Второй такой же рапорт + сверху (из провайдера) банил бы узел, который к площадке не ходил; + (е) бан-страница стоит РОВНО один POST: `SidecarBanPageError` — подкласс + `httpx.HTTPError`, и ретрай `fetch()` забирал её себе, удваивая fail-streak. Зеркалит стиль tests/test_3196_domclick_ban_kind.py и tests/test_3283_avito_sidecar_ban_is_platform_ban.py; харнесс пула (фейковый провайдер, @@ -38,7 +44,11 @@ import pytest os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") from scraper_kit.avito_exceptions import AvitoBlockedError, AvitoSidecarUnavailableError -from scraper_kit.browser_fetcher import BrowserFetcher, SidecarBanPageError +from scraper_kit.browser_fetcher import ( + _LEASE_ROTATE_AFTER_FAILS, + BrowserFetcher, + SidecarBanPageError, +) from scraper_kit.contracts import ProxyLease from scraper_kit.orchestration.pipeline import ban_kind_of_exception from scraper_kit.providers.avito.detail import fetch_detail @@ -260,3 +270,99 @@ async def test_same_500_different_marker_gives_different_node_fate() -> None: assert pool.health == [(13, False), (13, False)] assert pool.banned == [] + + +# ── (д) ротация после бана: наказан тот узел, который к площадке ходил ───────── + + +class _RotatingFakePool(_FakePool): + """Пул с НЕСКОЛЬКИМИ узлами: `acquire()` выдаёт их по очереди. + + Отличие от `_FakePool` выше (вечный узел 13) несущее: дедуп рапортов в + `BrowserFetcher.report_ban` держится на `_banned_lease_id`, а тот сбрасывается + взятием нового lease — на неподвижном пуле дефект «второй рапорт банит свежий + узел» физически не проявляется, и тест его не видел бы. + """ + + def __init__(self, leases: list[ProxyLease]) -> None: + super().__init__(lease=None) + self._queue = list(leases) + self.acquired: list[int | None] = [] + + def acquire(self, provider: str) -> ProxyLease | None: + lease = self._queue.pop(0) if self._queue else None + self.acquired.append(lease.id if lease is not None else None) + return lease + + +async def test_rotation_after_ban_does_not_ban_the_fresh_node() -> None: + """Узел 13 поймал бан-страницу, fail-streak сменил его на 14 — забанен ТОЛЬКО 13. + + Прод-сценарий: узел уже сыпался (streak на единицу ниже потолка), бан-страница + добивает его до ротации. Фальсификация: верни `browser_fetcher.report_ban(...)` + в ветку `SidecarBanPageError` в providers/avito/detail.py — к тому кадру lease + уже сменился, дедуп по `_banned_lease_id` сброшен взятием нового lease, и + значение станет [(13, 'avito'), (14, 'avito')]: свежий узел получает 6-часовой + отдых (с эскалацией ban_count) за отказ, которого он не видел. При трёх узлах + в пуле это выбивает две трети выдачи с одной бан-страницы. + """ + pool = _RotatingFakePool([_LEASE, ProxyLease(id=14, url="http://node14:8080", kind="http")]) + bf = await _fetcher(_ban_page_response(), pool) + bf._lease_fail_streak = _LEASE_ROTATE_AFTER_FAILS - 1 + + with patch("scraper_kit.browser_fetcher.asyncio.sleep", AsyncMock()): + with pytest.raises(AvitoBlockedError): + await fetch_detail(_ITEM_URL, browser_fetcher=bf) + + assert pool.acquired == [13, 14], f"ротация обязана была произойти: {pool.acquired}" + assert pool.banned == [(13, "avito")], ( + f"забанен должен быть узел, который сходил на площадку и получил отказ, " + f"а не тот, что пришёл ему на смену: {pool.banned}" + ) + + +# ── (е) одна бан-страница — один POST ───────────────────────────────────────── + + +async def test_ban_page_costs_exactly_one_post() -> None: + """Ретрай `fetch()` не имеет права трогать бан-страницу: это ответ площадки. + + Фальсификация: убери `except SidecarBanPageError: raise` перед + `except (httpx.HTTPError, ...)` в `fetch()` — станет 2 POST'а, а с ними и +2 к + `_lease_fail_streak` вместо +1 (ротация вдвое раньше задуманного). + """ + pool = _FakePool() + bf = await _fetcher(_ban_page_response(), pool) + + with patch("scraper_kit.browser_fetcher.asyncio.sleep", AsyncMock()): + with pytest.raises(AvitoBlockedError): + await fetch_detail(_ITEM_URL, browser_fetcher=bf) + + assert bf._client.post.await_count == 1, ( + f"бан-страницу ретраить нечем — площадка уже ответила: " + f"{bf._client.post.await_count} POST'ов" + ) + + +# ── (ж) пустой пул при ротации не подменяет диагноз ─────────────────────────── + + +async def test_empty_pool_during_post_ban_rotation_keeps_platform_diagnosis() -> None: + """Ротация — best-effort: её `NoProxyAvailableError` не должна съесть бан. + + Второго узла в пуле нет, поэтому `_acquire_lease()` внутри ротации падает. + Фальсификация: убери `except NoProxyAvailableError` в `_report_platform_ban` — + наверх уедет она вместо `SidecarBanPageError`, провайдер завернёт её в + AvitoSidecarUnavailableError, и подтверждённый отказ площадки попадёт в + `scrape_runs.ban_kind` как 'infra' (ровно та подмена, что чинил #3283). + """ + pool = _RotatingFakePool([_LEASE]) + bf = await _fetcher(_ban_page_response(), pool) + bf._lease_fail_streak = _LEASE_ROTATE_AFTER_FAILS - 1 + + with patch("scraper_kit.browser_fetcher.asyncio.sleep", AsyncMock()): + with pytest.raises(AvitoBlockedError) as ei: + await fetch_detail(_ITEM_URL, browser_fetcher=bf) + + assert ban_kind_of_exception(ei.value) == BAN_KIND_PLATFORM + assert pool.banned == [(13, "avito")], f"бан обязан быть отрапортован до ротации: {pool.banned}" diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/browser_fetcher.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/browser_fetcher.py index 3606a5fa..95c116bc 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/browser_fetcher.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/browser_fetcher.py @@ -539,6 +539,13 @@ class BrowserFetcher: return await self._post_fetch( url, origin, cookies, effective_reset, referer, fetch_mode=fetch_mode ) + except SidecarBanPageError: + # #3288: бан-страница — ОТВЕТ площадки, а не блип сайдкара; ретраить нечего. + # Ветка обязана стоять ДО httpx.HTTPError (SidecarBanPageError — его подкласс, + # см. test_sidecar_ban_page_is_a_subclass_of_httpx_error): иначе один бан + # давал второй POST тем же узлом и ВТОРОЙ инкремент _lease_fail_streak — + # ротация наступала вдвое раньше, чем задумано (_LEASE_ROTATE_AFTER_FAILS). + raise except (httpx.HTTPError, httpx.TransportError) as exc: logger.warning( "BrowserFetcher: ошибка запроса (%s), retry через %.1fs: %s", @@ -860,9 +867,23 @@ class BrowserFetcher: `_report_fetch_result` может его сменить (ротация после N подряд провалов). Fail-streak копим по-прежнему — сменить сожжённый площадкой адрес полезно, и новый lease забаненный узел уже не вернёт (`acquire` фильтрует бан по source.) + + Ротация внутри `_report_fetch_result` — best-effort: её `_acquire_lease()` может + поднять `NoProxyAvailableError` (прод, пул опустел mid-run), и та вылетела бы + ВМЕСТО `SidecarBanPageError`, ради которой мы сюда попали, — прогон получил бы + диагноз 'infra' на подтверждённом отказе площадки. Глотаем ровно её и ровно + здесь: бан уже отрапортован (`report_ban` выше), а «нечем ходить дальше» caller + узнает на следующем /fetch, где lease действительно нужен. """ self.report_ban(reason) - self._report_fetch_result(False, health=False) + try: + self._report_fetch_result(False, health=False) + except NoProxyAvailableError: + logger.warning( + "BrowserFetcher: пул пуст при ротации после бана (%s) — бан отрапортован, " + "поднимаем исходную ошибку площадки", + self._source, + ) async def _post_fetch( self, diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/avito/detail.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/avito/detail.py index a34a1835..a8797516 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/avito/detail.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/avito/detail.py @@ -583,7 +583,11 @@ async def fetch_detail( # {"infra":26,"platform":7}, при этом ни одной записи для source=avito в # scrape_proxy_source_bans — узел продолжал выдаваться в аренду). Зеркалит # DomClick (#3239/#3283, providers/domclick/detail.py). - browser_fetcher.report_ban(f"avito detail: sidecar ban page for {full_url}") + # + # report_ban ЗДЕСЬ НЕТ намеренно (#3288): бан рапортует сам fetcher, в + # _report_platform_ban, — раньше и по ПРАВИЛЬНОМУ lease. К моменту этого + # кадра fetcher мог уже сротироваться на свежий узел по fail-streak, и + # повторный рапорт отсюда банил бы того, кто к площадке не ходил. raise AvitoBlockedError( f"Avito detail: sidecar detected platform refusal for {full_url}: {exc}" ) from exc diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py index 27a7484f..4cf63d4b 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py @@ -552,12 +552,16 @@ async def fetch_detail( # #3239: сайдкар опознал статический отказ площадки по маркерам тела # (#3237 перенёс это распознавание из parse_detail_html в сайдкар — иначе # незавершённое QRATOR-рукопожатие считалось блоком). Это ГЕНУИННЫЙ - # ban-сигнал, ровно того же рода, что и ветка parse_detail_html ниже, - # поэтому здесь — report_ban, в отличие от транспортной ветки следом. + # ban-сигнал, ровно того же рода, что и ветка parse_detail_html ниже. # Статус берём из исключения: на error-пути fetch() обнулил # last_response_status, а без 401 классификатор поставил бы 'unknown' # вместо 'platform' и ротация IP не запустилась бы вовсе. - browser_fetcher.report_ban(f"domclick detail: sidecar ban page for {card_url}") + # + # report_ban ЗДЕСЬ НЕТ намеренно (#3288): его делает сам fetcher в + # _report_platform_ban — раньше и по ПРАВИЛЬНОМУ lease (после ротации по + # fail-streak этот кадр забанил бы уже СЛЕДУЮЩИЙ узел). Ветка + # parse_detail_html ниже — другой случай: там детект НАШ, fetcher его не + # видит, и report_ban остаётся. raise DomClickBlockedError( f"DomClick detail: sidecar detected platform refusal for {card_url}: {exc}", status=exc.upstream_status,