From a2e8c46db77897ff0a61737d0cec0ae7af1c75d9 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 13 Sep 2026 13:31:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein):=20startup=20topic-check=20=D1=80?= =?UTF-8?q?=D0=B5=D0=B0=D0=BB=D1=8C=D0=BD=D0=BE=20=D0=B2=D0=B0=D0=BB=D0=B8?= =?UTF-8?q?=D0=B4=D0=B8=D1=80=D1=83=D0=B5=D1=82=20message=5Fthread=5Fid?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sendChatAction не проверяет message_thread_id — живой прод-замер (#3471) показал, что она подтверждает даже заведомо несуществующую тему ({"ok":true,"result":true}). verify_chat_and_topic поэтому была ложно-зелёной ровно в сценарии, ради которого её писали: удалённая или переименованная тема форума проходила проверку, а реальная отправка потом падала на каждом сообщении. Заменил проверку темы на sendMessage+deleteMessage: sendMessage с несуществующим message_thread_id мгновенно отвечает 400 "message thread not found" (тоже подтверждено на проде), а с валидным создаёт сообщение, которое сразу стирается deleteMessage — в истории треда не остаётся ничего. Docstring send_chat_action поправлен — он утверждал обратное. Новый тест воспроизводит замеренное поведение (sendChatAction лжёт, sendMessage честно отказывает) и явно проверен на падение против до-фиксной реализации (git checkout HEAD -- client.py + прогон теста дал assert True is False). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG --- .../backend/app/services/tgbot/client.py | 71 +++++++++++----- .../test_topic_check_and_group_rate_limit.py | 83 ++++++++++++++++--- 2 files changed, 123 insertions(+), 31 deletions(-) diff --git a/tradein-mvp/backend/app/services/tgbot/client.py b/tradein-mvp/backend/app/services/tgbot/client.py index 3313988e..71a0e52c 100644 --- a/tradein-mvp/backend/app/services/tgbot/client.py +++ b/tradein-mvp/backend/app/services/tgbot/client.py @@ -67,6 +67,11 @@ _DEFAULT_MAX_RETRIES = 5 _DEFAULT_GROUP_RATE_LIMIT_PER_MINUTE = 18 _RATE_LIMIT_WINDOW_S = 60.0 +# Текст зондирующего сообщения `verify_chat_and_topic` — см. докстринг там. +# Живёт в чате доли секунды (удаляется сразу после отправки), но должен быть +# узнаваем в логах ретранслятора/дебаге, если удаление вдруг не отработает. +_TOPIC_PROBE_TEXT = "\U0001f50d startup topic check" + class TelegramGroupRateLimiter: """Общий (per-`chat_id`, НЕ per-теме) ограничитель частоты отправки в группу. @@ -744,13 +749,13 @@ class TelegramClient: ) -> bool: """sendChatAction — статус набора текста. Возвращает `True`/`False`, JSON-объекта нет. - Используется НЕ по прямому назначению (индикация набора), а как способ - проверить существование `message_thread_id` (темы форума) — см. - `verify_chat_and_topic`. Это единственный метод Bot API, который - принимает `message_thread_id` и не создаёт message-объект: если тема - удалена/переименована в другую с иным id, Telegram отвечает `Bad - Request: message thread not found` мгновенно, а в истории чата не - остаётся ни строки (индикатор эфемерный и не персистится).""" + ⚠️ НЕ провалидировано для проверки `message_thread_id`: живой прод-замер + (#3471) показал, что Telegram принимает и мгновенно подтверждает + `sendChatAction` с ЗАВЕДОМО несуществующим `message_thread_id` + (`{"ok":true,"result":true}`) — метод молча принимает любую тему, + существующую или нет. Раньше здесь было обратное (неверное) + утверждение; см. `verify_chat_and_topic`, которая для проверки темы + использует `sendMessage`+`deleteMessage`.""" payload: dict[str, Any] = {"chat_id": chat_id, "action": action} if message_thread_id: payload["message_thread_id"] = message_thread_id @@ -759,6 +764,22 @@ class TelegramClient: ) return bool(result) + async def delete_message(self, *, chat_id: int, message_id: int) -> bool: + """deleteMessage — удаляет сообщение бота. + + Используется `verify_chat_and_topic` для зачистки зондирующего + `sendMessage`-пробника сразу после проверки темы: тема существует + тогда и только тогда, когда сообщение вообще удалось отправить — + поэтому к моменту вызова `deleteMessage` id уже гарантированно + валиден.""" + result = await self._request( + "deleteMessage", + {"chat_id": chat_id, "message_id": message_id}, + max_retries=1, + max_backoff=5.0, + ) + return bool(result) + async def verify_chat_and_topic( client: TelegramClient, @@ -776,18 +797,23 @@ async def verify_chat_and_topic( отказами: тихий отказ хуже шума). Эта проверка переносит обнаружение с «через сутки тишины» на «в первую секунду после старта/рестарта». - Способ намеренно НЕ `sendMessage`+`deleteMessage`: + Способ: `getChat(chat_id)` + `sendMessage`/`deleteMessage` в саму тему. - `getChat(chat_id)` подтверждает валидность чата и то, что бот не выгнан/не заблокирован — чистый read, нулевой видимый след. - - `send_chat_action` (typing-индикатор с `message_thread_id`) — - единственный способ провалидировать САМУ тему без создания - message-объекта: Telegram обязан знать про `message_thread_id`, чтобы - показать «печатает...» именно в нужном треде, и явно отказывает, если - такой темы нет. `sendMessage`+`deleteMessage` тоже сработал бы, но - оставлял бы видимый (пусть на секунды) артефакт в истории треда при - КАЖДОМ рестарте контейнера — на rolling-деплое это многократно в - сутки; typing-индикатор того же результата достигает без единого - сообщения. + - Раньше здесь стоял `send_chat_action` (typing-индикатор с + `message_thread_id`) в расчёте на то, что Telegram обязан знать про + `message_thread_id`, чтобы показать «печатает...» именно в нужном + треде. Живой прод-замер (#3471) опроверг это: `sendChatAction` + принимает и подтверждает ЗАВЕДОМО несуществующий + `message_thread_id` (`{"ok":true,"result":true}`) — проверка была + ложно-зелёной ВСЕГДА, удалённая/переименованная тема проходила её + так же, как живая, а реальная отправка потом падала на каждом + сообщении. `sendMessage` с тем же мусорным id вместо этого мгновенно + отвечает `400 Bad Request: message thread not found` (эмпирически + подтверждено на том же проде), а с валидным id создаёт сообщение — + которое здесь же стирается `deleteMessage`, так что в истории треда + не остаётся ничего, кроме микросекундного технического сообщения на + каждом старте/рестарте контейнера. НЕ роняет процесс: любой `TelegramError` ловится здесь же и уходит в лог уровня error — задача явно требует шума в логе, а не падения воркера @@ -802,9 +828,16 @@ async def verify_chat_and_topic( try: await client.get_chat(chat_id) if topic_id: - await client.send_chat_action( - chat_id=chat_id, action="typing", message_thread_id=topic_id + probe = await client.send_message( + chat_id=chat_id, + text=_TOPIC_PROBE_TEXT, + message_thread_id=topic_id, + max_retries=1, + max_backoff=5.0, ) + probe_message_id = probe.get("message_id") + if probe_message_id: + await client.delete_message(chat_id=chat_id, message_id=probe_message_id) except TelegramError as exc: logger.error( "tg topic check [%s]: чат/тема недоступны для отправки " 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 73a8ecfa..1b23edd6 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 @@ -14,6 +14,7 @@ NEVER calls real Telegram API — httpx.MockTransport only, тот же патт from __future__ import annotations +import json import os from unittest import mock @@ -51,12 +52,19 @@ def _stop_patches(): # ── verify_chat_and_topic ──────────────────────────────────────────────────── -async def test_verify_chat_and_topic_success_calls_get_chat_and_send_chat_action() -> None: - """Happy path: getChat + typing-индикатор в тему, никакого видимого сообщения.""" +async def test_verify_chat_and_topic_success_uses_send_message_and_delete_message() -> None: + """Happy path: getChat + зондирующий sendMessage в тему + его deleteMessage. + + `sendChatAction` тут больше не используется: живой прод-замер (#3471) + показал, что она подтверждает даже несуществующий `message_thread_id` + (`{"ok":true,"result":true}`) — проверка была ложно-зелёной.""" calls: list[str] = [] def handler(request: httpx.Request) -> httpx.Response: - calls.append(request.url.path.rsplit("/", 1)[-1]) + method = request.url.path.rsplit("/", 1)[-1] + calls.append(method) + if method == "sendMessage": + return httpx.Response(200, json={"ok": True, "result": {"message_id": 42}}) return httpx.Response(200, json={"ok": True, "result": True}) _install_transport(handler) @@ -65,24 +73,31 @@ async def test_verify_chat_and_topic_success_calls_get_chat_and_send_chat_action ok = await verify_chat_and_topic(client, chat_id=-100123, topic_id=7, label="support") assert ok is True - assert calls == ["getChat", "sendChatAction"] + assert calls == ["getChat", "sendMessage", "deleteMessage"] -async def test_verify_chat_and_topic_no_visible_message_method_used() -> None: - """Ни один из вызовов не бьёт в sendMessage/copyMessage — не мусорим в чат.""" - methods: list[str] = [] +async def test_verify_chat_and_topic_deletes_probe_message_leaving_no_trace() -> None: + """Зонд `sendMessage` не остаётся видимым — `deleteMessage` бьёт по тому же + `message_id`, что вернул `sendMessage` (не мусорим в истории треда).""" + delete_payloads: list[dict[str, object]] = [] def handler(request: httpx.Request) -> httpx.Response: - methods.append(request.url.path.rsplit("/", 1)[-1]) - return httpx.Response(200, json={"ok": True, "result": True}) + method = request.url.path.rsplit("/", 1)[-1] + if method == "sendMessage": + return httpx.Response(200, json={"ok": True, "result": {"message_id": 986}}) + if method == "deleteMessage": + delete_payloads.append(json.loads(request.content)) + return httpx.Response(200, json={"ok": True, "result": True}) + return httpx.Response(200, json={"ok": True, "result": {}}) _install_transport(handler) client = TelegramClient(token="fake-token") - await verify_chat_and_topic(client, chat_id=-100123, topic_id=7, label="support") + ok = await verify_chat_and_topic(client, chat_id=-100123, topic_id=7, label="support") - assert "sendMessage" not in methods - assert "copyMessage" not in methods + assert ok is True + assert len(delete_payloads) == 1 + assert delete_payloads[0]["message_id"] == 986 async def test_verify_chat_and_topic_logs_error_and_does_not_raise(caplog) -> None: @@ -135,6 +150,50 @@ async def test_verify_chat_and_topic_skips_api_call_when_chat_id_is_zero() -> No assert calls == [] +async def test_verify_chat_and_topic_is_red_when_topic_deleted_even_though_send_chat_action_lies( + caplog, +) -> None: + """Регрессия #3471: удалённая/переименованная тема ДОЛЖНА красить проверку. + + Handler воспроизводит РЕАЛЬНО замеренное на проде поведение Bot API: + `sendChatAction` подтверждает даже несуществующий `message_thread_id` + (`{"ok":true,"result":true}` — живой прод-замер, #3471), а `sendMessage` + с тем же id честно отвечает `400 Bad Request: message thread not found`. + + На коде ДО правки (`verify_chat_and_topic` звала `send_chat_action`) этот + тест падает: `ok` был бы `True` — проверка молчаливо считала мёртвую тему + живой. Явно проверено при разработке (#3471): `git checkout HEAD -- + .../client.py` (до-фиксная версия) + прогон только этого теста дал + `assert True is False`; после возврата фикса — зелёный.""" + + def handler(request: httpx.Request) -> httpx.Response: + method = request.url.path.rsplit("/", 1)[-1] + if method == "getChat": + return httpx.Response(200, json={"ok": True, "result": {"id": -100123}}) + if method == "sendChatAction": + # Реальный прод-баг: sendChatAction НЕ валидирует message_thread_id. + return httpx.Response(200, json={"ok": True, "result": True}) + if method == "sendMessage": + return httpx.Response( + 400, + json={ + "ok": False, + "error_code": 400, + "description": "Bad Request: message thread not found", + }, + ) + return httpx.Response(200, json={"ok": True, "result": True}) + + _install_transport(handler) + client = TelegramClient(token="fake-token") + + with caplog.at_level("ERROR", logger="app.services.tgbot.client"): + ok = await verify_chat_and_topic(client, chat_id=-100123, topic_id=987654, label="support") + + assert ok is False + assert any(record.levelname == "ERROR" for record in caplog.records) + + # ── TelegramGroupRateLimiter ───────────────────────────────────────────────── -- 2.45.3