fix(tradein/domclick): подтверждённый отказ площадки уехал в ветку «сбой транспорта» и перестал банить узел #3241
6 changed files with 275 additions and 2 deletions
|
|
@ -36,7 +36,9 @@ import json
|
|||
from datetime import UTC, datetime, timedelta
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from scraper_kit.browser_fetcher import SidecarBanPageError
|
||||
from scraper_kit.domclick_exceptions import DomClickBlockedError, DomClickParseError
|
||||
from scraper_kit.offer_price_history import clamp_diff_percent
|
||||
from scraper_kit.providers.domclick.detail import (
|
||||
|
|
@ -438,6 +440,61 @@ async def test_fetch_detail_transport_failure_does_not_report_ban() -> None:
|
|||
bf.report_ban.assert_not_called()
|
||||
|
||||
|
||||
# ── #3239: бан-страница ОТ САЙДКАРА — тоже генуинный маркер-детект ────────────
|
||||
#
|
||||
# #3237 перенёс распознавание статического отказа Домклика из parse_detail_html в
|
||||
# сайдкар (иначе незавершённое QRATOR-рукопожатие считалось блоком). Отказ стал
|
||||
# приезжать обычной 500-кой и попадать в транспортную ветку, где report_ban по
|
||||
# построению НЕ зовётся → ban_kind 'unknown' вместо 'platform' → ротация IP не
|
||||
# запускалась вовсе. Тесты ниже держат обе ветки РАЗДЕЛЬНО: подтверждённый отказ
|
||||
# банит, голый транспортный сбой — нет (тест выше).
|
||||
|
||||
|
||||
def _sidecar_ban_page_error(upstream_status: int | None = 401) -> SidecarBanPageError:
|
||||
return SidecarBanPageError(
|
||||
"Server error '500' | tradein-browser: BanPageDetectedError: статический отказ",
|
||||
request=httpx.Request("POST", "http://tradein-browser:3000/fetch"),
|
||||
response=httpx.Response(500, request=httpx.Request("POST", "http://x/fetch")),
|
||||
upstream_status=upstream_status,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_detail_reports_ban_on_sidecar_ban_page() -> None:
|
||||
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]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_detail_sidecar_ban_page_carries_upstream_status() -> None:
|
||||
"""Статус берётся ИЗ ИСКЛЮЧЕНИЯ, а не из last_response_status.
|
||||
|
||||
Несущая деталь #3239: на error-пути fetch() обнуляет last_response_status,
|
||||
поэтому у Домклика (отказ приходит с 401) классификатор без этого поля
|
||||
поставил бы 'unknown'. MagicMock отдаёт last_response_status как Mock —
|
||||
если бы код читал его, ассерт ниже упал бы.
|
||||
"""
|
||||
bf = MagicMock()
|
||||
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error(401))
|
||||
with pytest.raises(DomClickBlockedError) as excinfo:
|
||||
await fetch_detail(_CARD_URL, browser_fetcher=bf)
|
||||
assert excinfo.value.status == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_detail_sidecar_ban_page_without_status_stays_none() -> None:
|
||||
"""Сайдкар не отдал статус → None, а не выдуманный код (#2764)."""
|
||||
bf = MagicMock()
|
||||
bf.fetch = AsyncMock(side_effect=_sidecar_ban_page_error(None))
|
||||
with pytest.raises(DomClickBlockedError) as excinfo:
|
||||
await fetch_detail(_CARD_URL, browser_fetcher=bf)
|
||||
assert excinfo.value.status is None
|
||||
|
||||
|
||||
# ── save_detail_enrichment (MagicMock — зеркало test_cian_detail) ─────────────
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -27,8 +27,14 @@ from __future__ import annotations
|
|||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from scraper_kit.browser_fetcher import BrowserFetcher, ban_kind_from_status
|
||||
from scraper_kit.browser_fetcher import (
|
||||
BrowserFetcher,
|
||||
SidecarBanPageError,
|
||||
_raise_for_sidecar_status,
|
||||
ban_kind_from_status,
|
||||
)
|
||||
|
||||
|
||||
def _mock_client(json_payload: dict[str, Any], *, raise_exc: Exception | None = None) -> MagicMock:
|
||||
|
|
@ -158,3 +164,73 @@ def test_ban_kind_values_fit_scrape_runs_check() -> None:
|
|||
allowed = {"platform", "infra", "unknown", None}
|
||||
for status in (None, 200, 301, 403, 404, 429, 499, 500, 503, 599, 600):
|
||||
assert ban_kind_from_status(status) in allowed
|
||||
|
||||
|
||||
# ── #3239: подтверждённая бан-страница отличима от прочих 500-ок ──────────────
|
||||
#
|
||||
# Тип, а не подстрока: _raise_for_sidecar_status обрезает тело до 300 символов, и
|
||||
# формулировка отказа у сайдкара менялась дважды за месяц. upstream_status нужен
|
||||
# отдельным полем — на error-пути fetch() обнуляет last_response_status, и без
|
||||
# него у DomClick (отказ приходит с 401) диагноз стал бы 'unknown' вместо
|
||||
# 'platform', то есть ротация IP не запустилась бы вовсе.
|
||||
|
||||
|
||||
def _sidecar_response(body: Any) -> httpx.Response:
|
||||
return httpx.Response(
|
||||
500, json=body, request=httpx.Request("POST", "http://tradein-browser:3000/fetch")
|
||||
)
|
||||
|
||||
|
||||
def test_ban_page_body_raises_typed_error_with_upstream_status() -> None:
|
||||
resp = _sidecar_response(
|
||||
{"error": "BanPageDetectedError: статический отказ", "ban_page": True, "status": 401}
|
||||
)
|
||||
with pytest.raises(SidecarBanPageError) as excinfo:
|
||||
_raise_for_sidecar_status(resp)
|
||||
assert excinfo.value.upstream_status == 401
|
||||
|
||||
|
||||
def test_ban_page_error_is_httpx_status_error() -> None:
|
||||
"""Подкласс — иначе retry-политика fetch() и ловля у прочих поставщиков сломались бы."""
|
||||
resp = _sidecar_response({"error": "BanPageDetectedError: x", "ban_page": True, "status": 403})
|
||||
with pytest.raises(httpx.HTTPStatusError):
|
||||
_raise_for_sidecar_status(resp)
|
||||
|
||||
|
||||
def test_plain_500_stays_plain_status_error() -> None:
|
||||
resp = _sidecar_response({"error": "Error: Page.goto: NS_ERROR_PROXY_BAD_GATEWAY"})
|
||||
with pytest.raises(httpx.HTTPStatusError) as excinfo:
|
||||
_raise_for_sidecar_status(resp)
|
||||
assert not isinstance(excinfo.value, SidecarBanPageError)
|
||||
|
||||
|
||||
def test_ban_page_without_status_gives_none() -> None:
|
||||
resp = _sidecar_response({"error": "BanPageDetectedError: x", "ban_page": True, "status": None})
|
||||
with pytest.raises(SidecarBanPageError) as excinfo:
|
||||
_raise_for_sidecar_status(resp)
|
||||
assert excinfo.value.upstream_status is None
|
||||
|
||||
|
||||
def test_ban_page_true_as_bool_status_is_rejected() -> None:
|
||||
"""JSON true не должен уехать статусом: bool — подтип int (та же ловушка, что в #3196)."""
|
||||
resp = _sidecar_response({"error": "x", "ban_page": True, "status": True})
|
||||
with pytest.raises(SidecarBanPageError) as excinfo:
|
||||
_raise_for_sidecar_status(resp)
|
||||
assert excinfo.value.upstream_status is None
|
||||
|
||||
|
||||
def test_non_json_error_body_does_not_break() -> None:
|
||||
resp = httpx.Response(
|
||||
500, text="not json", request=httpx.Request("POST", "http://tradein-browser:3000/fetch")
|
||||
)
|
||||
with pytest.raises(httpx.HTTPStatusError) as excinfo:
|
||||
_raise_for_sidecar_status(resp)
|
||||
assert not isinstance(excinfo.value, SidecarBanPageError)
|
||||
|
||||
|
||||
def test_success_response_raises_nothing() -> None:
|
||||
resp = httpx.Response(
|
||||
200, json={"html": "<html/>", "status": 200},
|
||||
request=httpx.Request("POST", "http://tradein-browser:3000/fetch"),
|
||||
)
|
||||
_raise_for_sidecar_status(resp)
|
||||
|
|
|
|||
|
|
@ -961,7 +961,17 @@ async def fetch_handler(request: web.Request) -> web.Response:
|
|||
type(exc).__name__,
|
||||
exc,
|
||||
)
|
||||
return web.json_response({"error": f"{type(exc).__name__}: {exc}"}, status=500)
|
||||
error_body: dict = {"error": f"{type(exc).__name__}: {exc}"}
|
||||
if isinstance(exc, BanPageDetectedError):
|
||||
# #3239: бан-страница — ПОДТВЕРЖДЁННЫЙ маркер-детект, а не сбой
|
||||
# транспорта, и только за первым стоит report_ban на клиенте.
|
||||
# До этой правки оба случая приезжали одинаковой 500-кой, клиент
|
||||
# различить их не мог и настоящий отказ площадки переставал
|
||||
# ротировать узел (регрессия #3237). Код ответа НЕ меняем: на 500
|
||||
# завязана classify_browser_probe, признак несёт тело.
|
||||
error_body["ban_page"] = True
|
||||
error_body["status"] = _last_response_status.get(provider)
|
||||
return web.json_response(error_body, status=500)
|
||||
|
||||
# Аддитивно (#3196): ключ "html" на месте и не изменился — клиент, читающий
|
||||
# только его, ничего не заметит. "status" может быть null (goto вернул None).
|
||||
|
|
|
|||
|
|
@ -293,3 +293,62 @@ def test_fetch_handler_status_null_without_response(monkeypatch: pytest.MonkeyPa
|
|||
body = _json_body(response)
|
||||
assert body["html"] == _REAL_HTML
|
||||
assert body["status"] is None
|
||||
|
||||
|
||||
# ── #3239: 500-ка бан-страницы отличима от 500-ки транспорта ──────────────────
|
||||
|
||||
|
||||
def _fetch_handler_error_body(
|
||||
monkeypatch: pytest.MonkeyPatch, exc: Exception, upstream_status: int | None
|
||||
) -> tuple[int, dict]:
|
||||
monkeypatch.setattr(server, "IS_PROD", False)
|
||||
|
||||
async def _ensure(provider: str, proxy_override: str | None = None) -> bool:
|
||||
return True
|
||||
|
||||
async def _fake_do_fetch(provider: str, url: str, **_kw: Any) -> str:
|
||||
server._last_response_status[provider] = upstream_status
|
||||
raise exc
|
||||
|
||||
monkeypatch.setattr(server, "_ensure_browser", _ensure)
|
||||
monkeypatch.setattr(server, "_do_fetch", _fake_do_fetch)
|
||||
response = asyncio.run(server.fetch_handler(_make_request({"url": "https://domclick.ru/x"})))
|
||||
return response.status, _json_body(response)
|
||||
|
||||
|
||||
def test_fetch_handler_marks_ban_page_in_error_body(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Бан-страница несёт ban_page + апстрим-статус.
|
||||
|
||||
Без этого признака клиент видит обычную 500-ку, уводит отказ в транспортную
|
||||
ветку (report_ban там не зовётся) и перестаёт ротировать отказавший узел —
|
||||
регрессия, которую #3237 внёс, а #3239 чинит.
|
||||
"""
|
||||
status, body = _fetch_handler_error_body(
|
||||
monkeypatch, server.BanPageDetectedError("статический отказ площадки"), 401
|
||||
)
|
||||
assert status == 500 # код НЕ меняем: на него завязана classify_browser_probe
|
||||
assert body["ban_page"] is True
|
||||
assert body["status"] == 401
|
||||
assert "BanPageDetectedError" in body["error"]
|
||||
|
||||
|
||||
def test_fetch_handler_transport_error_has_no_ban_page_flag(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Обычный сбой — признака нет вовсе, клиент трактует его как раньше."""
|
||||
status, body = _fetch_handler_error_body(
|
||||
monkeypatch, RuntimeError("NS_ERROR_PROXY_BAD_GATEWAY"), None
|
||||
)
|
||||
assert status == 500
|
||||
assert "ban_page" not in body
|
||||
|
||||
|
||||
def test_fetch_handler_ban_page_without_status_stays_null(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Апстрим-статуса нет — отдаём null, а не выдуманный код (#2764)."""
|
||||
_status, body = _fetch_handler_error_body(
|
||||
monkeypatch, server.BanPageDetectedError("бан-страница"), None
|
||||
)
|
||||
assert body["ban_page"] is True
|
||||
assert body["status"] is None
|
||||
|
|
|
|||
|
|
@ -130,6 +130,51 @@ _PROXY_FAIL_MARKERS: tuple[str, ...] = (
|
|||
_LEASE_ROTATE_AFTER_FAILS: int = 3
|
||||
|
||||
|
||||
class SidecarBanPageError(httpx.HTTPStatusError):
|
||||
"""Сайдкар подтвердил бан-страницу по маркерам её тела (#3239).
|
||||
|
||||
Подкласс ``HTTPStatusError``, а не самостоятельный тип: ловля по
|
||||
``httpx.HTTPError`` у всех прочих поставщиков и retry-политика
|
||||
``fetch()`` продолжают работать не зная о нём. Отличать его нужно ровно
|
||||
там, где решается судьба узла: за подтверждённым маркером стоит
|
||||
``report_ban`` (площадка отказала этому IP), за обычной 500-кой — нет
|
||||
(сбой транспорта, #2600 п.4).
|
||||
|
||||
``upstream_status`` — HTTP-код САМОЙ целевой навигации, а не ответа
|
||||
сайдкара. На error-пути ``fetch()`` обнуляет ``last_response_status``,
|
||||
поэтому иначе диагноз узнать неоткуда: у DomClick отказ приходит с 401
|
||||
и без него классификатор ставит 'unknown' вместо 'platform'.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
*,
|
||||
request: httpx.Request,
|
||||
response: httpx.Response,
|
||||
upstream_status: int | None,
|
||||
) -> None:
|
||||
super().__init__(message, request=request, response=response)
|
||||
self.upstream_status = upstream_status
|
||||
|
||||
|
||||
def _sidecar_ban_page_status(resp: httpx.Response) -> tuple[bool, int | None]:
|
||||
"""(это бан-страница?, апстрим-статус) из тела ошибки сайдкара (#3239).
|
||||
|
||||
Тело — ``{"error": ..., "ban_page": true, "status": <int|null>}``. Сайдкар
|
||||
старой сборки ключей не отдаёт → (False, None), поведение как до правки.
|
||||
``bool`` отсекаем явно: он подтип ``int`` и JSON ``true`` уехал бы статусом.
|
||||
"""
|
||||
try:
|
||||
body = resp.json()
|
||||
except Exception:
|
||||
return False, None
|
||||
if not isinstance(body, dict) or body.get("ban_page") is not True:
|
||||
return False, None
|
||||
raw = body.get("status")
|
||||
return True, raw if isinstance(raw, int) and not isinstance(raw, bool) else None
|
||||
|
||||
|
||||
def _raise_for_sidecar_status(resp: httpx.Response) -> None:
|
||||
"""`raise_for_status()`, но с ПРИЧИНОЙ отказа из тела ответа сайдкара в тексте ошибки.
|
||||
|
||||
|
|
@ -153,6 +198,17 @@ def _raise_for_sidecar_status(resp: httpx.Response) -> None:
|
|||
# Тело не прочиталось/не декодируется — причина не обязана быть; отдаём
|
||||
# исходную ошибку, а не роняем вызывающего на разборе тела.
|
||||
raise exc from None
|
||||
is_ban_page, upstream_status = _sidecar_ban_page_status(resp)
|
||||
if is_ban_page:
|
||||
# #3239: тип несёт диагноз наверх — подстрокой в тексте его искать
|
||||
# нельзя, detail обрезан до 300 символов и формулировка отказа
|
||||
# менялась дважды за месяц.
|
||||
raise SidecarBanPageError(
|
||||
f"{exc} | tradein-browser: {detail or 'ban page'}",
|
||||
request=exc.request,
|
||||
response=exc.response,
|
||||
upstream_status=upstream_status,
|
||||
) from exc
|
||||
if not detail:
|
||||
raise
|
||||
raise httpx.HTTPStatusError(
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ from scraper_kit.domclick_exceptions import (
|
|||
DomClickBlockedError,
|
||||
DomClickParseError,
|
||||
)
|
||||
from scraper_kit.browser_fetcher import SidecarBanPageError
|
||||
from scraper_kit.offer_price_history import clamp_diff_percent
|
||||
from scraper_kit.repair_state_normalizer import (
|
||||
infer_repair_state_from_text,
|
||||
|
|
@ -544,6 +545,20 @@ async def fetch_detail(
|
|||
# но НЕ сообщаем пулу здесь: неизвестно, был ли это реальный маркер-бан или
|
||||
# обёртка сетевой ошибки — не смешиваем "сеть" с "бан" (issue #2600 п.4).
|
||||
raise
|
||||
except SidecarBanPageError as exc:
|
||||
# #3239: сайдкар опознал статический отказ площадки по маркерам тела
|
||||
# (#3237 перенёс это распознавание из parse_detail_html в сайдкар — иначе
|
||||
# незавершённое QRATOR-рукопожатие считалось блоком). Это ГЕНУИННЫЙ
|
||||
# ban-сигнал, ровно того же рода, что и ветка parse_detail_html ниже,
|
||||
# поэтому здесь — report_ban, в отличие от транспортной ветки следом.
|
||||
# Статус берём из исключения: на error-пути fetch() обнулил
|
||||
# last_response_status, а без 401 классификатор поставил бы 'unknown'
|
||||
# вместо 'platform' и ротация IP не запустилась бы вовсе.
|
||||
browser_fetcher.report_ban(f"domclick detail: sidecar ban page for {card_url}")
|
||||
raise DomClickBlockedError(
|
||||
f"DomClick detail: sidecar detected platform refusal for {card_url}: {exc}",
|
||||
status=exc.upstream_status,
|
||||
) from exc
|
||||
except Exception as exc:
|
||||
# Сетевая/инфраструктурная ошибка самого fetch() (timeout/5xx/transport) —
|
||||
# НЕ репортим mark_banned: это не подтверждённый маркер-бан площадки, а
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue