fix(ptica): download_binary переживает транзиентный ответ, как и get_json (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 3m0s
CI / backend-tests (pull_request) Successful in 17m31s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 3m0s
CI / backend-tests (pull_request) Successful in 17m31s
Обе функции ходят через один браузерный контекст, под один и тот же WAF.
`get_json` держит до пяти попыток с экспоненциальным backoff на 429/5xx/0,
а `download_binary` не имел ретраев вовсе: один 429 ронял загрузку
картинки насовсем, и вызывающий (`download_plan_image`, `download_photos`)
писал в лог «не удалось» — неотличимо от «файла нет».
Правильный образец лежал в этом же классе, двадцатью строками выше.
Непереходные коды (403, 404) поднимаются сразу, без ожидания: повтор их
не изменит, а лишний стук под WAF вредит. Разбор статуса вынесен ЗА
семафор — sleep не должен держать слот.
Двусторонне: против origin/main транзиентные тесты красные с конкретным
значением («вместо байтов получили RuntimeError('binary http 429…');
попыток=1»), ни одного ImportError/TypeError.
Контроли зелёные с обеих сторон: 403 и 404 не повторяются, исчерпание
попыток даёт честную ошибку, а не пустые байты, успех с первой попытки не
порождает лишних запросов.
Отдельный контроль на паузы: без него «ретраит» и «долбит без пауз»
неотличимы в тесте, а под WAF разница между ними решающая — проверяется,
что задержки растут как 1, 2 секунды.
pytest backend/tests/services/scrapers/ — 340 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
b975451b53
commit
f72a08eb80
2 changed files with 196 additions and 10 deletions
|
|
@ -288,17 +288,47 @@ class BrowserSession:
|
|||
|
||||
Uses Playwright APIRequest which goes through the browser context — same
|
||||
cookies, same TLS fingerprint as the page itself.
|
||||
|
||||
Ретраи с backoff на транзиентных ответах (429 / 5xx / 0) — так же, как в
|
||||
get_json выше (#2464). Раньше их не было: один 429 под тем же WAF, под
|
||||
которым get_json переживает до пяти попыток, ронял загрузку картинки
|
||||
насовсем, и вызывающий (download_plan_image, download_photos) записывал
|
||||
это в лог как «не удалось» — неотличимо от «файла нет».
|
||||
|
||||
Непереходные коды (403, 404) поднимаются сразу, без ожидания: повтор их
|
||||
не изменит, а под WAF лишний стук вредит.
|
||||
"""
|
||||
if self._context is None:
|
||||
raise RuntimeError("BrowserSession not bootstrapped")
|
||||
async with self._sem:
|
||||
await jitter_sleep(200, 500) # Lighter throttle for static assets.
|
||||
self._request_count += 1
|
||||
resp = await self._context.request.get(
|
||||
url,
|
||||
headers={"Authorization": self.auth} if self.auth else {},
|
||||
)
|
||||
if resp.status != 200:
|
||||
last_err: Exception | None = None
|
||||
for attempt in range(5):
|
||||
async with self._sem:
|
||||
await jitter_sleep(200, 500) # Lighter throttle for static assets.
|
||||
self._request_count += 1
|
||||
try:
|
||||
resp = await self._context.request.get(
|
||||
url,
|
||||
headers={"Authorization": self.auth} if self.auth else {},
|
||||
)
|
||||
except Exception as e:
|
||||
last_err = e
|
||||
logger.warning("download_binary err attempt=%d url=%s: %r", attempt, url, e)
|
||||
await asyncio.sleep(2**attempt)
|
||||
continue
|
||||
status = resp.status
|
||||
if status == 200:
|
||||
return await resp.body()
|
||||
body = await resp.text()
|
||||
raise RuntimeError(f"binary http {resp.status}: {body[:200]}")
|
||||
return await resp.body()
|
||||
# Разбор статуса — ВНЕ семафора: sleep не должен держать слот.
|
||||
if status in (429,) or status >= 500:
|
||||
last_err = RuntimeError(f"binary transient status={status}")
|
||||
logger.warning(
|
||||
"download_binary transient status=%d attempt=%d url=%s, backing off",
|
||||
status,
|
||||
attempt,
|
||||
url,
|
||||
)
|
||||
await asyncio.sleep(2**attempt)
|
||||
continue
|
||||
raise RuntimeError(f"binary http {status}: {body[:200]}")
|
||||
raise RuntimeError(f"binary max retries exhausted: {last_err!r}")
|
||||
|
|
|
|||
|
|
@ -0,0 +1,156 @@
|
|||
"""download_binary переживает транзиентный ответ, как и get_json (#2464).
|
||||
|
||||
Обе функции ходят через один и тот же браузерный контекст, под один и тот же WAF.
|
||||
`get_json` держит до пяти попыток с экспоненциальным backoff на 429/5xx/0, а
|
||||
`download_binary` не имел ретраев вовсе: один 429 ронял загрузку картинки
|
||||
насовсем, и вызывающий (`download_plan_image`, `download_photos`) писал в лог
|
||||
«не удалось» — неотличимо от «файла нет».
|
||||
|
||||
Правильный образец лежал в этом же классе, двадцатью строками выше.
|
||||
|
||||
Тесты гоняют настоящий `download_binary` на двойнике Playwright-контекста;
|
||||
`asyncio.sleep` подменён, чтобы backoff не тратил время теста, — но факт ожидания
|
||||
проверяется отдельно, иначе «ретраит» и «долбит без пауз» неотличимы.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from app.services.scrapers.stealth import BrowserSession
|
||||
|
||||
|
||||
class _Resp:
|
||||
def __init__(self, status: int, body: bytes = b"", text: str = "") -> None:
|
||||
self.status = status
|
||||
self._body = body
|
||||
self._text = text
|
||||
|
||||
async def body(self) -> bytes:
|
||||
return self._body
|
||||
|
||||
async def text(self) -> str:
|
||||
return self._text
|
||||
|
||||
|
||||
class _Request:
|
||||
"""Отдаёт заранее заданную очередь ответов и считает вызовы."""
|
||||
|
||||
def __init__(self, ответы: list[Any]) -> None:
|
||||
self._ответы = list(ответы)
|
||||
self.calls = 0
|
||||
|
||||
async def get(self, url: str, headers: dict | None = None) -> _Resp:
|
||||
self.calls += 1
|
||||
item = self._ответы.pop(0) if self._ответы else _Resp(200, b"ok")
|
||||
if isinstance(item, Exception):
|
||||
raise item
|
||||
return item
|
||||
|
||||
|
||||
class _Context:
|
||||
def __init__(self, ответы: list[Any]) -> None:
|
||||
self.request = _Request(ответы)
|
||||
|
||||
|
||||
def _session(ответы: list[Any]) -> tuple[BrowserSession, _Context]:
|
||||
s = BrowserSession.__new__(BrowserSession)
|
||||
ctx = _Context(ответы)
|
||||
s._context = ctx # type: ignore[attr-defined]
|
||||
s._sem = asyncio.Semaphore(1) # type: ignore[attr-defined]
|
||||
s._request_count = 0 # type: ignore[attr-defined]
|
||||
s.auth = None # type: ignore[attr-defined]
|
||||
return s, ctx
|
||||
|
||||
|
||||
def _run(ответы: list[Any]) -> tuple[Any, _Context, list[float]]:
|
||||
"""Прогнать download_binary, вернуть (результат|исключение, контекст, паузы)."""
|
||||
паузы: list[float] = []
|
||||
|
||||
async def _fake_sleep(d: float) -> None:
|
||||
паузы.append(d)
|
||||
|
||||
s, ctx = _session(ответы)
|
||||
with (
|
||||
patch("app.services.scrapers.stealth.asyncio.sleep", _fake_sleep),
|
||||
patch("app.services.scrapers.stealth.jitter_sleep", lambda *a, **k: _fake_sleep(0)),
|
||||
):
|
||||
try:
|
||||
res: Any = asyncio.run(s.download_binary("https://x/y.png"))
|
||||
except Exception as exc:
|
||||
res = exc
|
||||
return res, ctx, паузы
|
||||
|
||||
|
||||
def test_transient_429_is_retried_not_fatal() -> None:
|
||||
"""Головной: 429 с последующим успехом обязан дать байты, а не исключение.
|
||||
|
||||
На origin/main первая же попытка поднимает RuntimeError — картинка теряется.
|
||||
"""
|
||||
res, ctx, _ = _run([_Resp(429, text="rate limited"), _Resp(200, b"PNGDATA")])
|
||||
assert res == b"PNGDATA", f"вместо байтов получили {res!r}; попыток={ctx.request.calls}"
|
||||
assert ctx.request.calls == 2, f"повтор не выполнен: попыток={ctx.request.calls}"
|
||||
|
||||
|
||||
def test_transient_5xx_is_retried() -> None:
|
||||
"""503 — тоже транзиент, как и в get_json."""
|
||||
res, ctx, _ = _run([_Resp(503, text="bad gw"), _Resp(502, text="bad gw"), _Resp(200, b"OK")])
|
||||
assert res == b"OK", f"{res!r}"
|
||||
assert ctx.request.calls == 3
|
||||
|
||||
|
||||
def test_backoff_actually_waits() -> None:
|
||||
"""Контроль от «ретраит, но долбит без пауз»: паузы растут экспоненциально.
|
||||
|
||||
Без этой проверки цикл без sleep выглядел бы в тестах так же, как с ним, —
|
||||
а под WAF разница между ними решающая.
|
||||
"""
|
||||
_, _, паузы = _run([_Resp(429), _Resp(429), _Resp(200, b"OK")])
|
||||
задержки = [p for p in паузы if p > 0]
|
||||
assert задержки[:2] == [1, 2], f"backoff не экспоненциальный: {задержки}"
|
||||
|
||||
|
||||
def test_permanent_403_is_not_retried() -> None:
|
||||
"""Контроль от переусердствования: 403 поднимается сразу.
|
||||
|
||||
Повтор его не изменит, а лишний стук под WAF вредит.
|
||||
"""
|
||||
res, ctx, _ = _run([_Resp(403, text="forbidden"), _Resp(200, b"OK")])
|
||||
assert isinstance(res, RuntimeError), f"403 не поднял ошибку: {res!r}"
|
||||
assert "403" in str(res)
|
||||
assert ctx.request.calls == 1, f"403 был повторён: попыток={ctx.request.calls}"
|
||||
|
||||
|
||||
def test_404_is_not_retried() -> None:
|
||||
"""Контроль: отсутствующий файл — не транзиент."""
|
||||
res, ctx, _ = _run([_Resp(404, text="no such file")])
|
||||
assert isinstance(res, RuntimeError) and "404" in str(res)
|
||||
assert ctx.request.calls == 1
|
||||
|
||||
|
||||
def test_exhausted_retries_raise_with_the_last_error() -> None:
|
||||
"""Контроль честности отказа: после пяти попыток — ошибка, а не пустые байты."""
|
||||
res, ctx, _ = _run([_Resp(429) for _ in range(5)])
|
||||
assert isinstance(res, RuntimeError), f"{res!r}"
|
||||
assert "exhausted" in str(res), str(res)
|
||||
assert ctx.request.calls == 5, f"попыток={ctx.request.calls}"
|
||||
|
||||
|
||||
def test_network_exception_is_retried_too() -> None:
|
||||
"""Обрыв соединения — тоже транзиент (зеркалит ветку except в get_json)."""
|
||||
res, ctx, _ = _run([ConnectionError("boom"), _Resp(200, b"OK")])
|
||||
assert res == b"OK", f"{res!r}"
|
||||
assert ctx.request.calls == 2
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", [200])
|
||||
def test_success_on_first_try_makes_one_request(status: int) -> None:
|
||||
"""Контроль: успех с первой попытки не порождает лишних запросов."""
|
||||
res, ctx, паузы = _run([_Resp(status, b"OK")])
|
||||
assert res == b"OK"
|
||||
assert ctx.request.calls == 1
|
||||
assert [p for p in паузы if p > 0] == [], f"лишние паузы: {паузы}"
|
||||
Loading…
Add table
Reference in a new issue