diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index 95de5ccb..485484d8 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -1306,6 +1306,25 @@ class Settings(BaseSettings): # обычно только зеркалирование/уведомления, которые могут подождать дольше # (см. `TelegramGroupRateLimiter.acquire` про `max_wait=None` для фона). # 0/отрицательное значение выключает лимитер для соответствующего процесса. + # + # ЧЕСТНО ПРО ОГРАНИЧЕНИЕ ЭТОГО ДИЗАЙНА (review, #3471): + # `telegram_group_rate_limit_api_per_minute` — ОДИН общий бюджет на ВСЕ + # отправки процесса API в эту группу, а туда + # пишут И зеркала веб-чата поддержки (`app/api/v1/support.py`), И + # GlitchTip-алерты (`app/api/v1/glitchtip.py`) — обе ручки идут через один + # и тот же `get_telegram_client()` (см. `app/services/tgbot/shared.py`). + # Приоритета между ними НЕТ: кто первый встал в очередь `TelegramGroupRateLimiter`, + # тот и получил слот. Оба пути передают узкий `timeout` (5с у support, 8с у + # glitchtip) — он же становится потолком ожидания слота (см. + # `TelegramClient._request`, review H1). Значит при всплеске алертов (пачка + # ошибок прода бьёт в вебхук залпом) реально возможен сценарий: бюджет + # 12/мин исчерпан алертами → следующая отправка живого клиента в веб-чате + # ждёт до 5с и получает `TelegramRateLimitedError` → 502 клиенту поддержки. + # То есть при достаточно большом всплеске алертов веб-чат ДЕЙСТВИТЕЛЬНО + # может временно вставать. Разделить бюджет по ИСТОЧНИКУ (не по процессу) — + # отдельная задача: нужен свой `TelegramGroupRateLimiter` на алерты с явно + # малой квотой и/или приоритет для support-трафика; здесь НЕ сделано + # (вне бюджета этой правки). telegram_group_rate_limit_api_per_minute: int = Field( default=12, validation_alias="TELEGRAM_GROUP_RATE_LIMIT_API_PER_MINUTE" ) diff --git a/tradein-mvp/backend/tests/conftest.py b/tradein-mvp/backend/tests/conftest.py index 02b64a9f..71a916a3 100644 --- a/tradein-mvp/backend/tests/conftest.py +++ b/tradein-mvp/backend/tests/conftest.py @@ -4,8 +4,10 @@ `--strict-markers` в pyproject.toml не включён, так что незарегистрированный маркер только предупреждал бы) и сторожит глобальное состояние, которое переживает отдельный тест: общий rate-limiter POST /estimate (см. -`_reset_estimate_rate_limiter`) и слоты проверки пароля (см. -`_no_leaked_password_verify_slots`). +`_reset_estimate_rate_limiter`), слоты проверки пароля (см. +`_no_leaked_password_verify_slots`) и синглтон Telegram-клиента (см. +`_reset_telegram_shared_client`, #3471 — иначе его rate limiter копит +реальное время между тестами и вешает прогон). """ from __future__ import annotations @@ -48,6 +50,35 @@ def _reset_estimate_rate_limiter() -> None: ) +@pytest.fixture(autouse=True) +def _reset_telegram_shared_client(): + """`app.services.tgbot.shared._client` — модульный синглтон `TelegramClient` + (#3471). Его `TelegramGroupRateLimiter` копит РЕАЛЬНЫЕ метки времени + (`time.monotonic()`, ничем не замоканные) по `chat_id` за весь pytest-процесс, + а не по тесту — а тестовые настройки `telegram_alerts_chat_id`/ + `telegram_support_chat_id` дефолтятся в 0, так что ЛЮБЫЕ тесты, бьющие в + `app.api.v1.support`/`glitchtip` через реальный (не замоканный) shared-клиент, + делят ОДИН и тот же ключ бакета. После N-й (лимит роли, по умолчанию 12) + такой отправки в пределах 60 реальных секунд следующая уходит в настоящий + `asyncio.sleep` до 60с — тест не падает, а зависает, и именно так выглядела + смерть CI-джобы на #3494 (обрыв на ~9%, 75с жизни, ни строки об ошибке; + `pytest-timeout` затем добивает зависший тест снаружи). + + Фикстура не выключает и не завышает лимит (в проде он ДОЛЖЕН оставаться + ниже площадочного потолка) — она просто гарантирует каждому тесту СВЕЖИЙ + клиент (и тем самым свежий, пустой `TelegramGroupRateLimiter`), так же как + `_reset_estimate_rate_limiter` выше делает для `_estimate_limiter`. Сброс + и ДО, и ПОСЛЕ теста — тест мог создать клиент через `get_telegram_client()`, + не пройдя явный локальный `_reset_singleton` (см. `test_shared.py`), и не + должен оставить накопленное состояние следующему тесту. + """ + from app.services.tgbot import shared + + shared._client = None + yield + shared._client = None + + @pytest.fixture(autouse=True) def _no_leaked_password_verify_slots(): """Тест не оставляет за собой занятых слотов проверки пароля (#2665, #2714). diff --git a/tradein-mvp/backend/tests/services/tgbot/test_topic_check_and_group_rate_limit.py b/tradein-mvp/backend/tests/services/tgbot/test_topic_check_and_group_rate_limit.py index bd69bdd1..73a8ecfa 100644 --- a/tradein-mvp/backend/tests/services/tgbot/test_topic_check_and_group_rate_limit.py +++ b/tradein-mvp/backend/tests/services/tgbot/test_topic_check_and_group_rate_limit.py @@ -224,17 +224,36 @@ async def test_send_message_rate_limits_across_different_topics_same_chat() -> N Без правки лимитера нет вовсе — этот тест на старом коде проходил бы "слишком хорошо" (sleep не звался никогда), что и есть баг. + + ВАЖНО (post-merge review, #3494 CI hang): `fake_sleep` ОБЯЗАН двигать + подменённый `time.monotonic` вперёд, а не оставаться чистым no-op. Без + этого 3-я отправка (сверх лимита=2) уходит в `acquire()`, `wait_s` + пересчитывается из РЕАЛЬНОГО `time.monotonic()` (не замоканного здесь), + окно не истекает — и `while True` крутится настоящим busy-spin БЕЗ + единого реального ожидания, пока `pytest-timeout` не убьёт тест десятками + секунд спустя. Именно так этот тест сам стал причиной 75-секундного + зависания CI-джобы (обрыв на ~9%, ни строки об ошибке) — не сам лимитер и + не синглтон `shared.py` (гипотеза координатора была разумной, но к этому + конкретному зависанию отношения не имела). """ sleep_calls: list[float] = [] + fake_now = [0.0] + + def fake_monotonic() -> float: + return fake_now[0] async def fake_sleep(seconds: float) -> None: sleep_calls.append(seconds) + fake_now[0] += seconds def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(200, json={"ok": True, "result": {"message_id": 1}}) _install_transport(handler) - with mock.patch("app.services.tgbot.client.asyncio.sleep", fake_sleep): + with ( + mock.patch("app.services.tgbot.client.time.monotonic", fake_monotonic), + mock.patch("app.services.tgbot.client.asyncio.sleep", fake_sleep), + ): client = TelegramClient(token="fake-token", group_rate_limit_per_minute=2) await client.send_message(chat_id=42, text="a", message_thread_id=1) # тема поддержки await client.send_message(chat_id=42, text="b", message_thread_id=2) # тема алертов