fix(tradein/proxy): бан площадки рапортует только фетчер — ротация больше не банит свежий узел
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m1s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m1s
Дедуп report_ban по _banned_lease_id не достигал цели при ротации. Узел 13 ловит бан-страницу → фетчер репортит бан 13 и по fail-streak меняет lease на 14 → провайдерский report_ban в providers/avito/detail.py видит уже сброшенный _banned_lease_id и банит СВЕЖИЙ узел 14, который к площадке не ходил. При трёх узлах в пуле одна бан-страница выбивала две трети выдачи на 6 часов с эскалацией ban_count. Убран провайдерский report_ban на ветках SidecarBanPageError в avito/detail.py и domclick/detail.py: фетчер репортит сам, раньше и по правильному lease. Детекты не от сайдкара (firewall / 0 карточек в serp.py, QRATOR-маркеры parse_detail_html) фетчеру не видны — там report_ban остаётся. Плюс два смежных: fetch()-ретрай ловил httpx.HTTPError, подклассом которого является SidecarBanPageError, — каждая бан-страница стоила 2 POST'а и +2 к fail-streak (ротация вдвое раньше задуманного); и NoProxyAvailableError из ротационного _acquire_lease внутри _report_platform_ban вылетала ВМЕСТО SidecarBanPageError, подменяя диагноз platform на infra — теперь ротация там best-effort. Тесты: рабочий пул теперь РОТИРУЮЩИЙ (13→14) — на неподвижном пуле дефект физически не проявляется. Два теста, пинившие прежний контракт (провайдер репортит), инвертированы: у них MagicMock-фетчер, который настоящего рапорта не делает. Refs #3288
This commit is contained in:
parent
3b8545f609
commit
c57138f4c7
6 changed files with 161 additions and 15 deletions
|
|
@ -463,13 +463,18 @@ def _sidecar_ban_page_error(upstream_status: int | None = 401) -> SidecarBanPage
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@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 = MagicMock()
|
||||||
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error())
|
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error())
|
||||||
with pytest.raises(DomClickBlockedError):
|
with pytest.raises(DomClickBlockedError):
|
||||||
await fetch_detail(_CARD_URL, browser_fetcher=bf)
|
await fetch_detail(_CARD_URL, browser_fetcher=bf)
|
||||||
bf.report_ban.assert_called_once()
|
bf.report_ban.assert_not_called()
|
||||||
assert _CARD_URL in bf.report_ban.call_args.args[0]
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|
|
||||||
|
|
@ -67,17 +67,23 @@ async def test_fetch_detail_sidecar_ban_page_raises_platform_block_not_infra() -
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_fetch_detail_sidecar_ban_page_reports_ban() -> None:
|
async def test_fetch_detail_sidecar_ban_page_does_not_report_ban_again() -> None:
|
||||||
"""Подтверждённый маркер-бан обязан попасть в scrape_proxy_source_bans через
|
"""Бан рапортует ФЕТЧЕР (`_report_platform_ban`), провайдер — уже нет (#3288).
|
||||||
report_ban — иначе узел не ротируется и продолжает выдаваться в аренду."""
|
|
||||||
|
До #3288 здесь стоял `assert_called_once()`, и это было верно, пока фетчер о
|
||||||
|
бане не знал. Теперь рапорт идёт из `_post_fetch` — РАНЬШЕ и по правильному
|
||||||
|
lease; повтор отсюда приходит после возможной ротации по fail-streak и банит
|
||||||
|
свежий узел, который к площадке не ходил. Что бан всё-таки доезжает до пула —
|
||||||
|
пин по значению в tests/test_3288_avito_ban_per_source.py (там настоящий
|
||||||
|
BrowserFetcher с фейковым пулом, а не MagicMock).
|
||||||
|
"""
|
||||||
bf = MagicMock()
|
bf = MagicMock()
|
||||||
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error())
|
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error())
|
||||||
|
|
||||||
with pytest.raises(AvitoBlockedError):
|
with pytest.raises(AvitoBlockedError):
|
||||||
await fetch_detail(_ITEM_URL, browser_fetcher=bf)
|
await fetch_detail(_ITEM_URL, browser_fetcher=bf)
|
||||||
|
|
||||||
bf.report_ban.assert_called_once()
|
bf.report_ban.assert_not_called()
|
||||||
assert _ITEM_URL in bf.report_ban.call_args.args[0]
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|
|
||||||
|
|
@ -19,7 +19,13 @@
|
||||||
через обёртку `fetch_detail` он остаётся распознаваемым ПО ТИПУ в цепочке причин
|
через обёртку `fetch_detail` он остаётся распознаваемым ПО ТИПУ в цепочке причин
|
||||||
(приём `_iter_causes`), а не по подстроке «no proxy available» (#3272);
|
(приём `_iter_causes`), а не по подстроке «no proxy available» (#3272);
|
||||||
(г) порядок веток `except`: ban-ветка стоит ДО общего `except Exception`. Пин по
|
(г) порядок веток `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_3196_domclick_ban_kind.py и
|
||||||
tests/test_3283_avito_sidecar_ban_is_platform_ban.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")
|
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||||||
|
|
||||||
from scraper_kit.avito_exceptions import AvitoBlockedError, AvitoSidecarUnavailableError
|
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.contracts import ProxyLease
|
||||||
from scraper_kit.orchestration.pipeline import ban_kind_of_exception
|
from scraper_kit.orchestration.pipeline import ban_kind_of_exception
|
||||||
from scraper_kit.providers.avito.detail import fetch_detail
|
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.health == [(13, False), (13, False)]
|
||||||
assert pool.banned == []
|
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}"
|
||||||
|
|
|
||||||
|
|
@ -539,6 +539,13 @@ class BrowserFetcher:
|
||||||
return await self._post_fetch(
|
return await self._post_fetch(
|
||||||
url, origin, cookies, effective_reset, referer, fetch_mode=fetch_mode
|
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:
|
except (httpx.HTTPError, httpx.TransportError) as exc:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"BrowserFetcher: ошибка запроса (%s), retry через %.1fs: %s",
|
"BrowserFetcher: ошибка запроса (%s), retry через %.1fs: %s",
|
||||||
|
|
@ -860,9 +867,23 @@ class BrowserFetcher:
|
||||||
`_report_fetch_result` может его сменить (ротация после N подряд провалов).
|
`_report_fetch_result` может его сменить (ротация после N подряд провалов).
|
||||||
Fail-streak копим по-прежнему — сменить сожжённый площадкой адрес полезно, и
|
Fail-streak копим по-прежнему — сменить сожжённый площадкой адрес полезно, и
|
||||||
новый lease забаненный узел уже не вернёт (`acquire` фильтрует бан по source.)
|
новый 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_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(
|
async def _post_fetch(
|
||||||
self,
|
self,
|
||||||
|
|
|
||||||
|
|
@ -583,7 +583,11 @@ async def fetch_detail(
|
||||||
# {"infra":26,"platform":7}, при этом ни одной записи для source=avito в
|
# {"infra":26,"platform":7}, при этом ни одной записи для source=avito в
|
||||||
# scrape_proxy_source_bans — узел продолжал выдаваться в аренду). Зеркалит
|
# scrape_proxy_source_bans — узел продолжал выдаваться в аренду). Зеркалит
|
||||||
# DomClick (#3239/#3283, providers/domclick/detail.py).
|
# 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(
|
raise AvitoBlockedError(
|
||||||
f"Avito detail: sidecar detected platform refusal for {full_url}: {exc}"
|
f"Avito detail: sidecar detected platform refusal for {full_url}: {exc}"
|
||||||
) from exc
|
) from exc
|
||||||
|
|
|
||||||
|
|
@ -552,12 +552,16 @@ async def fetch_detail(
|
||||||
# #3239: сайдкар опознал статический отказ площадки по маркерам тела
|
# #3239: сайдкар опознал статический отказ площадки по маркерам тела
|
||||||
# (#3237 перенёс это распознавание из parse_detail_html в сайдкар — иначе
|
# (#3237 перенёс это распознавание из parse_detail_html в сайдкар — иначе
|
||||||
# незавершённое QRATOR-рукопожатие считалось блоком). Это ГЕНУИННЫЙ
|
# незавершённое QRATOR-рукопожатие считалось блоком). Это ГЕНУИННЫЙ
|
||||||
# ban-сигнал, ровно того же рода, что и ветка parse_detail_html ниже,
|
# ban-сигнал, ровно того же рода, что и ветка parse_detail_html ниже.
|
||||||
# поэтому здесь — report_ban, в отличие от транспортной ветки следом.
|
|
||||||
# Статус берём из исключения: на error-пути fetch() обнулил
|
# Статус берём из исключения: на error-пути fetch() обнулил
|
||||||
# last_response_status, а без 401 классификатор поставил бы 'unknown'
|
# last_response_status, а без 401 классификатор поставил бы 'unknown'
|
||||||
# вместо 'platform' и ротация IP не запустилась бы вовсе.
|
# вместо '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(
|
raise DomClickBlockedError(
|
||||||
f"DomClick detail: sidecar detected platform refusal for {card_url}: {exc}",
|
f"DomClick detail: sidecar detected platform refusal for {card_url}: {exc}",
|
||||||
status=exc.upstream_status,
|
status=exc.upstream_status,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue