From f72a08eb801c5f048e6566c5e7ca3fd51591d5fd Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 20 Aug 2026 22:57:14 +0500 Subject: [PATCH] =?UTF-8?q?fix(ptica):=20download=5Fbinary=20=D0=BF=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=B6=D0=B8=D0=B2=D0=B0=D0=B5=D1=82=20=D1=82=D1=80?= =?UTF-8?q?=D0=B0=D0=BD=D0=B7=D0=B8=D0=B5=D0=BD=D1=82=D0=BD=D1=8B=D0=B9=20?= =?UTF-8?q?=D0=BE=D1=82=D0=B2=D0=B5=D1=82,=20=D0=BA=D0=B0=D0=BA=20=D0=B8?= =?UTF-8?q?=20get=5Fjson=20(#2464)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Обе функции ходят через один браузерный контекст, под один и тот же 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 --- backend/app/services/scrapers/stealth.py | 50 ++++-- .../test_2464_download_binary_retry.py | 156 ++++++++++++++++++ 2 files changed, 196 insertions(+), 10 deletions(-) create mode 100644 backend/tests/services/scrapers/test_2464_download_binary_retry.py diff --git a/backend/app/services/scrapers/stealth.py b/backend/app/services/scrapers/stealth.py index 6abf2b13..e8fa949f 100644 --- a/backend/app/services/scrapers/stealth.py +++ b/backend/app/services/scrapers/stealth.py @@ -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}") diff --git a/backend/tests/services/scrapers/test_2464_download_binary_retry.py b/backend/tests/services/scrapers/test_2464_download_binary_retry.py new file mode 100644 index 00000000..43a442ed --- /dev/null +++ b/backend/tests/services/scrapers/test_2464_download_binary_retry.py @@ -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"лишние паузы: {паузы}"