Refactoring telegram bridge#43
Conversation
📝 WalkthroughWalkthroughReplaced the example's Changes
Sequence Diagram(s)sequenceDiagram
participant TelegramBot as Telegram Bot
participant Bridge as telegram_bridge.py
participant Max as MaxClient
participant Remote as RemoteFileServer
TelegramBot->>Bridge: Incoming message (user/media)
Bridge->>Max: send_message / forward to MAX (with attachment meta)
Max->>Bridge: delivery confirmation
Max->>Bridge: Incoming MAX message with attachment
Bridge->>Remote: download_file_bytes(url)
Remote-->>Bridge: file bytes + filename
Bridge->>TelegramBot: send_document/send_photo/send_video (with downloaded bytes)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@examples/telegram_bridge.py`:
- Around line 45-46: get_user() can return None so the code must guard against a
missing sender: after calling max_client.get_user(...) assign a safe sender_name
(e.g., use sender.names[0].name if sender is not None else a fallback like
f"User {message.sender}" or "Unknown") and then replace all direct accesses to
sender.names[0].name with this sender_name; update the code around the sender
assignment (the sender variable from max_client.get_user) and usages at the
later points so they use sender_name and avoid AttributeError when sender is
None.
- Around line 140-150: The KeyError can occur when looking up
chats_telegram[message.chat.id] because the try currently only wraps
max_client.send_message; move the try/except so the dictionary access is inside
the try (or check membership first) in handle_tg_message and catch KeyError to
return gracefully; also guard against None for message.from_user and
message.text (e.g., use safe defaults like empty string or skip non-text
messages) before calling max_client.send_message so you never pass None values
to send_message.
- Around line 49-73: get_video_by_id can return None but the code immediately
uses video.url, risking AttributeError; update the VideoAttach handling block
(around get_video_by_id, download_file_bytes, and telegram_bot.send_video) to
check whether video is None after calling max_client.get_video_by_id and, if so,
log or print a clear message and skip sending (continue/return) before accessing
video.url; only call download_file_bytes and operate on video_bytes when video
is not None, and ensure video_bytes.close() is executed only when it was opened.
- Around line 75-118: The photo and file branches need the same None-safety
guards as the video path: before calling download_file_bytes, verify that
attach.base_url (for PhotoAttach) is not None and that the result of
max_client.get_file_by_id (for FileAttach) is not None and has a non-None
file.url; if any are None, log/skip gracefully instead of calling
download_file_bytes. Update the PhotoAttach branch (inspect attach.base_url) and
the FileAttach branch (check the returned file and file.url from get_file_by_id)
to return/continue on None, and only call download_file_bytes,
send_photo/send_document, and close the bytes when the URL and byte-object
exist.
🧹 Nitpick comments (1)
examples/telegram_bridge.py (1)
28-35: Add timeout and handle missing filename header.Two concerns with this helper:
- No timeout on the HTTP request — could hang indefinitely on unresponsive servers.
X-File-Nameheader may be absent, leavingfile_bytes.name = None. Downstream code passes this toBufferedInputFile(filename=...), which may fail or produce unnamed files.♻️ Suggested improvement
async def download_file_bytes(url: str) -> BytesIO: """Загружает файл по URL и возвращает его в виде BytesIO.""" - async with aiohttp.ClientSession() as session: - async with session.get(url) as response: + timeout = aiohttp.ClientTimeout(total=60) + async with aiohttp.ClientSession(timeout=timeout) as session: + async with session.get(url) as response: response.raise_for_status() # Кидаем exception в случае ошибки HTTP file_bytes = BytesIO(await response.read()) # Читаем ответ в файлоподобный объект - file_bytes.name = response.headers.get("X-File-Name") # Ставим "файлу" имя из заголовков ответа + file_bytes.name = response.headers.get("X-File-Name") or "file" return file_bytes
| sender = await max_client.get_user(user_id=message.sender) # pyright: ignore[reportArgumentType] | ||
|
|
There was a problem hiding this comment.
Handle None sender to prevent AttributeError.
get_user() can return None (per API docs in user.py). The sender variable is later accessed at lines 64, 83, 109, and 122 via sender.names[0].name, which will crash if sender is None.
🐛 Suggested fix
sender = await max_client.get_user(user_id=message.sender) # pyright: ignore[reportArgumentType]
+ sender_name = sender.names[0].name if sender and sender.names else "Unknown"Then replace all sender.names[0].name usages with sender_name.
🤖 Prompt for AI Agents
In `@examples/telegram_bridge.py` around lines 45 - 46, get_user() can return None
so the code must guard against a missing sender: after calling
max_client.get_user(...) assign a safe sender_name (e.g., use
sender.names[0].name if sender is not None else a fallback like f"User
{message.sender}" or "Unknown") and then replace all direct accesses to
sender.names[0].name with this sender_name; update the code around the sender
assignment (the sender variable from max_client.get_user) and usages at the
later points so they use sender_name and avoid AttributeError when sender is
None.
| if isinstance(attach, VideoAttach): # Проверка на видео | ||
| try: | ||
| # Получаем видео из max по айди | ||
| video = await max_client.get_video_by_id( | ||
| chat_id=message.chat_id, # pyright: ignore[reportArgumentType] | ||
| message_id=message.id, | ||
| video_id=attach.video_id | ||
| ) | ||
|
|
||
| # Загружаем видео по URL | ||
| video_bytes = await download_file_bytes(video.url) # pyright: ignore[reportOptionalMemberAccess] | ||
|
|
||
| # Отправляем видео через тг | ||
| await telegram_bot.send_video( | ||
| chat_id=tg_id, | ||
| caption=f"{sender.names[0].name}: {message.text}", # pyright: ignore[reportOptionalMemberAccess] | ||
| video=types.BufferedInputFile(video_bytes.getvalue(), filename=video_bytes.name) | ||
| ) | ||
|
|
||
| video_bytes.close() # Удаляем видео из памяти | ||
|
|
||
| except aiohttp.ClientError as e: | ||
| print(f"Ошибка при загрузке видео: {e}") | ||
| except Exception as e: | ||
| print(f"Ошибка при отправке видео: {e}") |
There was a problem hiding this comment.
Check for None video before accessing .url.
get_video_by_id() returns VideoRequest | None. Line 59 accesses video.url with only a pyright suppression — if video is None, this raises AttributeError at runtime.
🐛 Suggested fix
video = await max_client.get_video_by_id(
chat_id=message.chat_id, # pyright: ignore[reportArgumentType]
message_id=message.id,
video_id=attach.video_id
)
+ if not video or not video.url:
+ print(f"Не удалось получить видео {attach.video_id}")
+ continue
# Загружаем видео по URL
- video_bytes = await download_file_bytes(video.url) # pyright: ignore[reportOptionalMemberAccess]
+ video_bytes = await download_file_bytes(video.url)🧰 Tools
🪛 Ruff (0.14.13)
72-72: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@examples/telegram_bridge.py` around lines 49 - 73, get_video_by_id can return
None but the code immediately uses video.url, risking AttributeError; update the
VideoAttach handling block (around get_video_by_id, download_file_bytes, and
telegram_bot.send_video) to check whether video is None after calling
max_client.get_video_by_id and, if so, log or print a clear message and skip
sending (continue/return) before accessing video.url; only call
download_file_bytes and operate on video_bytes when video is not None, and
ensure video_bytes.close() is executed only when it was opened.
| elif isinstance(attach, PhotoAttach): # Проверка на фото | ||
| try: | ||
| # Загружаем изображение по URL | ||
| photo_bytes = await download_file_bytes(attach.base_url) # pyright: ignore[reportOptionalMemberAccess] | ||
|
|
||
| # Отправляем фото через тг бота | ||
| await telegram_bot.send_photo( | ||
| chat_id=tg_id, | ||
| caption=f"{sender.names[0].name}: {message.text}", # pyright: ignore[reportOptionalMemberAccess] | ||
| photo=types.BufferedInputFile(photo_bytes.getvalue(), filename=photo_bytes.name) | ||
| ) | ||
|
|
||
| photo_bytes.close() # Удаляем фото из памяти | ||
|
|
||
| except aiohttp.ClientError as e: | ||
| print(f"Ошибка при загрузке изображения: {e}") | ||
| except Exception as e: | ||
| print(f"Ошибка при отправке фото: {e}") | ||
|
|
||
| elif isinstance(attach, FileAttach): # Проверка на файл | ||
| try: | ||
| # Получаем файл по айди | ||
| file = await max_client.get_file_by_id( | ||
| chat_id=message.chat_id, # pyright: ignore[reportArgumentType] | ||
| message_id=message.id, | ||
| file_id=attach.file_id | ||
| ) | ||
|
|
||
| # Загружаем файл по URL | ||
| file_bytes = await download_file_bytes(file.url) # pyright: ignore[reportOptionalMemberAccess] | ||
|
|
||
| # Отправляем файл через тг бота | ||
| await telegram_bot.send_document( | ||
| chat_id=tg_id, | ||
| caption=f"{sender.names[0].name}: {message.text}", # pyright: ignore[reportOptionalMemberAccess] | ||
| document=types.BufferedInputFile(file_bytes.getvalue(), filename=file_bytes.name) | ||
| ) | ||
|
|
||
| file_bytes.close() # Удаляем файл из памяти | ||
|
|
||
| except aiohttp.ClientError as e: | ||
| print(f"Ошибка при загрузке файла: {e}") | ||
| except Exception as e: | ||
| print(f"Ошибка при отправке файла: {e}") |
There was a problem hiding this comment.
Same None safety issues apply to photo and file handling.
- Line 78:
attach.base_urlmay beNoneforPhotoAttach. - Line 104:
file.urlrequires aNonecheck afterget_file_by_id().
Apply the same guard pattern as suggested for video:
🐛 Suggested fixes
For photo (line 78):
elif isinstance(attach, PhotoAttach): # Проверка на фото
try:
+ if not attach.base_url:
+ print(f"Нет URL для фото")
+ continue
# Загружаем изображение по URL
- photo_bytes = await download_file_bytes(attach.base_url) # pyright: ignore[reportOptionalMemberAccess]
+ photo_bytes = await download_file_bytes(attach.base_url)For file (after line 101):
file = await max_client.get_file_by_id(
chat_id=message.chat_id,
message_id=message.id,
file_id=attach.file_id
)
+ if not file or not file.url:
+ print(f"Не удалось получить файл {attach.file_id}")
+ continue
# Загружаем файл по URL
- file_bytes = await download_file_bytes(file.url) # pyright: ignore[reportOptionalMemberAccess]
+ file_bytes = await download_file_bytes(file.url)🧰 Tools
🪛 Ruff (0.14.13)
91-91: Do not catch blind exception: Exception
(BLE001)
117-117: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@examples/telegram_bridge.py` around lines 75 - 118, The photo and file
branches need the same None-safety guards as the video path: before calling
download_file_bytes, verify that attach.base_url (for PhotoAttach) is not None
and that the result of max_client.get_file_by_id (for FileAttach) is not None
and has a non-None file.url; if any are None, log/skip gracefully instead of
calling download_file_bytes. Update the PhotoAttach branch (inspect
attach.base_url) and the FileAttach branch (check the returned file and file.url
from get_file_by_id) to return/continue on None, and only call
download_file_bytes, send_photo/send_document, and close the bytes when the URL
and byte-object exist.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@examples/telegram_bridge.py`:
- Around line 28-35: The download_file_bytes function sets file_bytes.name from
response.headers.get("X-File-Name") which can be None; update
download_file_bytes to provide a sensible fallback filename when the header is
missing (e.g., derive from URL path or use a constant like "file.bin" or
preserve original extension if present) so that file_bytes.name is never None
before returning; ensure you still set file_bytes.name using the header value
when present and only use the fallback when that header is falsy.
♻️ Duplicate comments (5)
examples/telegram_bridge.py (5)
45-46: HandleNonesender to preventAttributeError.The
get_user()call can returnNone, butsender.names[0].nameis accessed at lines 64, 83, 109, and 122 without a guard. This will crash ifsenderisNone.🐛 Suggested fix
sender = await max_client.get_user(user_id=message.sender) # pyright: ignore[reportArgumentType] + sender_name = sender.names[0].name if sender and sender.names else f"User {message.sender}"Then replace all
sender.names[0].nameusages withsender_name.
49-73: AddNonecheck for video before accessing.url.
get_video_by_id()has return typeVideoRequest | None. Line 59 accessesvideo.urlwith only a pyright suppression — ifvideoisNone, this raisesAttributeErrorat runtime.🐛 Suggested fix
video = await max_client.get_video_by_id( chat_id=message.chat_id, # pyright: ignore[reportArgumentType] message_id=message.id, video_id=attach.video_id ) + if not video or not video.url: + print(f"Не удалось получить видео {attach.video_id}") + continue # Загружаем видео по URL - video_bytes = await download_file_bytes(video.url) # pyright: ignore[reportOptionalMemberAccess] + video_bytes = await download_file_bytes(video.url)
75-93: AddNonecheck forattach.base_urlbefore downloading.While
PhotoAttach.base_urlis typed asstr(non-optional), the pyright suppression suggests uncertainty. Add a guard for defensive coding.🐛 Suggested fix
elif isinstance(attach, PhotoAttach): # Проверка на фото try: + if not attach.base_url: + print("Нет URL для фото") + continue # Загружаем изображение по URL - photo_bytes = await download_file_bytes(attach.base_url) # pyright: ignore[reportOptionalMemberAccess] + photo_bytes = await download_file_bytes(attach.base_url)
94-118: AddNonecheck for file before accessing.url.
get_file_by_id()returnsFileRequest | None. Line 104 accessesfile.urlwith only a pyright suppression.🐛 Suggested fix
file = await max_client.get_file_by_id( chat_id=message.chat_id, # pyright: ignore[reportArgumentType] message_id=message.id, file_id=attach.file_id ) + if not file or not file.url: + print(f"Не удалось получить файл {attach.file_id}") + continue # Загружаем файл по URL - file_bytes = await download_file_bytes(file.url) # pyright: ignore[reportOptionalMemberAccess] + file_bytes = await download_file_bytes(file.url)
140-150: Guard againstNoneformessage.from_userandmessage.text.While the
KeyErrorfix is now properly placed,message.from_usercan beNonefor channel posts or service messages, andmessage.textcan beNonefor media-only messages. The current code will raiseAttributeErrorin these cases.🐛 Suggested fix
`@dp.message`() -async def handle_tg_message(message: types.Message, bot: Bot) -> None: +async def handle_tg_message(message: types.Message, bot: Bot) -> None: # noqa: ARG001 + if not message.from_user or not message.text: + return # Skip non-text or system messages + try: max_id = chats_telegram[message.chat.id] await max_client.send_message( chat_id=max_id, - text=f"{message.from_user.first_name}: {message.text}", # pyright: ignore[reportOptionalMemberAccess] + text=f"{message.from_user.first_name}: {message.text}", ) except KeyError: return
| async def download_file_bytes(url: str) -> BytesIO: | ||
| """Загружает файл по URL и возвращает его в виде BytesIO.""" | ||
| async with aiohttp.ClientSession() as session: | ||
| async with session.get(url) as response: | ||
| response.raise_for_status() # Кидаем exception в случае ошибки HTTP | ||
| file_bytes = BytesIO(await response.read()) # Читаем ответ в файлоподобный объект | ||
| file_bytes.name = response.headers.get("X-File-Name") # Ставим "файлу" имя из заголовков ответа | ||
| return file_bytes |
There was a problem hiding this comment.
Provide a fallback filename when header is missing.
response.headers.get("X-File-Name") returns None if the header is absent. This propagates to Telegram sends where filename=video_bytes.name could be None, potentially causing files to be sent without a recognizable name.
🐛 Suggested fix
file_bytes = BytesIO(await response.read()) # Читаем ответ в файлоподобный объект
- file_bytes.name = response.headers.get("X-File-Name") # Ставим "файлу" имя из заголовков ответа
+ file_bytes.name = response.headers.get("X-File-Name") or "file" # Ставим "файлу" имя из заголовков ответа
return file_bytes🧰 Tools
🪛 Ruff (0.14.13)
29-29: Docstring contains ambiguous е (CYRILLIC SMALL LETTER IE). Did you mean e (LATIN SMALL LETTER E)?
(RUF002)
29-29: Docstring contains ambiguous г (CYRILLIC SMALL LETTER GHE). Did you mean r (LATIN SMALL LETTER R)?
(RUF002)
29-29: Docstring contains ambiguous о (CYRILLIC SMALL LETTER O). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
🤖 Prompt for AI Agents
In `@examples/telegram_bridge.py` around lines 28 - 35, The download_file_bytes
function sets file_bytes.name from response.headers.get("X-File-Name") which can
be None; update download_file_bytes to provide a sensible fallback filename when
the header is missing (e.g., derive from URL path or use a constant like
"file.bin" or preserve original extension if present) so that file_bytes.name is
never None before returning; ensure you still set file_bytes.name using the
header value when present and only use the fallback when that header is falsy.
|
На счёт всех проблем связанных с None хз, по идее их возникнуть не должно, т.к. если есть файл, фото или видео - библиотека должна вернуть уж точно не None. Ну и с отправителем по аналогии (если пришло сообщение он должен быть). Так что я не уверен на счёт добавления проверок на None. В изначальной версии их тоже не было. |
|
Извиняюсь, я тут |
|
Вроде нормально, спасибо |
Описание
Сделал рефакторинг telegram_bridge.py из образцов. Поправил комментарии, убрал неиспользуемые импорты, вынес скачивание файла в отдельную функцию, поправил форматирование, добавил игнорирование для линтера в некоторых строках (часто ругается на возможный возврат None), добавил отлов исключения в handle_tg_message(), дал более ёмкое имя экземляру класса клиента max.
Тип изменений
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.