fix: мелкие исправления надёжности#92
Conversation
- webhook/base.py: предупреждение при запуске без secret - exceptions/dispatcher.py: __repr__ = __str__ для HandlerException и MiddlewareException (защита memory_context от утечки через %r) - methods/subscribe_webhook.py: предупреждение при подписке на HTTP URL - bot.py: __repr__ возвращает Bot(token='***') для защиты токена в логах Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Исправлена проверка marker=0 в пагинации (get_chats, get_members_chat) - Обёрнуты вызовы get_chat_member в contextlib.suppress в _resolve_from_user - Добавлен флаг _ready для идемпотентности __ready в диспетчере - Убран избыточный list comprehension в фильтре Command Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| event.from_user = await bot.get_chat_member( | ||
| chat_id=event.chat_id, user_id=event.user_id | ||
| ) | ||
| with contextlib.suppress(Exception): |
There was a problem hiding this comment.
Слишком общее исключение - лучше ограничить исключениями данного приложения. Не понятна мотивация заглушения ошибки - насколько мы хотим подавить ошибку? Может пусть лучше явно возникнет? Или пусть как минимум оставит следы?
Если это влияет выше на потерю батча обновлений - то надо это проблему выше решать, но текущее обновление ожидаемо должно явно упасть с ошибкой
Заменяет contextlib.suppress(Exception) на явный except (MaxApiError, MaxConnection) с logger.warning(), чтобы ошибки оставляли след в логах вместо молчаливого подавления.
There was a problem hiding this comment.
Pull request overview
PR направлен на повышение надёжности и предсказуемости поведения SDK: исправляет пограничные случаи пагинации, делает обработку обновлений устойчивее к ошибкам API, предотвращает повторную “инициализацию” диспетчера и убирает лишние аллокации в фильтре команд.
Changes:
- Исправлена обработка
marker=0в методах пагинации. - Добавлена защита
try/exceptвокругget_chat_member()при enrich’е событий обновлений. - Сделан
startup()/__ready()идемпотентным и оптимизированCommand-фильтр.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils/test_message_media_upload.py | Адаптация теста под изменение поведения закрытия сессии при ошибке upload token. |
| pyproject.toml | Добавлен per-file ignore C90 для maxapi/utils/updates.py. |
| maxapi/webhook/base.py | Предупреждение при запуске webhook без secret. |
| maxapi/utils/updates.py | Оборачивание get_chat_member() в try/except + логирование. |
| maxapi/utils/message.py | Убрано закрытие bot.session при отсутствии upload token для video/audio. |
| maxapi/methods/subscribe_webhook.py | Предупреждение при URL вебхука без HTTPS. |
| maxapi/methods/get_members_chat.py | Корректная проверка marker is not None. |
| maxapi/methods/get_chats.py | Корректная проверка marker is not None. |
| maxapi/filters/command.py | Удалена лишняя аллокация списка при case-insensitive проверке команды. |
| maxapi/exceptions/dispatcher.py | __repr__ приравнен к __str__ для исключений диспетчера/мидлварей. |
| maxapi/dispatcher.py | Добавлен флаг _ready для защиты от повторной инициализации; сброс в stop_polling(). |
| maxapi/connection/base.py | Более корректный content_type через mimetypes.guess_type и timeout для временных ClientSession. |
| maxapi/bot.py | Безопасный __repr__ без утечки токена. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self._ready: | ||
| return | ||
| self._ready = True | ||
|
|
There was a problem hiding this comment.
Флаг _ready выставляется в True до выполнения потенциально падающих шагов (например, check_me(), _prepare_handlers(), on_started_func). Если внутри __ready() произойдёт исключение, диспетчер останется в частично инициализированном состоянии, а повторный startup()/start_polling() будет сразу возвращаться из-за _ready=True. Лучше выставлять _ready=True только после успешной инициализации или сбрасывать флаг в except/finally при ошибке.
| try: | ||
| event.from_user = await bot.get_chat_member( | ||
| chat_id=event.chat_id, user_id=event.user_id | ||
| ) | ||
| except (MaxApiError, MaxConnection) as exc: | ||
| logger.warning("Не удалось получить участника чата: %s", exc) |
There was a problem hiding this comment.
Логирование исключения через %s вызывает MaxApiError.__str__, который включает raw ответ API. Это может писать в логи большой payload и потенциально персональные данные. Лучше логировать безопасный минимум (например, code и контекст chat_id/user_id) без raw, либо вынести raw на debug-уровень.
| try: | ||
| event.from_user = await bot.get_chat_member( | ||
| chat_id=event.chat_id, user_id=event.admin_id | ||
| ) | ||
| except (MaxApiError, MaxConnection) as exc: | ||
| logger.warning("Не удалось получить участника чата: %s", exc) |
There was a problem hiding this comment.
Здесь та же проблема с логированием: MaxApiError.__str__ включает raw, что может приводить к утечке/зашумлению логов. Рекомендуется логировать только code/тип события и идентификаторы (chat_id, admin_id) без полного raw ответа, либо логировать raw только на debug.
| if not url.startswith("https://"): | ||
| warnings.warn( | ||
| "URL вебхука не использует HTTPS. " | ||
| "Обновления будут передаваться по незашифрованному каналу.", | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
Проверка not url.startswith("https://") срабатывает также для невалидных URL (например, без схемы или с ftp://), но текст предупреждения говорит только про отсутствие HTTPS. Либо валидируйте, что URL начинается с http:// или https:// (и поднимайте ValueError при другом), либо ограничьте предупреждение случаем url.startswith("http://").
| self.secret = secret | ||
| if self.secret is None: | ||
| logger_dp.warning( | ||
| "Webhook запущен без secret. " | ||
| "Рекомендуется установить secret для защиты " | ||
| "от поддельных обновлений." | ||
| ) |
There was a problem hiding this comment.
Сейчас предупреждение выводится только когда secret is None. Если передать пустую строку secret="", проверки секрета фактически не будет (заголовок можно прислать пустым), но предупреждения не появится. Стоит либо валидировать secret (как минимум if not secret: ...), либо нормализовать пустую строку к None.
| """ | ||
| if self.polling: | ||
| self.polling = False | ||
| self._ready = False |
There was a problem hiding this comment.
stop_polling() сбрасывает _ready=False, поэтому последующий start_polling() снова зайдёт в __ready() и выполнит self.routers += [self]. Поскольку список routers здесь не очищается, при циклах stop/start в одном процессе будут накапливаться дубликаты роутеров (и потенциально лишние предупреждения/рост памяти). Либо не сбрасывайте _ready без очистки/восстановления routers, либо сделайте добавление self в routers идемпотентным (не добавлять, если уже есть).
| self._ready = False |
- updates.py: split except (MaxApiError, MaxConnection) into separate clauses; log only exc.code and chat_id (not raw) to avoid PII leakage via MaxApiError.__str__ which includes the raw API response - dispatcher.py: moved _ready=True to after successful init completion (after check_me, _prepare_handlers, on_started_func) so partial init failure doesn't leave the dispatcher in a permanently-ready state - dispatcher.py: made self.routers addition idempotent (if self not in self.routers) to prevent duplicates on stop+restart cycles - webhook/base.py: changed 'secret is None' to 'not secret' so that empty string '' also triggers the missing-secret warning - subscribe_webhook.py: narrowed URL check from 'not https://' to 'http://' so only plaintext HTTP triggers the warning (not wss:// etc) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers all 8 code paths flagged by Codecov: - utils/updates.py: MaxApiError and MaxConnection in _resolve_from_user - dispatcher.py: _ready flag prevents double init - methods/subscribe_webhook.py: warns on http:// URL - filters/command.py: case-insensitive command match - methods/get_chats.py: marker=0 handled via `is not None` - methods/get_members_chat.py: marker=0 handled via `is not None` - connection/base.py: temp ClientSession + mimetypes.guess_extension - bot.py: __repr__ does not leak token Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- I001: отсортированы импорты - F401: удалены неиспользуемые импорты (Bot, Dispatcher, logging) - E501: разбиты длинные строки (docstrings переведены на русский, lambda вынесена в обычную функцию, patch разбит на несколько строк) - ruff format: убрана лишняя пустая строка в test_message_media_upload.py
Добавлен класс TestBaseMaxWebhookSecretWarning в tests/test_coverage_gaps.py с тремя кейсами для проверки `if not self.secret` в webhook/base.py:50: - secret=None — warning должен сработать - secret="" — warning должен сработать (edge case, который и был причиной перехода с `is None` на `not self.secret` в love-apples#92) - secret="my-secret" — warning должен молчать Покрывает оставшийся непокрытый путь после слияния PR love-apples#92.
Добавлен класс TestBaseMaxWebhookSecretWarning в tests/test_coverage_gaps.py с тремя кейсами для проверки `if not self.secret` в webhook/base.py:50: - secret=None — warning должен сработать - secret="" — warning должен сработать (edge case, который и был причиной перехода с `is None` на `not self.secret` в #92) - secret="my-secret" — warning должен молчать Покрывает оставшийся непокрытый путь после слияния PR #92.
Описание
1. Исправлена проверка
markerв пагинацииФайлы:
methods/get_chats.py,methods/get_members_chat.pyif self.marker:пропускалmarker=0. Вget_updates.pyмаркер уже проверялся корректно черезif self.marker is not None:— приведено к единообразию.2. Защита от исключений в
_resolve_from_userФайл:
utils/updates.pybot.get_chat_member()вызывался безtry/except. Если пользователь уже покинул чат к моменту обработкиMessageRemoved, API возвращал ошибку, что приводило к потере всего батча обновлений.3. Идемпотентность
__readyв диспетчереФайл:
dispatcher.pyПри повторном вызове
startup()(например, FastAPI lifespan + ручной вызов)on_started_funcвызывался дважды, аself.routersнакапливал дубликаты. Добавлен флаг_readyдля защиты от повторного входа.4. Убран избыточный list comprehension в фильтре
CommandФайл:
filters/command.py[commands.lower() for commands in self.commands]создавался на каждое сообщение, хотяself.commandsуже в нижнем регистре после__init__. Убрана лишняя аллокация.Тестирование