From c5784e85bce81624c11db77de44465a654ac48e6 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 29 Aug 2026 19:12:36 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/domclick):=20=D0=BF=D0=BE=D0=B4?= =?UTF-8?q?=D1=82=D0=B2=D0=B5=D1=80=D0=B6=D0=B4=D1=91=D0=BD=D0=BD=D1=8B?= =?UTF-8?q?=D0=B9=20=D0=BE=D1=82=D0=BA=D0=B0=D0=B7=20=D0=BF=D0=BB=D0=BE?= =?UTF-8?q?=D1=89=D0=B0=D0=B4=D0=BA=D0=B8=20=D1=83=D0=B5=D1=85=D0=B0=D0=BB?= =?UTF-8?q?=20=D0=B2=20=D0=B2=D0=B5=D1=82=D0=BA=D1=83=20=C2=AB=D1=81=D0=B1?= =?UTF-8?q?=D0=BE=D0=B9=20=D1=82=D1=80=D0=B0=D0=BD=D1=81=D0=BF=D0=BE=D1=80?= =?UTF-8?q?=D1=82=D0=B0=C2=BB=20=D0=B8=20=D0=BF=D0=B5=D1=80=D0=B5=D1=81?= =?UTF-8?q?=D1=82=D0=B0=D0=BB=20=D0=B1=D0=B0=D0=BD=D0=B8=D1=82=D1=8C=20?= =?UTF-8?q?=D1=83=D0=B7=D0=B5=D0=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3237 научил сайдкар опознавать статический отказ Домклика самостоятельно — это правильно и работает, но вместе с распознаванием ban-сигнал переехал не туда. Отказ стал приезжать обычной 500-кой: browser_fetcher обнуляет на ней last_response_status, и в detail.py срабатывает ветка except Exception, которая по построению НЕ зовёт report_ban («не подтверждённый маркер-бан, а сбой транспорта», #2600 п.4). Итог на проде (прогон 5287): ban_kinds сменился с platform на unknown, и при шести «статический отказ площадки» подряд в логах сайдкара в scrape_proxy_source_bans не появилось НИ ОДНОЙ записи. Это не косметика счётчиков — platform единственный диагноз, запускающий ротацию IP, поэтому мы продолжали бы долбиться в отказавший узел вместо перехода на свободный. Правка возвращает отказ на ban-путь, сохраняя разделение, ради которого #3237 и делался: - сайдкар кладёт в тело ошибки структурный признак ban_page и апстрим-статус. HTTP-код НЕ меняем: на 500 завязана classify_browser_probe; - SidecarBanPageError — подкласс httpx.HTTPStatusError, поэтому ловля у прочих поставщиков и retry-политика fetch() не замечают нового типа; - detail.py различает две ветки: подтверждённый отказ → report_ban + статус из исключения, транспортный сбой — как раньше. Статус несём отдельным полем, а не через last_response_status: на error-пути fetch() его обнуляет, а у Домклика отказ приходит с 401, без которого классификатор ставит unknown. Подстрокой в тексте исключения признак искать нельзя — _raise_for_sidecar_status обрезает тело до 300 символов, и формулировка отказа менялась дважды за месяц. Тесты держат обе ветки раздельно на всех трёх уровнях: сайдкар (признак есть у бан-страницы, отсутствует у транспортной ошибки), фетчер (тип и upstream_status, включая ловушку bool-как-int из #3196), detail.py (report_ban зовётся / не зовётся, статус доезжает). Closes #3239 --- .../tests/scrapers/test_domclick_detail.py | 57 ++++++++++++++ .../tests/test_kit_browser_fetcher_status.py | 78 ++++++++++++++++++- tradein-mvp/browser/server.py | 12 ++- .../browser/test_server_http_status.py | 59 ++++++++++++++ .../src/scraper_kit/browser_fetcher.py | 56 +++++++++++++ .../scraper_kit/providers/domclick/detail.py | 15 ++++ 6 files changed, 275 insertions(+), 2 deletions(-) diff --git a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py index 3b46b334..ddeed11b 100644 --- a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py +++ b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py @@ -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) ───────────── diff --git a/tradein-mvp/backend/tests/test_kit_browser_fetcher_status.py b/tradein-mvp/backend/tests/test_kit_browser_fetcher_status.py index d24b70cb..506a9dc8 100644 --- a/tradein-mvp/backend/tests/test_kit_browser_fetcher_status.py +++ b/tradein-mvp/backend/tests/test_kit_browser_fetcher_status.py @@ -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": "", "status": 200}, + request=httpx.Request("POST", "http://tradein-browser:3000/fetch"), + ) + _raise_for_sidecar_status(resp) diff --git a/tradein-mvp/browser/server.py b/tradein-mvp/browser/server.py index a7e55c98..48c3d37c 100644 --- a/tradein-mvp/browser/server.py +++ b/tradein-mvp/browser/server.py @@ -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). diff --git a/tradein-mvp/browser/test_server_http_status.py b/tradein-mvp/browser/test_server_http_status.py index 36fb7428..958331ee 100644 --- a/tradein-mvp/browser/test_server_http_status.py +++ b/tradein-mvp/browser/test_server_http_status.py @@ -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 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 e1b00841..f05a1a21 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 @@ -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": }``. Сайдкар + старой сборки ключей не отдаёт → (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( 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 ef935261..08a5cf07 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 @@ -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: это не подтверждённый маркер-бан площадки, а -- 2.45.3