Refactoring telegram bridge #43
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
Comment on lines
+45
to
+46
| sender = await max_client.get_user(user_id=message.sender) # pyright: ignore[reportArgumentType] | ||
|
|
Contributor
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.
Comment on lines
+49
to
+73
| 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}") |
Contributor
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.
Comment on lines
+75
to
+118
| 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}") |
Contributor
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.
Contributor
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
Comment on lines
+28
to
+35
| 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 |
Contributor
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.
Author
|
На счёт всех проблем связанных с None хз, по идее их возникнуть не должно, т.к. если есть файл, фото или видео - библиотека должна вернуть уж точно не None. Ну и с отправителем по аналогии (если пришло сообщение он должен быть). Так что я не уверен на счёт добавления проверок на None. В изначальной версии их тоже не было. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Описание
Сделал рефакторинг 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.