From 8901e60f6ad1168c4e82e263e70c0302e4afd96c Mon Sep 17 00:00:00 2001 From: Mikhail Putilovskij Date: Thu, 2 Apr 2026 13:44:59 +0300 Subject: [PATCH] =?UTF-8?q?fix(tg):=20reviewer=20fixes=20=E2=80=94=20error?= =?UTF-8?q?=20handling,=20timeouts,=20db=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - commands.py: try/except TelegramBadRequest around all Bot API calls (#2); /new handles "topics limit" with user-friendly message (#4) - start.py: isolate _check_and_prune_stale_topics with try/except Exception (#3) - message.py: asyncio.timeout(30) around stream_message; handle TimeoutError (#6) - db.py: add idx_chats_user_id index in init_db() (#7) - settings.py: remove dead active_chat_id variable (#8) - tests: add test_message.py (stream error/success); add 2 tests in test_commands.py (topics limit, /archive in General topic) --- adapter/telegram/db.py | 1 + adapter/telegram/handlers/commands.py | 31 +++++++-- adapter/telegram/handlers/message.py | 33 ++++++---- adapter/telegram/handlers/settings.py | 3 - adapter/telegram/handlers/start.py | 5 +- tests/adapter/telegram/test_commands.py | 26 ++++++++ tests/adapter/telegram/test_message.py | 87 +++++++++++++++++++++++++ 7 files changed, 161 insertions(+), 25 deletions(-) create mode 100644 tests/adapter/telegram/test_message.py diff --git a/adapter/telegram/db.py b/adapter/telegram/db.py index d9c10aa..7e4602a 100644 --- a/adapter/telegram/db.py +++ b/adapter/telegram/db.py @@ -29,6 +29,7 @@ def init_db() -> None: created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, PRIMARY KEY (user_id, thread_id) ); + CREATE INDEX IF NOT EXISTS idx_chats_user_id ON chats(user_id); """) diff --git a/adapter/telegram/handlers/commands.py b/adapter/telegram/handlers/commands.py index a18e2af..65efead 100644 --- a/adapter/telegram/handlers/commands.py +++ b/adapter/telegram/handlers/commands.py @@ -2,6 +2,7 @@ from __future__ import annotations import structlog from aiogram import Router +from aiogram.exceptions import TelegramBadRequest from aiogram.filters import Command from aiogram.types import Message @@ -20,7 +21,15 @@ async def cmd_new(message: Message) -> None: chat_id = message.chat.id n = db.count_active_chats(user_id) + 1 new_name = f"Чат #{n}" - topic = await message.bot.create_forum_topic(chat_id=chat_id, name=new_name) + try: + topic = await message.bot.create_forum_topic(chat_id=chat_id, name=new_name) + except TelegramBadRequest as e: + if "topics limit" in str(e).lower(): + await message.answer("Достигнут лимит топиков (1000). Заархивируй неиспользуемые чаты.") + else: + logger.error("cmd_new_failed", error=str(e)) + await message.answer("Не удалось создать чат, попробуй позже.") + return thread_id = topic.message_thread_id db.create_chat(user_id=user_id, thread_id=thread_id, chat_name=new_name) await message.bot.send_message( @@ -40,7 +49,10 @@ async def cmd_archive(message: Message) -> None: if chat is None or chat["archived_at"] is not None: await message.answer("Этот чат не найден или уже архивирован.") return - await message.bot.close_forum_topic(chat_id=message.chat.id, message_thread_id=thread_id) + try: + await message.bot.close_forum_topic(chat_id=message.chat.id, message_thread_id=thread_id) + except TelegramBadRequest as e: + logger.warning("cmd_archive_bot_error", error=str(e)) db.archive_chat(user_id=user_id, thread_id=thread_id) logger.info("cmd_archive", user_id=user_id, thread_id=thread_id) @@ -59,11 +71,16 @@ async def cmd_rename(message: Message) -> None: if chat is None: await message.answer("Этот чат не найден.") return - await message.bot.edit_forum_topic( - chat_id=message.chat.id, - message_thread_id=thread_id, - name=new_name[:128], - ) + try: + await message.bot.edit_forum_topic( + chat_id=message.chat.id, + message_thread_id=thread_id, + name=new_name[:128], + ) + except TelegramBadRequest as e: + logger.error("cmd_rename_failed", error=str(e)) + await message.answer("Не удалось переименовать топик.") + return db.rename_chat(user_id=user_id, thread_id=thread_id, new_name=new_name[:128]) logger.info("cmd_rename", user_id=user_id, thread_id=thread_id, new_name=new_name) diff --git a/adapter/telegram/handlers/message.py b/adapter/telegram/handlers/message.py index 22f8770..d70df0d 100644 --- a/adapter/telegram/handlers/message.py +++ b/adapter/telegram/handlers/message.py @@ -1,5 +1,6 @@ from __future__ import annotations +import asyncio import time import structlog @@ -46,22 +47,26 @@ async def handle_topic_message(message: Message, dispatcher: EventDispatcher) -> last_edit_len = 0 try: - async for chunk in dispatcher._platform.stream_message( - user_id=platform_user.user_id, - chat_id=str(thread_id), - text=incoming.text, - attachments=None, - ): - accumulated += chunk.delta - now = time.monotonic() - delta = len(accumulated) - last_edit_len - if delta >= STREAM_MIN_DELTA and (now - last_edit_time) >= STREAM_EDIT_INTERVAL: - await _safe_edit(placeholder, accumulated) - last_edit_time = now - last_edit_len = len(accumulated) + async with asyncio.timeout(30): + async for chunk in dispatcher._platform.stream_message( + user_id=platform_user.user_id, + chat_id=str(thread_id), + text=incoming.text, + attachments=None, + ): + accumulated += chunk.delta + now = time.monotonic() + delta = len(accumulated) - last_edit_len + if delta >= STREAM_MIN_DELTA and (now - last_edit_time) >= STREAM_EDIT_INTERVAL: + await _safe_edit(placeholder, accumulated) + last_edit_time = now + last_edit_len = len(accumulated) - await _safe_edit(placeholder, accumulated or "...") + await _safe_edit(placeholder, accumulated or "...") + except TimeoutError: + logger.warning("platform_timeout", user_id=user_id, thread_id=thread_id) + await _safe_edit(placeholder, "Сервис не отвечает, попробуй позже") except TelegramBadRequest as e: if "thread not found" in str(e).lower(): db.archive_chat(user_id=user_id, thread_id=thread_id) diff --git a/adapter/telegram/handlers/settings.py b/adapter/telegram/handlers/settings.py index afab801..7a98a1b 100644 --- a/adapter/telegram/handlers/settings.py +++ b/adapter/telegram/handlers/settings.py @@ -34,9 +34,6 @@ async def cb_settings_back(callback: CallbackQuery, state: FSMContext) -> None: @router.callback_query(F.data == "settings:skills") async def cb_skills(callback: CallbackQuery, state: FSMContext, dispatcher: EventDispatcher) -> None: - data = await state.get_data() - active_chat_id = data.get("active_chat_id", "") - # Get platform user id platform_user_id = str(callback.from_user.id) settings = await dispatcher._platform.get_settings(platform_user_id) diff --git a/adapter/telegram/handlers/start.py b/adapter/telegram/handlers/start.py index a33dd04..789f649 100644 --- a/adapter/telegram/handlers/start.py +++ b/adapter/telegram/handlers/start.py @@ -24,7 +24,10 @@ async def cmd_start(message: Message) -> None: user_id = message.from_user.id chat_id = message.chat.id - await _check_and_prune_stale_topics(message, user_id, chat_id) + try: + await _check_and_prune_stale_topics(message, user_id, chat_id) + except Exception: + logger.exception("prune_stale_topics_error", user_id=user_id) active = db.get_active_chats(user_id) diff --git a/tests/adapter/telegram/test_commands.py b/tests/adapter/telegram/test_commands.py index 1d8d1b9..0d7d321 100644 --- a/tests/adapter/telegram/test_commands.py +++ b/tests/adapter/telegram/test_commands.py @@ -5,6 +5,7 @@ from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock import pytest +from aiogram.exceptions import TelegramBadRequest @pytest.fixture(autouse=True) @@ -74,3 +75,28 @@ async def test_cmd_rename_updates_db_and_topic(fresh_db, monkeypatch): chat_id=100, message_thread_id=42, name="Работа" ) assert fresh_db.get_chat(1, 42)["chat_name"] == "Работа" + + +async def test_cmd_new_topics_limit(fresh_db): + """When Telegram returns topics limit error, user gets a friendly message.""" + import adapter.telegram.handlers.commands as mod + importlib.reload(mod) + msg = make_message(user_id=1, thread_id=42, chat_id=100) + msg.bot.create_forum_topic = AsyncMock( + side_effect=TelegramBadRequest(method=MagicMock(), message="topics limit exceeded") + ) + await mod.cmd_new(msg) + msg.answer.assert_called_once() + assert "лимит" in msg.answer.call_args[0][0] + # No chat should be created + assert fresh_db.count_active_chats(1) == 0 + + +async def test_cmd_archive_general_topic(fresh_db): + """/archive in General topic (thread_id=None) replies with 'not found'.""" + import adapter.telegram.handlers.commands as mod + importlib.reload(mod) + msg = make_message(user_id=1, thread_id=None, chat_id=100) + await mod.cmd_archive(msg) + msg.answer.assert_called_once() + msg.bot.close_forum_topic.assert_not_called() diff --git a/tests/adapter/telegram/test_message.py b/tests/adapter/telegram/test_message.py new file mode 100644 index 0000000..69aab1e --- /dev/null +++ b/tests/adapter/telegram/test_message.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +import importlib +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +@pytest.fixture(autouse=True) +def fresh_db(tmp_path, monkeypatch): + monkeypatch.setenv("DB_PATH", str(tmp_path / "test.db")) + import adapter.telegram.db as db_mod + importlib.reload(db_mod) + db_mod.init_db() + return db_mod + + +def make_message(*, user_id=1, thread_id=42, chat_id=100): + m = SimpleNamespace() + m.from_user = SimpleNamespace(id=user_id, full_name="Alice") + m.message_thread_id = thread_id + m.chat = SimpleNamespace(id=chat_id) + m.text = "Hello" + m.photo = None + m.document = None + m.voice = None + m.video = None + m.sticker = None + m.answer = AsyncMock() + placeholder = MagicMock() + placeholder.edit_text = AsyncMock() + m.reply = AsyncMock(return_value=placeholder) + m.bot = MagicMock() + return m, placeholder + + +def make_dispatcher(chunks=None, raise_exc=None): + """Build a mock EventDispatcher with configurable stream_message behaviour.""" + async def _stream(*args, **kwargs): + if raise_exc is not None: + raise raise_exc + for chunk in (chunks or []): + yield chunk + + platform = MagicMock() + platform.get_or_create_user = AsyncMock( + return_value=SimpleNamespace(user_id="uid-1") + ) + platform.stream_message = _stream + + dispatcher = MagicMock() + dispatcher._platform = platform + return dispatcher + + +async def test_stream_exception_shows_error(fresh_db): + """When stream_message raises, the placeholder is updated with an error message.""" + fresh_db.create_chat(1, 42, "Чат #1") + import adapter.telegram.handlers.message as mod + importlib.reload(mod) + + msg, placeholder = make_message() + dispatcher = make_dispatcher(raise_exc=RuntimeError("boom")) + + await mod.handle_topic_message(msg, dispatcher) + + placeholder.edit_text.assert_called() + last_call_text = placeholder.edit_text.call_args[0][0] + assert "недоступен" in last_call_text or "ошибка" in last_call_text.lower() + + +async def test_stream_success_edits_placeholder(fresh_db): + """When stream_message succeeds, the placeholder is updated with the response.""" + fresh_db.create_chat(1, 42, "Чат #1") + import adapter.telegram.handlers.message as mod + importlib.reload(mod) + + chunks = [SimpleNamespace(delta="Hello "), SimpleNamespace(delta="world")] + msg, placeholder = make_message() + dispatcher = make_dispatcher(chunks=chunks) + + await mod.handle_topic_message(msg, dispatcher) + + placeholder.edit_text.assert_called() + last_call_text = placeholder.edit_text.call_args[0][0] + assert "Hello world" in last_call_text