diff --git a/tradein-mvp/browser/server.py b/tradein-mvp/browser/server.py index 834ca272..eda91f0b 100644 --- a/tradein-mvp/browser/server.py +++ b/tradein-mvp/browser/server.py @@ -249,7 +249,15 @@ async def _safe_relaunch(reason: str) -> None: случится позже, когда параллельная нагрузка спадёт. Crash-recovery при этом не теряется: per-page краш изолирован (своя страница), а реально мёртвый браузер поднимет _ensure_browser на следующем запросе. + + Сброс ``_page_counter = 0`` происходит АТОМАРНО с реальным relaunch'ем (под + launch-локом, только когда guard inflight<=1 пройден). Иначе под конкуренцией + N>1 несколько корутин видят counter >= порога, все откладывают relaunch (guard + inflight>1), но counter НИКОГДА не сбрасывается → recycle-check срабатывает на + каждом fetch вечно, а реальный recycle голодает. При отложенном relaunch'е + counter НЕ трогаем — следующий запрос (когда нагрузка спадёт) дожмёт recycle. """ + global _page_counter assert _launch_lock is not None, "_launch_lock not initialised" async with _launch_lock: if _inflight > 1: @@ -260,6 +268,7 @@ async def _safe_relaunch(reason: str) -> None: ) return logger.info("tradein-browser: relaunch (%s)", reason) + _page_counter = 0 await _relaunch_browser() @@ -585,81 +594,85 @@ async def _login_once(params: dict) -> list[dict]: success_cookie: str = params.get("success_cookie", "") wait_ms: int = params.get("wait_ms") or BROWSER_WAIT_MS + # Страница закрывается в finally: asyncio.CancelledError (BaseException, НЕ + # Exception) обходит except-ветку ниже — без finally страница утекала бы при + # отмене таска (например, на shutdown). _fetch_once делает то же. page = await browser.new_page() # type: ignore[attr-defined] try: - await page.goto( # type: ignore[attr-defined] - url, timeout=BROWSER_NAV_TIMEOUT_MS, wait_until="domcontentloaded" - ) - await page.wait_for_timeout(wait_ms) # type: ignore[attr-defined] + try: + await page.goto( # type: ignore[attr-defined] + url, timeout=BROWSER_NAV_TIMEOUT_MS, wait_until="domcontentloaded" + ) + await page.wait_for_timeout(wait_ms) # type: ignore[attr-defined] - # Pre-click sequence: каждый клик non-fatal (AnotherAccountBtn на fresh - # headless-сессии отсутствует — молча пропускается). - for sel in pre_click_selectors: - try: - await page.click(sel, timeout=12000) # type: ignore[attr-defined] - await page.wait_for_timeout(1200) # type: ignore[attr-defined] - except Exception: - logger.info("login: pre-click skipped: %s", sel) + # Pre-click sequence: каждый клик non-fatal (AnotherAccountBtn на fresh + # headless-сессии отсутствует — молча пропускается). + for sel in pre_click_selectors: + try: + await page.click(sel, timeout=12000) # type: ignore[attr-defined] + await page.wait_for_timeout(1200) # type: ignore[attr-defined] + except Exception: + logger.info("login: pre-click skipped: %s", sel) - await page.fill(email_selector, email, timeout=15000) # type: ignore[attr-defined] - await page.fill(password_selector, password, timeout=15000) # type: ignore[attr-defined] + await page.fill(email_selector, email, timeout=15000) # type: ignore[attr-defined] + await page.fill(password_selector, password, timeout=15000) # type: ignore[attr-defined] - # 2-шаговый submit: Cian после первого сабмита показывает экран - # «Введите пароль» с тем же password_selector — дозаполняем и повторяем. - cookie_found = False - for attempt in range(2): - await page.click(submit_selector, timeout=15000) # type: ignore[attr-defined] - if success_cookie: - cookie_found = await _wait_cookie( - page, success_cookie, tries=20, interval_ms=500 - ) - if cookie_found: + # 2-шаговый submit: Cian после первого сабмита показывает экран + # «Введите пароль» с тем же password_selector — дозаполняем и повторяем. + cookie_found = False + for attempt in range(2): + await page.click(submit_selector, timeout=15000) # type: ignore[attr-defined] + if success_cookie: + cookie_found = await _wait_cookie( + page, success_cookie, tries=20, interval_ms=500 + ) + if cookie_found: + break + # 2-step: если поле пароля ещё видимо — дозаполнить и повторить + still_visible = False + try: + still_visible = await page.is_visible(password_selector) # type: ignore[attr-defined] + except Exception: + pass + if attempt == 0 and still_visible: + await page.fill(password_selector, password, timeout=15000) # type: ignore[attr-defined] + await page.wait_for_timeout(800) # type: ignore[attr-defined] + continue break - # 2-step: если поле пароля ещё видимо — дозаполнить и повторить - still_visible = False + + logger.info( + "tradein-browser: login success_cookie=%r present=%s", + success_cookie, + cookie_found, + ) + + cookies: list[dict] = await page.context.cookies() # type: ignore[attr-defined] + logger.info( + "tradein-browser: login OK url=%r email_selector=%r cookie_count=%d", + url, + email_selector, + len(cookies), + ) + except Exception as exc: + screenshot_b64 = "" try: - still_visible = await page.is_visible(password_selector) # type: ignore[attr-defined] + screenshot_bytes: bytes = await page.screenshot() # type: ignore[attr-defined] + screenshot_b64 = base64.b64encode(screenshot_bytes).decode() except Exception: pass - if attempt == 0 and still_visible: - await page.fill(password_selector, password, timeout=15000) # type: ignore[attr-defined] - await page.wait_for_timeout(800) # type: ignore[attr-defined] - continue - break - - logger.info( - "tradein-browser: login success_cookie=%r present=%s", - success_cookie, - cookie_found, - ) - - cookies: list[dict] = await page.context.cookies() # type: ignore[attr-defined] - logger.info( - "tradein-browser: login OK url=%r email_selector=%r cookie_count=%d", - url, - email_selector, - len(cookies), - ) - except Exception as exc: - screenshot_b64 = "" - try: - screenshot_bytes: bytes = await page.screenshot() # type: ignore[attr-defined] - screenshot_b64 = base64.b64encode(screenshot_bytes).decode() - except Exception: - pass - page_url = "" - try: - page_url = page.url # type: ignore[attr-defined] - except Exception: - pass + page_url = "" + try: + page_url = page.url # type: ignore[attr-defined] + except Exception: + pass + raise LoginError({ + "error": f"{type(exc).__name__}: {exc}", + "page_url": page_url, + "screenshot_b64": screenshot_b64, + }) from exc + finally: await page.close() # type: ignore[attr-defined] - raise LoginError({ - "error": f"{type(exc).__name__}: {exc}", - "page_url": page_url, - "screenshot_b64": screenshot_b64, - }) from exc - await page.close() # type: ignore[attr-defined] _page_counter += 1 if _page_counter >= BROWSER_RECYCLE_PAGES: logger.info( diff --git a/tradein-mvp/browser/test_server_smoke.py b/tradein-mvp/browser/test_server_smoke.py index 6addea83..616d5149 100644 --- a/tradein-mvp/browser/test_server_smoke.py +++ b/tradein-mvp/browser/test_server_smoke.py @@ -286,6 +286,120 @@ def test_ensure_browser_serialized_by_launch_lock( assert calls["n"] == 1, "launch должен произойти ровно один раз (double-check под локом)" +# ── _safe_relaunch resets _page_counter (recycle starvation fix) ───────────────── + + +def test_safe_relaunch_resets_page_counter_when_proceeds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_safe_relaunch сбрасывает _page_counter=0 когда relaunch проходит (inflight<=1). + + Регрессия: без сброса под конкуренцией counter копился вечно (>= порога на + каждом fetch) → recycle голодал. Сброс атомарен с relaunch'ем под локом. + """ + monkeypatch.setattr(server, "_launch_lock", asyncio.Lock()) + monkeypatch.setattr(server, "_inflight", 1) # мы единственный in-flight + monkeypatch.setattr(server, "_page_counter", 999) + + relaunched = {"n": 0} + + async def _fake_relaunch() -> None: + relaunched["n"] += 1 + + monkeypatch.setattr(server, "_relaunch_browser", _fake_relaunch) + + asyncio.run(server._safe_relaunch("recycle")) + + assert relaunched["n"] == 1, "relaunch должен выполниться при inflight<=1" + assert server._page_counter == 0, "counter должен сброситься при реальном relaunch" + + +def test_safe_relaunch_keeps_page_counter_when_deferred( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_safe_relaunch НЕ трогает _page_counter когда relaunch отложен (inflight>1). + + Отложенный relaunch оставляет counter, чтобы следующий запрос (когда нагрузка + спадёт) дожал recycle — иначе recycle потерялся бы навсегда. + """ + monkeypatch.setattr(server, "_launch_lock", asyncio.Lock()) + monkeypatch.setattr(server, "_inflight", 3) # есть параллельные запросы + monkeypatch.setattr(server, "_page_counter", 42) + + relaunched = {"n": 0} + + async def _fake_relaunch() -> None: + relaunched["n"] += 1 + + monkeypatch.setattr(server, "_relaunch_browser", _fake_relaunch) + + asyncio.run(server._safe_relaunch("recycle")) + + assert relaunched["n"] == 0, "relaunch должен быть отложен при inflight>1" + assert server._page_counter == 42, "counter НЕ трогаем при отложенном relaunch" + + +# ── _login_once closes page on CancelledError (BaseException leak fix) ──────────── + + +class _CancellingLoginPage: + """Page для /login, которая бросает CancelledError внутри логина.""" + + def __init__(self, tracker: "_Tracker") -> None: + self._tracker = tracker + self.url = "https://cian.example/login" + + async def goto(self, url: str, **kwargs: Any) -> None: + return None + + async def wait_for_timeout(self, ms: int) -> None: + return None + + async def fill(self, selector: str, value: str, **kwargs: Any) -> None: + # Отмена таска (BaseException, не Exception) — обходит except Exception. + raise asyncio.CancelledError() + + async def close(self) -> None: + self._tracker.closed += 1 + + +class _CancellingLoginBrowser: + def __init__(self, tracker: "_Tracker") -> None: + self._tracker = tracker + + async def new_page(self) -> _CancellingLoginPage: + self._tracker.opened += 1 + return _CancellingLoginPage(self._tracker) + + +def test_login_closes_page_on_cancelled_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_login_once закрывает страницу даже при CancelledError (finally, не except). + + CancelledError — BaseException, обходит except Exception, без finally страница + утекала бы при отмене таска (shutdown / dropped client). + """ + tracker = _Tracker() + browser = _CancellingLoginBrowser(tracker) + monkeypatch.setattr(server, "_browser", browser) + + params = { + "url": "https://cian.example/login", + "email": "u@example.com", + "password": "secret", + "email_selector": "#email", + "password_selector": "#password", + "submit_selector": "#submit", + } + + with pytest.raises(asyncio.CancelledError): + asyncio.run(server._login_once(params)) + + assert tracker.opened == 1 + assert tracker.closed == 1, "страница должна закрыться в finally при CancelledError" + + async def _coro(value: Any) -> Any: """Хелпер: оборачивает значение в awaitable для подмены request.json().""" return value