Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ExceptionHandler handle() method as async for async telebot #2074

Merged

Conversation

oalexandere
Copy link
Contributor

@oalexandere oalexandere commented Nov 8, 2023

issue #2070

Description

Class ExceptionHandler has sync method handle() which is not compatible with async telebot in some cases.

Describe your tests

How did you test your change?

export CHAT_ID='' GROUP_ID=''
cd tests
pytest

As a result:
2 failed, 106 passed, 2 skipped, 1 warning in 117.93s (0:01:57)

2 failed:

FAILED test_telebot.py::TestTeleBot::test_send_file_with_filename - telebot.apihelper.ApiTelegramException: A request to the Telegram API was unsuccessful. Error code: 400. Description: Bad Request: file must be non-empty
FAILED test_telebot.py::TestTeleBot::test_chat_commands - IndexError: list index out of range

Python version:
3.10

OS:
macOS 14.1

Checklist:

  • I added/edited example on new feature/change (if exists)
  • My changes won't break backward compatibility
  • I made changes both for sync and async

@Badiboy
Copy link
Collaborator

Badiboy commented Nov 8, 2023

Looks fine for me.

@coder2020official ?

@oalexandere oalexandere force-pushed the feature/async-exception-handler branch from 4314ea3 to a1bb695 Compare November 9, 2023 07:17
@oalexandere
Copy link
Contributor Author

Added same edits (method self._handle_exception) for sync telebot just for code sameness.

@coder2020official
Copy link
Collaborator

LGTM, thanks, and sorry for taking that long; @Badiboy u can merge if you agree

@Badiboy Badiboy merged commit 9ff9e11 into eternnoir:master Nov 17, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants