All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 10s
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 / browser-tests (pull_request) Successful in 1m17s
CI Trade-In / backend-tests (pull_request) Successful in 4m54s
#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
236 lines
10 KiB
Python
236 lines
10 KiB
Python
"""HTTP-статус сайдкара наверх: `BrowserFetcher.last_response_status` (#3196).
|
||
|
||
Сайдкар (tradein-mvp/browser/server.py) теперь кладёт в тело /fetch HTTP-код целевой
|
||
навигации рядом с html: ``{"html": ..., "status": <int|null>}``. Kit выносит его на
|
||
инстанс фетчера — АТРИБУТОМ, а не возвратом ``fetch()``: поток управления менять
|
||
нельзя, ``fetch()`` по-прежнему отдаёт ``str`` и по-прежнему не бросает там, где не
|
||
бросал раньше.
|
||
|
||
Зачем: ДомКлик отдаёт статическую страницу «403 | Домклик» на 26 624 байта, где нет
|
||
ни startpow, ни qrator, ни капчи — ни один текстовый маркер сайдкара (все сняты с
|
||
Авито) на неё не срабатывает, и отказ уезжал наверх как валидный контент. 14 прогонов
|
||
domclick_detail_backfill подряд получили ban_kind=unknown ровно поэтому.
|
||
|
||
Инварианты:
|
||
- status из тела → last_response_status (int) на КАЖДЫЙ успешный fetch;
|
||
- ключа "status" нет (сайдкар старой версии) ИЛИ он null → None, БЕЗ исключения;
|
||
- status нечислового типа → None (мусор в теле не должен ронять фетч);
|
||
- fetch упал → last_response_status сброшен в None (не отдаём статус прошлого);
|
||
- ban_kind_from_status раскладывает код в значение, допустимое CHECK-ограничением
|
||
scrape_runs.ban_kind ("platform" | "infra" | "unknown" | NULL).
|
||
|
||
httpx полностью замокан (зеркалит test_kit_browser_fetcher_proxy_pool.py).
|
||
"""
|
||
|
||
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,
|
||
SidecarBanPageError,
|
||
_raise_for_sidecar_status,
|
||
ban_kind_from_status,
|
||
)
|
||
|
||
|
||
def _mock_client(json_payload: dict[str, Any], *, raise_exc: Exception | None = None) -> MagicMock:
|
||
"""httpx.AsyncClient-заглушка: .post → resp c raise_for_status/json."""
|
||
resp = MagicMock()
|
||
if raise_exc is not None:
|
||
resp.raise_for_status.side_effect = raise_exc
|
||
else:
|
||
resp.raise_for_status.return_value = None
|
||
resp.json.return_value = json_payload
|
||
client = MagicMock()
|
||
client.post = AsyncMock(return_value=resp)
|
||
client.aclose = AsyncMock(return_value=None)
|
||
return client
|
||
|
||
|
||
async def _fetcher(client: MagicMock, **kwargs: Any) -> BrowserFetcher:
|
||
"""Реально входит в `__aenter__`, потом подменяет httpx-клиент."""
|
||
bf = BrowserFetcher(endpoint="http://browser:3000", **kwargs)
|
||
await bf.__aenter__()
|
||
bf._client = client
|
||
return bf
|
||
|
||
|
||
# ── last_response_status ──────────────────────────────────────────────────────
|
||
|
||
|
||
async def test_status_starts_as_none() -> None:
|
||
"""До первого fetch статуса нет — атрибут существует и равен None."""
|
||
client = _mock_client({"html": "<ok>", "status": 200})
|
||
bf = await _fetcher(client, source="domclick")
|
||
|
||
assert bf.last_response_status is None
|
||
|
||
|
||
async def test_status_from_body_is_exposed() -> None:
|
||
client = _mock_client({"html": "<403 page>", "status": 403})
|
||
bf = await _fetcher(client, source="domclick")
|
||
|
||
html = await bf.fetch("https://domclick.ru/card/1")
|
||
|
||
assert html == "<403 page>" # поток управления не изменился — fetch отдаёт str
|
||
assert bf.last_response_status == 403
|
||
|
||
|
||
async def test_status_updated_on_every_fetch() -> None:
|
||
"""Атрибут обновляется КАЖДЫМ _post_fetch, а не только первым."""
|
||
client = _mock_client({"html": "<ok>", "status": 200})
|
||
bf = await _fetcher(client, source="domclick")
|
||
|
||
await bf.fetch("https://domclick.ru/1")
|
||
assert bf.last_response_status == 200
|
||
|
||
client.post.return_value.json.return_value = {"html": "<403>", "status": 403}
|
||
await bf.fetch("https://domclick.ru/2")
|
||
assert bf.last_response_status == 403
|
||
|
||
|
||
async def test_missing_status_key_is_none_and_does_not_raise() -> None:
|
||
"""Сайдкар старой версии (тело без "status") — фетч проходит, статуса просто нет."""
|
||
client = _mock_client({"html": "<ok>"})
|
||
bf = await _fetcher(client, source="avito")
|
||
|
||
html = await bf.fetch("https://avito.ru/x")
|
||
|
||
assert html == "<ok>"
|
||
assert bf.last_response_status is None
|
||
|
||
|
||
async def test_null_status_is_none() -> None:
|
||
"""goto вернул None (редирект/навигационная гонка) → сайдкар шлёт status=null."""
|
||
client = _mock_client({"html": "<ok>", "status": None})
|
||
bf = await _fetcher(client, source="avito")
|
||
|
||
await bf.fetch("https://avito.ru/x")
|
||
|
||
assert bf.last_response_status is None
|
||
|
||
|
||
async def test_non_int_status_is_ignored() -> None:
|
||
"""Мусор в поле status не должен ронять фетч — читается как «статуса нет»."""
|
||
client = _mock_client({"html": "<ok>", "status": "403"})
|
||
bf = await _fetcher(client, source="avito")
|
||
|
||
await bf.fetch("https://avito.ru/x")
|
||
|
||
assert bf.last_response_status is None
|
||
|
||
|
||
async def test_status_reset_on_failed_fetch() -> None:
|
||
"""Фетч упал — не отдаём статус ПРОШЛОГО запроса."""
|
||
client = _mock_client({"html": "<ok>", "status": 200})
|
||
bf = await _fetcher(client, source="avito")
|
||
await bf.fetch("https://avito.ru/1")
|
||
assert bf.last_response_status == 200
|
||
|
||
client.post.side_effect = RuntimeError("transport down")
|
||
with pytest.raises(RuntimeError):
|
||
await bf.fetch("https://avito.ru/2")
|
||
|
||
assert bf.last_response_status is None
|
||
|
||
|
||
# ── ban_kind_from_status ──────────────────────────────────────────────────────
|
||
|
||
|
||
@pytest.mark.parametrize(
|
||
("status", "expected"),
|
||
[
|
||
(403, "platform"),
|
||
(429, "platform"),
|
||
(500, "infra"),
|
||
(502, "infra"),
|
||
(599, "infra"),
|
||
(200, None),
|
||
(301, None),
|
||
(404, None),
|
||
(None, None),
|
||
],
|
||
)
|
||
def test_ban_kind_from_status(status: int | None, expected: str | None) -> None:
|
||
assert ban_kind_from_status(status) == expected
|
||
|
||
|
||
def test_ban_kind_values_fit_scrape_runs_check() -> None:
|
||
"""Возврат обязан быть пригоден для scrape_runs.ban_kind как есть."""
|
||
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)
|