fix(tgbot): stop CI-hanging busy-spin in rate-limit test, isolate shared client in tests
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
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 / backend-tests (pull_request) Successful in 7m27s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
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 / backend-tests (pull_request) Successful in 7m27s
Root cause of the red PR #3494 CI job (9% progress, 75s life, no error line): test_send_message_rate_limits_across_different_topics_same_chat mocked asyncio.sleep as a pure no-op without advancing time.monotonic. The 3rd send (over the test's limit=2) entered TelegramGroupRateLimiter.acquire(), which recomputes wait_s from the real, unmocked clock every iteration - since the fake sleep never advances it, the window never expires and the while-loop busy-spins forever instead of actually waiting, until pytest-timeout kills it. Fixed by advancing a fake monotonic clock inside fake_sleep, matching the already-correct pattern used by the other tests in this file. Also added _reset_telegram_shared_client (tests/conftest.py, same pattern as _reset_estimate_rate_limiter): app.services.tgbot.shared._client is a module-level singleton whose rate limiter otherwise accumulates real wall-clock timestamps across the whole pytest session, not per test. Documented honestly in config.py: the API-role budget is shared between support web-chat mirrors and GlitchTip alerts with no priority between them, so a large alert burst can make the web-chat wait out its own timeout and return 502 - flagged as a known follow-up, not fixed here. NOTE: a full `pytest -q --timeout=60` run still hangs further into the suite, at tests/test_glitchtip_webhook.py::test_telegram_failure_returns_502_not_500. Not root-caused within this session's budget - the test's _fake_telegram_client fixture correctly monkeypatches glitchtip_module.get_telegram_client, but the anyio worker thread running the ASGI request is seen parked in a real event-loop poll/select wait, consistent with an actual (non-mocked) sleep somewhere in that path. Needs a follow-up session with a fresh time budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG
This commit is contained in:
parent
acbfdf0a18
commit
8142834555
3 changed files with 72 additions and 3 deletions
|
|
@ -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"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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) # тема алертов
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue