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

Error intervals to stop flooding logs & set non_stop to true by default #2332

Merged
merged 3 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions telebot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2598,6 +2598,10 @@ def send_document(
logger.warning('The parameter "thumb" is deprecated. Use "thumbnail" instead.')
thumbnail = thumb

if isinstance(document, types.InputFile) and visible_file_name:
# inputfile name ignored, warn
logger.warning('Cannot use both InputFile and visible_file_name. InputFile name will be ignored.')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

files[key] = (value.file_name, value.file)

visible_file_name will be ignoread, isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it returns a tuple and in apihelper, tuple value[0] is unchanged, thus ignoring the inputfile naming


return types.Message.de_json(
apihelper.send_data(
self.token, chat_id, document, 'document',
Expand Down
2 changes: 1 addition & 1 deletion telebot/apihelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def _make_request(token, method_name, method='get', params=None, files=None):
# process types.InputFile
for key, value in files_copy.items():
if isinstance(value, types.InputFile):
files[key] = value.file
files[key] = (value.file_name, value.file)
elif isinstance(value, tuple) and (len(value) == 2) and isinstance(value[1], types.InputFile):
files[key] = (value[0], value[1].file)

Expand Down
29 changes: 21 additions & 8 deletions telebot/async_telebot.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def _setup_change_detector(self, path_to_watch: str) -> None:
self.event_observer.schedule(self.event_handler, path, recursive=True)
self.event_observer.start()

async def polling(self, non_stop: bool=False, skip_pending=False, interval: int=0, timeout: int=20,
async def polling(self, non_stop: bool=True, skip_pending=False, interval: int=0, timeout: int=20,
request_timeout: Optional[int]=None, allowed_updates: Optional[List[str]]=None,
none_stop: Optional[bool]=None, restart_on_change: Optional[bool]=False, path_to_watch: Optional[str]=None):
"""
Expand All @@ -257,11 +257,6 @@ async def polling(self, non_stop: bool=False, skip_pending=False, interval: int=

Always gets updates.

.. note::

Set non_stop=True if you want your bot to continue receiving updates
if there is an error.

.. note::

Install watchdog and psutil before using restart_on_change option.
Expand Down Expand Up @@ -393,6 +388,15 @@ def __hide_token(self, message: str) -> str:
return message.replace(code, "*" * len(code))
else:
return message

async def _handle_error_interval(self, error_interval: float):
logger.debug('Waiting for %s seconds before retrying', error_interval)
await asyncio.sleep(error_interval)
if error_interval * 2 < 60: # same logic as sync
error_interval *= 2
else:
error_interval = 60
return error_interval

async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout: int=20,
request_timeout: int=None, allowed_updates: Optional[List[str]]=None):
Expand Down Expand Up @@ -426,16 +430,18 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout:

self._polling = True

error_interval = 0.25

try:
while self._polling:
try:

updates = await self.get_updates(offset=self.offset, allowed_updates=allowed_updates, timeout=timeout, request_timeout=request_timeout)
if updates:
self.offset = updates[-1].update_id + 1
# noinspection PyAsyncCall
asyncio.create_task(self.process_new_updates(updates)) # Seperate task for processing updates
if interval: await asyncio.sleep(interval)
error_interval = 0.25 # drop error_interval if no errors

except KeyboardInterrupt:
return
Expand All @@ -446,9 +452,10 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout:
if not handled:
logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
logger.debug(self.__hide_token(traceback.format_exc()))
error_interval = await self._handle_error_interval(error_interval)

if non_stop or handled:
await asyncio.sleep(2)
#await asyncio.sleep(2) # used error_interval instead
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed behaviour!

In old version sleep(2) is for non_stop or handled. In new version it is for not hadled and independently from non_stop.

  1. Adding for not handled is fine.
  2. Removing from handled is not fine, because handled exceptions will cause now immediate re-run instead of 2 second delay.
  3. Make sleep() for non_stop looks also not needed.

I propose:

                    if not handled:
                        logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
                        logger.debug(self.__hide_token(traceback.format_exc()))

                    if non_stop:
                        error_interval = await self._handle_error_interval(error_interval)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we still don't need sleep for handled? or should I add sleep to handled (2 seconds) back?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to sleep both for handled (as before) and not handled (it's reasonable).

But I propose only if non_stop: otherwise sleeping is useless, because polling will stop anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, before, script slept 2 seconds for handled or non_stop. So, should I:

  • Change your last condition to "if non_stop or handled"?
  • Add another condition for handled, making the script sleep for 2 seconds?

Copy link
Collaborator Author

@coder2020official coder2020official Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not affect not handled: there will be no sleeping as you said, the sleep will be useless. currently we are talking about handled. Or, do you think the user should manually do that and not rely on internal behaviour?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come on. All I proposed is written here.
image
What else are you trying to get from me now? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am talking about the need to have "if handled" condition.
In previous version, as you know, sleep is triggered on handled or non_stop. After I copy your suggestion, sleep is not triggered for handled. is that fine?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I copy your suggestion, sleep is not triggered for handled. is that fine?

It is not. But if you copy my version - it will work both for handled and not handled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I copy your suggestion, sleep is not triggered for handled. is that fine?

It is not.

I don't know what you did, but if you'll take my version - it will work both for handled and not handled. Isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, sorry
was sleepy a little, should work, yep

continue
else:
return
Expand All @@ -457,6 +464,7 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout:
if not handled:
logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
logger.debug(self.__hide_token(traceback.format_exc()))
error_interval = await self._handle_error_interval(error_interval)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here.

                    if not handled:
                        logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
                        logger.debug(self.__hide_token(traceback.format_exc()))

                    if non_stop:
                        error_interval = await self._handle_error_interval(error_interval)

                    if non_stop or handled:
                        continue


if non_stop or handled:
continue
Expand All @@ -467,6 +475,7 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout:
if not handled:
logger.error('Unhandled exception (full traceback for debug level): %s', str(e))
logger.debug(traceback.format_exc())
error_interval = await self._handle_error_interval(error_interval)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here.


if non_stop or handled:
continue
Expand Down Expand Up @@ -4014,6 +4023,10 @@ async def send_document(
if reply_parameters and (reply_parameters.allow_sending_without_reply is None):
reply_parameters.allow_sending_without_reply = self.allow_sending_without_reply

if isinstance(document, types.InputFile) and visible_file_name:
# inputfile name ignored, warn
logger.warning('Cannot use both InputFile and visible_file_name. InputFile name will be ignored.')

return types.Message.de_json(
await asyncio_helper.send_data(
self.token, chat_id, document, 'document',
Expand Down
2 changes: 2 additions & 0 deletions telebot/asyncio_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ def _prepare_data(params=None, files=None):
if isinstance(f, tuple):
if len(f) == 2:
file_name, file = f
if isinstance(file, types.InputFile):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot verify this, never worked with InputFile.

file = file.file
else:
raise ValueError('Tuple must have exactly 2 elements: filename, fileobj')
elif isinstance(f, types.InputFile):
Expand Down
14 changes: 12 additions & 2 deletions telebot/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -7735,8 +7735,11 @@ class InputFile:
InputFile(pathlib.Path('/path/to/file/file.txt'))
)
"""
def __init__(self, file) -> None:
self._file, self.file_name = self._resolve_file(file)
def __init__(self, file: Union[str, IOBase, Path], file_name: Optional[str] = None):
self._file, self._file_name = self._resolve_file(file)
if file_name:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not )

self._file_name = file_name


@staticmethod
def _resolve_file(file):
Expand All @@ -7757,6 +7760,13 @@ def file(self):
File object.
"""
return self._file

@property
def file_name(self):
"""
File name.
"""
return self._file_name


class ForumTopicCreated(JsonDeserializable):
Expand Down
Loading