fix(tradein): time-budget estimate enrichment + observable errors to stop opaque 502 (#654)

POST /estimate returned an opaque 502 (gateway timeout), not a null-floor
crash: the ungated Yandex valuation (30s httpx) + geocode/Overpass chain run
serially on every estimate. Wraps slow enrichments in asyncio.wait_for budgets
(config) so a slow upstream degrades to None instead of exceeding the gateway
read timeout. Handler try/except -> logger.exception + 503 so future real
errors are visible (GlitchTip) not masked. Explicit Caddy read/write timeout.
Tests: null-floor skips IMV/Cian (no 5xx); Yandex timeout degrades. 65 regr pass.
NOTE: Caddyfile.tradein-fragment is shared infra — devops review required.

Closes #654

tradein-mvp/backend/app/api/v1/trade_in.py

@@ -61,7 +61,22 @@ async def estimate(
     """
     account_quota.check_and_raise(db, x_authenticated_user)
     from app.services.estimator import estimate_quality
-    result = await estimate_quality(payload, db)
+
+    # #654: ранее любое исключение estimate_quality всплывало необработанным и
+    # маскировалось апстрим-прокси (Caddy) как непрозрачный 502. Ловим, логируем
+    # через logger.exception (→ GlitchTip/Sentry получает stack trace) и отдаём
+    # явный 503 — так любая БУДУЩАЯ реальная ошибка становится видимой, а не
+    # «глотается» шлюзом. HTTPException пробрасываем как есть (это не сбой).
+    try:
+        result = await estimate_quality(payload, db)
+    except HTTPException:
+        raise
+    except Exception:
+        logger.exception("estimate failed for address=%r", payload.address)
+        raise HTTPException(
+            status_code=503,
+            detail="estimate temporarily unavailable — try again shortly",
+        ) from None
     account_quota.increment(db, x_authenticated_user)
     return result
 

tradein-mvp/backend/app/core/config.py

@@ -47,5 +47,20 @@ class Settings(BaseSettings):
     dadata_api_token: str | None = None
     dadata_api_secret: str | None = None
 
+    # ── Estimate enrichment time-budgets (#654) ──────────────────────────────
+    # POST /estimate делает несколько ПОСЛЕДОВАТЕЛЬНЫХ блокирующих сетевых
+    # вызовов (geocode → Overpass → Yandex valuation → IMV → Cian). Yandex
+    # valuation (внутренний httpx timeout 30s) НЕ gated на наличие floor и
+    # выполняется на каждой оценке — главный подозреваемый на gateway-таймаут
+    # (Caddy 502/504). Эти budget'ы оборачивают самые медленные ungated-вызовы
+    # в asyncio.wait_for(): при превышении источник деградирует в None (тот же
+    # graceful-путь что и сетевая ошибка), а НЕ роняет весь /estimate в 5xx.
+    # Держать суммарный budget ниже Caddy read/write timeout (см.
+    # deploy/Caddyfile.tradein-fragment). ENV: ESTIMATE_YANDEX_VALUATION_TIMEOUT_S,
+    # ESTIMATE_GEOCODE_BUDGET_S, ESTIMATE_HOUSE_META_TIMEOUT_S.
+    estimate_yandex_valuation_timeout_s: float = 8.0
+    estimate_geocode_budget_s: float = 12.0
+    estimate_house_meta_timeout_s: float = 8.0
+
 
 settings = Settings()

tradein-mvp/backend/app/services/estimator.py

@@ -19,6 +19,7 @@
 
 from __future__ import annotations
 
+import asyncio
 import hashlib
 import json
 import logging
@@ -32,6 +33,7 @@ from uuid import uuid4
 from sqlalchemy import text
 from sqlalchemy.orm import Session
 
+from app.core.config import settings
 from app.schemas.trade_in import (
     AggregatedEstimate,
     AnalogLot,
@@ -658,6 +660,27 @@ def _save_yandex_history_items(
         return 0
 
 
+# ── Time-budget guard (#654) ────────────────────────────────────────────────
+async def _with_budget(coro: Any, budget_s: float, *, label: str) -> Any:
+    """Await `coro` under an asyncio.wait_for() time budget.
+
+    On timeout the coroutine is cancelled and we return None — mapping a slow
+    upstream onto the SAME graceful "None" path these enrichments already take
+    on network error, so a single slow source degrades the estimate instead of
+    blowing the gateway read timeout (#654: opaque Caddy 502/504).
+
+    budget_s <= 0 disables the guard (await directly) — escape hatch via config.
+    """
+    if budget_s is None or budget_s <= 0:
+        return await coro
+    try:
+        return await asyncio.wait_for(coro, timeout=budget_s)
+    except TimeoutError:
+        # asyncio.TimeoutError is an alias of builtin TimeoutError (py3.11+).
+        logger.warning("%s exceeded %.1fs budget — degrading to None (#654)", label, budget_s)
+        return None
+
+
 # ── Public ───────────────────────────────────────────────────────────────────
 async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> AggregatedEstimate:
     """Главная функция — оценка квартиры по реальным данным.
@@ -672,10 +695,15 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
     Returns:
         AggregatedEstimate с estimate_id, медианой, диапазоном, аналогами.
     """
-    # 1. Geocode
+    # 1. Geocode (#654: time-budgeted — Yandex/Nominatim retry chain can stack
+    #    multiple network round-trips + 1s Nominatim rate-limit sleeps).
     geo: GeocodeResult | None = None
     if payload.address:
-        geo = await geocode(payload.address, db)
+        geo = await _with_budget(
+            geocode(payload.address, db),
+            settings.estimate_geocode_budget_s,
+            label="geocode",
+        )
 
     if geo is None:
         # Без координат не можем искать через PostGIS. Возвращаем low confidence.
@@ -716,10 +744,16 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
     # 2. #392: обогащаем год / тип дома из картографии (OSM Overpass), если
     #    пользователь их не указал — это улучшает house-match аналогов (#6).
     #    Best-effort: при недоступности OSM target_* остаются None.
+    #    #654: time-budgeted — Overpass httpx timeout 15s сам по себе близок к
+    #    gateway-таймауту; деградируем в None при превышении budget.
     target_year = payload.year_built
     target_house_type = payload.house_type
     if target_year is None or target_house_type is None:
-        house_meta = await get_house_metadata(geo.lat, geo.lon, db)
+        house_meta = await _with_budget(
+            get_house_metadata(geo.lat, geo.lon, db),
+            settings.estimate_house_meta_timeout_s,
+            label="house_metadata(overpass)",
+        )
         if house_meta is not None:
             if target_year is None:
                 target_year = house_meta.year_built
@@ -932,11 +966,18 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
         sources_used_pre = sorted(set(sources_used_pre) | {"avito_imv"})
 
     # ── Stage 8: Yandex Valuation as on-demand source (anonymous, cached 24h) ──
+    # #654: главный латентность-подозреваемый. Этот источник UNGATED — бежит на
+    # КАЖДОЙ оценке (в т.ч. без floor/total_floors), а его внутренний httpx
+    # timeout 30s + curl_cffi impersonation + sleep_between_requests могут одни
+    # превысить gateway read timeout → opaque 502. Оборачиваем в budget: при
+    # превышении → None (cache-hit путь быстрый, timeout бьёт только по медленному
+    # cache-miss fetch). Деградация идентична сетевой ошибке внутри функции.
     yandex_val: YandexValuationResult | None = None
     if geo is not None and geo.full_address:
-        yandex_val = await _get_or_fetch_yandex_valuation_cached(
-            db,
-            address=geo.full_address,
+        yandex_val = await _with_budget(
+            _get_or_fetch_yandex_valuation_cached(db, address=geo.full_address),
+            settings.estimate_yandex_valuation_timeout_s,
+            label="yandex_valuation",
         )
         if yandex_val is not None:
             sources_used_pre = sorted(set(sources_used_pre) | {"yandex_valuation"})

tradein-mvp/backend/tests/test_estimator_null_floor_timeout.py

@@ -0,0 +1,232 @@
+"""Regression tests for #654 — POST /estimate 502 on null floor / slow upstream.
+
+Two contracts locked here:
+  1. estimate with floor=None & total_floors=None returns an AggregatedEstimate
+     (no exception) AND the floor-gated sources (Avito IMV + Cian valuation) are
+     NOT awaited — this is the gating contract that makes null-floor safe.
+  2. A Yandex-valuation timeout (asyncio.TimeoutError / httpx.TimeoutException)
+     degrades gracefully: estimate WITHOUT 'yandex_valuation' in sources_used,
+     no 5xx. Locks the #654 time-budget → None mapping.
+
+Style mirrors test_estimator_cian_integration.py: MagicMock db, geocode /
+_fetch_analogs / _fetch_deals patched, scrapers stubbed.
+"""
+
+import os
+
+# Settings requires DATABASE_URL at init time. Set dummy DSN before any app import.
+os.environ.setdefault("DATABASE_URL", "postgresql://test:test@localhost/test_db")
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import anyio
+import httpx
+
+
+def _make_fake_geo():
+    from app.services.geocoder import GeocodeResult
+
+    return GeocodeResult(
+        lat=56.838,
+        lon=60.595,
+        full_address="Свердловская обл., Екатеринбург, ул. Учителей, 18",
+        provider="nominatim",
+    )
+
+
+def _make_payload_null_floor():
+    """Simplified-form payload: floor + total_floors are None (the #654 trigger)."""
+    from app.schemas.trade_in import TradeInEstimateInput
+
+    return TradeInEstimateInput(
+        address="ЕКБ, ул. Учителей, 18",
+        area_m2=38.8,
+        rooms=1,
+        floor=None,
+        total_floors=None,
+    )
+
+
+def _make_payload_full():
+    from app.schemas.trade_in import TradeInEstimateInput
+
+    return TradeInEstimateInput(
+        address="ЕКБ, ул. Учителей, 18",
+        area_m2=38.8,
+        rooms=1,
+        floor=4,
+        total_floors=16,
+    )
+
+
+def test_estimate_null_floor_skips_imv_and_cian() -> None:
+    """floor=None & total_floors=None → AggregatedEstimate, IMV+Cian NOT awaited.
+
+    Locks the gating contract: the two floor-required sources must be skipped
+    (not called) when floor data is absent — confirming the null path is safe
+    and the 502 was never a null-floor crash (#654).
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_null_floor()
+
+    imv_mock = AsyncMock(return_value=None)
+    cian_mock = AsyncMock(return_value=None)
+    yandex_mock = AsyncMock(return_value=None)
+
+    async def _run() -> None:
+        with (
+            patch("app.services.estimator.geocode",
+                  new=AsyncMock(return_value=_make_fake_geo())),
+            patch("app.services.estimator.get_house_metadata",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached", new=imv_mock),
+            patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
+                  new=yandex_mock),
+            patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        # No exception — got a real estimate back.
+        assert result.estimate_id is not None
+        # Gating contract: floor-required sources never awaited.
+        imv_mock.assert_not_awaited()
+        cian_mock.assert_not_awaited()
+        assert "avito_imv" not in result.sources_used
+        assert "cian_valuation" not in result.sources_used
+
+    anyio.run(_run)
+
+
+def test_estimate_full_floor_awaits_imv_and_cian() -> None:
+    """Sanity counter-test: with floor present, IMV+Cian ARE awaited.
+
+    Guards against a regression where the gating predicate gets inverted /
+    broken — proves the null-floor skip above is the gate, not a no-op.
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_full()
+
+    imv_mock = AsyncMock(return_value=None)
+    cian_mock = AsyncMock(return_value=None)
+    yandex_mock = AsyncMock(return_value=None)
+
+    async def _run() -> None:
+        with (
+            patch("app.services.estimator.geocode",
+                  new=AsyncMock(return_value=_make_fake_geo())),
+            patch("app.services.estimator.get_house_metadata",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached", new=imv_mock),
+            patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
+                  new=yandex_mock),
+            patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        assert result.estimate_id is not None
+        cian_mock.assert_awaited_once()
+        # IMV is additionally gated on house_type/renovation mapping; with
+        # house_type=None it is correctly skipped — so we assert on Cian only,
+        # which is gated purely on floor/total_floors presence.
+
+    anyio.run(_run)
+
+
+def test_estimate_yandex_timeout_degrades_no_5xx() -> None:
+    """Yandex valuation raising TimeoutError → estimate without it, no exception.
+
+    The _with_budget() guard wraps the call; asyncio.wait_for surfaces a
+    TimeoutError which must map to the same graceful None path as a network
+    error. Estimator returns an AggregatedEstimate; 'yandex_valuation' absent.
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_null_floor()
+
+    # Yandex call hangs/raises a timeout — simulate the slow-upstream case.
+    yandex_mock = AsyncMock(side_effect=TimeoutError("yandex slow"))
+
+    async def _run() -> None:
+        with (
+            patch("app.services.estimator.geocode",
+                  new=AsyncMock(return_value=_make_fake_geo())),
+            patch("app.services.estimator.get_house_metadata",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
+                  new=yandex_mock),
+            patch("app.services.estimator.estimate_via_cian_valuation",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        assert result.estimate_id is not None  # no 5xx — degraded gracefully
+        assert "yandex_valuation" not in result.sources_used
+
+    anyio.run(_run)
+
+
+def test_estimate_yandex_httpx_timeout_at_scraper_degrades_no_5xx() -> None:
+    """Real network path: scraper raises httpx.TimeoutException on cache-miss fetch.
+
+    Here we DON'T patch _get_or_fetch_yandex_valuation_cached — we let the real
+    cache helper run with a cache MISS, then make the underlying scraper raise
+    httpx.TimeoutException. The helper's internal `except Exception` maps it to
+    None (graceful), so the estimate completes without 'yandex_valuation' and
+    no 5xx. This is the production-realistic timeout path (#654).
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    # Cache MISS so the helper proceeds to the (timing-out) fresh fetch.
+    db.execute.return_value.mappings.return_value.first.return_value = None
+    payload = _make_payload_null_floor()
+
+    fake_scraper = MagicMock()
+    fake_scraper.__aenter__ = AsyncMock(return_value=fake_scraper)
+    fake_scraper.__aexit__ = AsyncMock(return_value=False)
+    fake_scraper.fetch_house_history = AsyncMock(
+        side_effect=httpx.TimeoutException("read timeout")
+    )
+
+    async def _run() -> None:
+        with (
+            patch("app.services.estimator.geocode",
+                  new=AsyncMock(return_value=_make_fake_geo())),
+            patch("app.services.estimator.get_house_metadata",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator.YandexValuationScraper",
+                  return_value=fake_scraper),
+            patch("app.services.estimator.estimate_via_cian_valuation",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        assert result.estimate_id is not None
+        assert "yandex_valuation" not in result.sources_used
+
+    anyio.run(_run)

tradein-mvp/deploy/Caddyfile.tradein-fragment

@@ -15,7 +15,21 @@
 handle_path /trade-in/api/* {
     # handle_path вырезает префикс /trade-in перед прокси →
     # FastAPI получает /api/v1/...
-    reverse_proxy tradein-backend:8000
+    reverse_proxy tradein-backend:8000 {
+        # #654: явный read/write timeout. POST /api/v1/trade-in/estimate делает
+        # цепочку последовательных внешних вызовов (geocode → Overpass → Yandex
+        # valuation → IMV → Cian). In-app time-budget (config.py
+        # estimate_*_timeout_s) суммарно ≈28s worst-case; ставим upstream timeout
+        # ВЫШЕ него (45s), чтобы 503-деградация из приложения доходила до клиента
+        # как 503, а НЕ обрезалась прокси в непрозрачный 502/504. Делает поведение
+        # детерминированным и согласованным с in-app budget.
+        # SHARED-INFRA: этот фрагмент инклудится в основной gendsgn.ru Caddy-блок
+        # на bot-server — требует review devops перед deploy.
+        transport http {
+            read_timeout 45s
+            write_timeout 45s
+        }
+    }
 }
 
 handle /trade-in {