fix(tg): reviewer fixes — error handling, timeouts, db index
- 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)
This commit is contained in:
parent
c95360ce1f
commit
8901e60f6a
7 changed files with 161 additions and 25 deletions
|
|
@ -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);
|
||||
""")
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
87
tests/adapter/telegram/test_message.py
Normal file
87
tests/adapter/telegram/test_message.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue