From ec7838b7a98196a205adaf6332aeb2134f3ab083 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 6 Sep 2026 11:58:42 +0500 Subject: [PATCH] =?UTF-8?q?fix(#3398):=20=D0=BF=D1=83=D1=81=D1=82=D0=BE?= =?UTF-8?q?=D0=B9=20=D0=BF=D1=83=D0=BB=20=D0=B2=20=D1=84=D0=BE=D0=BD=D0=BE?= =?UTF-8?q?=D0=B2=D0=BE=D0=B9=20=D0=B4=D0=BE=D0=B3=D1=80=D1=83=D0=B7=D0=BA?= =?UTF-8?q?=D0=B5=20=E2=80=94=20WARNING=20=D0=B1=D0=B5=D0=B7=20=D1=82?= =?UTF-8?q?=D1=80=D0=B5=D0=B9=D1=81=D0=B1=D0=B5=D0=BA=D0=B0=20(=D0=BE?= =?UTF-8?q?=D0=B1=D1=89=D0=B0=D1=8F=20=D1=84=D1=83=D0=BD=D0=BA=D1=86=D0=B8?= =?UTF-8?q?=D1=8F);=20=D0=BA=D0=BE=D0=BC=D0=BC=D0=B5=D0=BD=D1=82=D0=B0?= =?UTF-8?q?=D1=80=D0=B8=D0=B9=20=D0=BF=D1=80=D0=BE=20stateless?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit На проде `ESTIMATE_EXTERNAL_SOURCES_BACKGROUND=true` (docker-compose.prod.yml:296), поэтому синхронный cian-вызов идёт с `fetch_on_miss=False` и возвращает None ДО прокси-слоя (`providers/cian/valuation.py:171`) — добавленная в этой ветке ветка WARNING в `estimate_quality` на проде почти не звучит. Настоящий фетч уходит в `_defer_external_refresh`, где `NoProxyAvailableError` попадал в общий `except Exception: logger.exception(...)` → ERROR + traceback → событие в GlitchTip на каждый /estimate по новому адресу: ровно тот шум, который PR и убирает. Правка в ОБЩЕЙ функции отложенной догрузки, а не в cian-ветке: через неё идут все источники фонового режима (yandex тоже — у него swallow живёт внутри `_get_or_fetch_yandex_valuation_cached`, дыры нет, но следующий источник получит поведение бесплатно). Для прочих исключений всё как было: `logger.exception`. Тест по значению: background=True + пустой пул в production → фоновая догрузка cian логирует WARNING «пул прокси пуст», записей ERROR/traceback у логгера эстиматора нет. Задача дожидается внутри того же loop'а и не снимая патчей (`_DEFERRED_REFRESH_TASKS` + `asyncio.gather`) — иначе `anyio.run` закрывает loop раньше старта задачи и тест был бы зелёным по построению. На HEAD ветки тест красный: ERROR app.services.estimator:estimator.py:910 deferred cian_valuation: догрузка не удалась (кэш не прогрет) + Traceback … NoProxyAvailableError. Комментарий у `_c_kwargs`: весь dict переиспользуется замыканием фоновой задачи, то есть `config`/`proxy_provider` — один инстанс на два возможно-одновременных вызова. Корректно ровно пока оба stateless (`RealScraperConfig` — read-only снимок настроек, `RealProxyProvider` без полей, короткая сессия БД на операцию); появится per-вызов состояние — фоновой задаче нужен свой инстанс. --- tradein-mvp/backend/app/services/estimator.py | 30 +++++++-- ...st_3398_estimator_valuations_proxy_pool.py | 63 ++++++++++++++++++- 2 files changed, 85 insertions(+), 8 deletions(-) diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index 73557971..0a2da10a 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -906,8 +906,18 @@ def _defer_external_refresh(label: str, work: Callable[[Session], Awaitable[obje db = SessionLocal() try: await work(db) - except Exception: - logger.exception("deferred %s: догрузка не удалась (кэш не прогрет)", label) + except Exception as exc: + if caused_by_no_proxy(exc): + # #3398: пустой пул — НАША инфраструктура, HTTP-запрос не уходил вовсе + # (NoProxyAvailableError поднимается в curl_proxy_url ДО запроса). WARNING, + # не ERROR: GlitchTip слушает event_level=ERROR, а это штатная деградация + # прогрева, не сбой площадки — событие тут было бы шумом. + # На проде ESTIMATE_EXTERNAL_SOURCES_BACKGROUND=true, поэтому настоящий + # фетч уходит именно сюда: synchronous-ветка с fetch_on_miss=False отдаёт + # None ДО прокси-слоя и до своего WARNING в estimate_quality не доходит. + logger.warning("deferred %s: пул прокси пуст — кэш не прогрет: %s", label, exc) + else: + logger.exception("deferred %s: догрузка не удалась (кэш не прогрет)", label) finally: db.close() @@ -4647,10 +4657,18 @@ async def estimate_quality( "house_id": target_house_id, # #3398: без provider'а curl_proxy_url считает use_pool=False (флаг AND # provider is not None) → env-прокси CIAN_PROXY_URL/SCRAPER_PROXY_URL, мёртвый - # узел (#2613) → «Cian valuation fetch failed: … 407». Провайдер stateless - # (сессия на операцию), поэтому один инстанс на оба call site: основной вызов - # и отложенная фоновая догрузка ниже. Lease живёт внутри curl_proxy_url: - # acquire до запроса, release в finally. + # узел (#2613) → «Cian valuation fetch failed: … 407». Lease живёт внутри + # curl_proxy_url: acquire до запроса, release в finally. + # + # Весь этот dict переиспользуется отложенной фоновой догрузкой ниже + # (`_defer_external_refresh` захватывает `_c_kwargs` замыканием), то есть + # `config` и `proxy_provider` — ОДИН инстанс на два вызова, которые могут идти + # одновременно (фон стартует после ответа, но живёт своей задачей). Корректно + # это ровно пока оба stateless: `RealScraperConfig` — read-only снимок настроек, + # `RealProxyProvider` не хранит полей вообще и открывает короткую сессию БД на + # каждую операцию (acquire/release/mark_health), поэтому lease'ы двух вызовов не + # пересекаются. Появится у любого из них per-вызов состояние (кэш lease'а, + # счётчик, открытая сессия) — фоновой задаче нужен СВОЙ инстанс, а не общий. "proxy_provider": RealProxyProvider(), } try: diff --git a/tradein-mvp/backend/tests/test_3398_estimator_valuations_proxy_pool.py b/tradein-mvp/backend/tests/test_3398_estimator_valuations_proxy_pool.py index 8743784d..6c019c49 100644 --- a/tradein-mvp/backend/tests/test_3398_estimator_valuations_proxy_pool.py +++ b/tradein-mvp/backend/tests/test_3398_estimator_valuations_proxy_pool.py @@ -11,12 +11,17 @@ curl уходил на env-прокси `SCRAPER_PROXY_URL` (выключенн (а) оба call site передают provider'а (на main здесь None); (б) пул пуст + environment=production → оценка отдаётся БЕЗ этих источников, без исключения, HTTP не уходит, а причина в логе честная — «пул прокси пуст»; + (б2) прод-режим `ESTIMATE_EXTERNAL_SOURCES_BACKGROUND=true`: синхронный вызов уходит + с `fetch_on_miss=False` и до прокси-слоя не доходит вовсе, настоящий фетч делает + `_defer_external_refresh` — там пустой пул тоже WARNING, а не ERROR с трейсбеком + (иначе GlitchTip получает событие на каждый /estimate по новому адресу); (в) lease освобождён ровно один раз (успех и ошибка фетча) — через настоящий `curl_proxy_url`, а не через мок провайдера. """ from __future__ import annotations +import asyncio import logging import os from contextlib import ExitStack @@ -170,11 +175,16 @@ def _listing(i: int) -> dict[str, Any]: } -def _run_estimate(*, extra_patches: list[Any]) -> Any: +def _run_estimate(*, extra_patches: list[Any], drain_deferred: bool = False) -> Any: """estimate_quality() со всеми внешними источниками кроме Cian заглушенными. Cian намеренно НЕ патчится списком по умолчанию: тесты (б)/(в) гоняют настоящую kit-функцию, чтобы прокси-слой (`curl_proxy_url`) реально отработал. + + drain_deferred: дождаться задач фоновой догрузки (`_DEFERRED_REFRESH_TASKS`) ВНУТРИ + того же loop'а и не снимая патчей. Без ожидания `anyio.run` закрывает loop сразу + после ответа, задача умирает не начавшись («Task was destroyed but it is pending») + и тест по фоновому пути был бы зелёным по построению. """ payload = TradeInEstimateInput(address=_ADDRESS, area_m2=45.0, rooms=2, floor=5, total_floors=9) @@ -203,7 +213,12 @@ def _run_estimate(*, extra_patches: list[Any]) -> Any: with ExitStack() as stack: # список патчей переменной длины — не `with (...)` for p in patches: stack.enter_context(p) - return await estimate_quality(payload, MagicMock()) + est = await estimate_quality(payload, MagicMock()) + if drain_deferred: + # gather с return_exceptions=False: задача сама гасит свои ошибки, а если + # перестанет — тест обязан покраснеть, а не проглотить. + await asyncio.gather(*list(estimator._DEFERRED_REFRESH_TASKS)) + return est return anyio.run(_run) @@ -272,6 +287,50 @@ def test_cian_empty_pool_in_production_degrades( assert "lookup failed" not in caplog.text, "причина подменена на неспецифичную" +# ── (б2) фоновый режим (прод-конфиг): пустой пул — WARNING, не ERROR ───────── + + +def test_cian_deferred_refresh_empty_pool_warns_without_traceback( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Прод-путь: ESTIMATE_EXTERNAL_SOURCES_BACKGROUND=true → фетч уходит в фон. + + На проде (`docker-compose.prod.yml`) флаг включён, поэтому синхронный вызов идёт с + `fetch_on_miss=False` и возвращает None ДО прокси-слоя (`providers/cian/valuation.py` + «cache MISS — fetch отложен») — ветка WARNING в `estimate_quality` там не звучит. + Настоящий фетч делает `_defer_external_refresh`, где `NoProxyAvailableError` попадал в + общий `except Exception: logger.exception(...)` → ERROR + traceback → событие в + GlitchTip: ровно тот шум, который #3398 и убирает. + """ + _use_pool_in_production(monkeypatch) + monkeypatch.setattr(settings, "estimate_external_sources_background", True) + estimator._DEFERRED_REFRESH_TASKS.clear() # чужие мёртвые задачи из прошлых loop'ов + + with ( + patch.object(estimator, "RealProxyProvider", _EmptyPoolProvider), + # Фоновая задача открывает СВОЮ сессию (сессия запроса закрыта вместе с ответом) — + # в тесте она не должна ходить в реальную БД. + patch.object(estimator, "SessionLocal", MagicMock(return_value=MagicMock())), + patch("scraper_kit.providers.cian.valuation._load_from_cache", return_value=None), + patch("scraper_kit.providers.cian.valuation.load_session", return_value={"cookie": "x"}), + patch("scraper_kit.providers.cian.valuation.AsyncSession", _no_http), + caplog.at_level(logging.INFO, logger="app.services.estimator"), + ): + est = _run_estimate(extra_patches=[], drain_deferred=True) + + assert est is not None + ours = [r for r in caplog.records if r.name == "app.services.estimator"] + deferred = [r for r in ours if "deferred cian_valuation" in r.getMessage()] + assert deferred, [r.getMessage() for r in ours] # фоновая задача не отработала + assert all("пул прокси пуст" in r.getMessage() for r in deferred), [ + r.getMessage() for r in deferred + ] + errors = [r for r in ours if r.levelno >= logging.ERROR] + assert not errors, [(r.levelname, r.getMessage()) for r in errors] + assert not any(r.exc_info for r in ours), "traceback приложен — GlitchTip получит событие" + assert "Traceback" not in caplog.text, caplog.text + + # ── (в) lease освобождён ровно один раз, через настоящий curl_proxy_url ──────