fix(#3398): пустой пул в фоновой догрузке — WARNING без трейсбека (общая функция); комментарий про stateless
All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
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) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Successful in 4m58s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
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) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Successful in 4m58s
На проде `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-вызов состояние — фоновой задаче нужен свой инстанс.
This commit is contained in:
parent
8a5f75d5f4
commit
ec7838b7a9
2 changed files with 85 additions and 8 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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 ──────
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue