diff --git a/telegram/ext/dispatcher.py b/telegram/ext/dispatcher.py index 55c1485202b..f33126e4c6e 100644 --- a/telegram/ext/dispatcher.py +++ b/telegram/ext/dispatcher.py @@ -62,7 +62,8 @@ class DispatcherHandlerStop(Exception): """ - Raise this in handler to prevent execution of any other handler (even in different group). + Raise this in a handler or an error handler to prevent execution of any other handler (even in + different group). In order to use this exception in a :class:`telegram.ext.ConversationHandler`, pass the optional ``state`` parameter instead of returning the next state: @@ -73,6 +74,9 @@ def callback(update, context): ... raise DispatcherHandlerStop(next_state) + Note: + Has no effect, if the handler or error handler is run asynchronously. + Attributes: state (:obj:`object`): Optional. The next state of the conversation. @@ -320,15 +324,16 @@ def _pooled(self) -> None: # Avoid infinite recursion of error handlers. if promise.pooled_function in self.error_handlers: - self.logger.error('An uncaught error was raised while handling the error.') + self.logger.exception( + 'An error was raised and an uncaught error was raised while ' + 'handling the error with an error_handler.', + exc_info=promise.exception, + ) continue # If we arrive here, an exception happened in the promise and was neither # DispatcherHandlerStop nor raised by an error handler. So we can and must handle it - try: - self.dispatch_error(promise.update, promise.exception, promise=promise) - except Exception: - self.logger.exception('An uncaught error was raised while handling the error.') + self.dispatch_error(promise.update, promise.exception, promise=promise) def run_async( self, func: Callable[..., object], *args: object, update: object = None, **kwargs: object @@ -452,10 +457,7 @@ def process_update(self, update: object) -> None: """ # An error happened while polling if isinstance(update, TelegramError): - try: - self.dispatch_error(None, update) - except Exception: - self.logger.exception('An uncaught error was raised while handling the error.') + self.dispatch_error(None, update) return context = None @@ -483,14 +485,9 @@ def process_update(self, update: object) -> None: # Dispatch any error. except Exception as exc: - try: - self.dispatch_error(update, exc) - except DispatcherHandlerStop: - self.logger.debug('Error handler stopped further handlers') + if self.dispatch_error(update, exc): + self.logger.debug('Error handler stopped further handlers.') break - # Errors should not stop the thread. - except Exception: - self.logger.exception('An uncaught error was raised while handling the error.') # Update persistence, if handled handled_only_async = all(sync_modes) @@ -606,56 +603,24 @@ def __update_persistence(self, update: object = None) -> None: self.bot.callback_data_cache.persistence_data ) except Exception as exc: - try: - self.dispatch_error(update, exc) - except Exception: - message = ( - 'Saving callback data raised an error and an ' - 'uncaught error was raised while handling ' - 'the error with an error_handler' - ) - self.logger.exception(message) + self.dispatch_error(update, exc) if self.persistence.store_data.bot_data: try: self.persistence.update_bot_data(self.bot_data) except Exception as exc: - try: - self.dispatch_error(update, exc) - except Exception: - message = ( - 'Saving bot data raised an error and an ' - 'uncaught error was raised while handling ' - 'the error with an error_handler' - ) - self.logger.exception(message) + self.dispatch_error(update, exc) if self.persistence.store_data.chat_data: for chat_id in chat_ids: try: self.persistence.update_chat_data(chat_id, self.chat_data[chat_id]) except Exception as exc: - try: - self.dispatch_error(update, exc) - except Exception: - message = ( - 'Saving chat data raised an error and an ' - 'uncaught error was raised while handling ' - 'the error with an error_handler' - ) - self.logger.exception(message) + self.dispatch_error(update, exc) if self.persistence.store_data.user_data: for user_id in user_ids: try: self.persistence.update_user_data(user_id, self.user_data[user_id]) except Exception as exc: - try: - self.dispatch_error(update, exc) - except Exception: - message = ( - 'Saving user data raised an error and an ' - 'uncaught error was raised while handling ' - 'the error with an error_handler' - ) - self.logger.exception(message) + self.dispatch_error(update, exc) def add_error_handler( self, @@ -663,15 +628,12 @@ def add_error_handler( run_async: Union[bool, DefaultValue] = DEFAULT_FALSE, # pylint: disable=W0621 ) -> None: """Registers an error handler in the Dispatcher. This handler will receive every error - which happens in your bot. + which happens in your bot. See the docs of :meth:`dispatch_error` for more details on how + errors are handled. Note: Attempts to add the same callback multiple times will be ignored. - Warning: - The errors handled within these handlers won't show up in the logger, so you - need to make sure that you reraise the error. - Args: callback (:obj:`callable`): The callback function for this error handler. Will be called when an error is raised. @@ -700,9 +662,21 @@ def remove_error_handler(self, callback: Callable[[object, CCT], None]) -> None: self.error_handlers.pop(callback, None) def dispatch_error( - self, update: Optional[object], error: Exception, promise: Promise = None - ) -> None: - """Dispatches an error. + self, + update: Optional[object], + error: Exception, + promise: Promise = None, + ) -> bool: + """Dispatches an error by passing it to all error handlers registered with + :meth:`add_error_handler`. If one of the error handlers raises + :class:`telegram.ext.DispatcherHandlerStop`, the update will not be handled by other error + handlers or handlers (even in other groups). All other exceptions raised by an error + handler will just be logged. + + .. versionchanged:: 14.0 + * Exceptions raised by error handlers are now properly logged. + * :class:`telegram.ext.DispatcherHandlerStop` is no longer reraised but converted into + the return value. Args: update (:obj:`object` | :class:`telegram.Update`): The update that caused the error. @@ -710,6 +684,9 @@ def dispatch_error( promise (:class:`telegram.utils.Promise`, optional): The promise whose pooled function raised the error. + Returns: + :obj:`bool`: :obj:`True` if one of the error handlers raised + :class:`telegram.ext.DispatcherHandlerStop`. :obj:`False`, otherwise. """ async_args = None if not promise else promise.args async_kwargs = None if not promise else promise.kwargs @@ -722,9 +699,19 @@ def dispatch_error( if run_async: self.run_async(callback, update, context, update=update) else: - callback(update, context) + try: + callback(update, context) + except DispatcherHandlerStop: + return True + except Exception as exc: + self.logger.exception( + 'An error was raised and an uncaught error was raised while ' + 'handling the error with an error_handler.', + exc_info=exc, + ) + return False - else: - self.logger.exception( - 'No error handlers are registered, logging exception.', exc_info=error - ) + self.logger.exception( + 'No error handlers are registered, logging exception.', exc_info=error + ) + return False diff --git a/telegram/ext/jobqueue.py b/telegram/ext/jobqueue.py index 444ebe22c3f..ac255ad355b 100644 --- a/telegram/ext/jobqueue.py +++ b/telegram/ext/jobqueue.py @@ -73,15 +73,7 @@ def _update_persistence(self, _: JobEvent) -> None: self._dispatcher.update_persistence() def _dispatch_error(self, event: JobEvent) -> None: - try: - self._dispatcher.dispatch_error(None, event.exception) - # Errors should not stop the thread. - except Exception: - self.logger.exception( - 'An error was raised while processing the job and an ' - 'uncaught error was raised while handling the error ' - 'with an error_handler.' - ) + self._dispatcher.dispatch_error(None, event.exception) @overload def _parse_time_input(self, time: None, shift_day: bool = False) -> None: @@ -524,15 +516,7 @@ def run(self, dispatcher: 'Dispatcher') -> None: try: self.callback(dispatcher.context_types.context.from_job(self, dispatcher)) except Exception as exc: - try: - dispatcher.dispatch_error(None, exc) - # Errors should not stop the thread. - except Exception: - dispatcher.logger.exception( - 'An error was raised while processing the job and an ' - 'uncaught error was raised while handling the error ' - 'with an error_handler.' - ) + dispatcher.dispatch_error(None, exc) def schedule_removal(self) -> None: """ diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 11e766f60ce..de83d73cefb 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -298,7 +298,9 @@ def test_async_handler_error_handler_that_raises_error(self, dp, caplog): dp.update_queue.put(self.message_update) sleep(0.1) assert len(caplog.records) == 1 - assert caplog.records[-1].getMessage().startswith('An uncaught error was raised') + assert ( + caplog.records[-1].getMessage().startswith('An error was raised and an uncaught') + ) # Make sure that the main loop still runs dp.remove_handler(handler) @@ -316,7 +318,9 @@ def test_async_handler_async_error_handler_that_raises_error(self, dp, caplog): dp.update_queue.put(self.message_update) sleep(0.1) assert len(caplog.records) == 1 - assert caplog.records[-1].getMessage().startswith('An uncaught error was raised') + assert ( + caplog.records[-1].getMessage().startswith('An error was raised and an uncaught') + ) # Make sure that the main loop still runs dp.remove_handler(handler) @@ -631,7 +635,7 @@ def test_sensible_worker_thread_names(self, dp2): for thread_name in thread_names: assert thread_name.startswith(f"Bot:{dp2.bot.id}:worker:") - def test_error_while_persisting(self, dp, monkeypatch): + def test_error_while_persisting(self, dp, caplog): class OwnPersistence(BasePersistence): def update(self, data): raise Exception('PersistenceError') @@ -681,27 +685,30 @@ def flush(self): def callback(update, context): pass - test_flag = False + test_flag = [] def error(update, context): nonlocal test_flag - test_flag = str(context.error) == 'PersistenceError' + test_flag.append(str(context.error) == 'PersistenceError') raise Exception('ErrorHandlingError') - def logger(message): - assert 'uncaught error was raised while handling' in message - update = Update( 1, message=Message(1, None, Chat(1, ''), from_user=User(1, '', False), text='Text') ) handler = MessageHandler(Filters.all, callback) dp.add_handler(handler) dp.add_error_handler(error) - monkeypatch.setattr(dp.logger, 'exception', logger) dp.persistence = OwnPersistence() - dp.process_update(update) - assert test_flag + + with caplog.at_level(logging.ERROR): + dp.process_update(update) + + assert test_flag == [True, True, True, True] + assert len(caplog.records) == 4 + for record in caplog.records: + message = record.getMessage() + assert message.startswith('An error was raised and an uncaught') def test_persisting_no_user_no_chat(self, dp): class OwnPersistence(BasePersistence): diff --git a/tests/test_jobqueue.py b/tests/test_jobqueue.py index 67e6242b5e4..cfeb94a30b0 100644 --- a/tests/test_jobqueue.py +++ b/tests/test_jobqueue.py @@ -449,15 +449,13 @@ def test_dispatch_error_that_raises_errors(self, job_queue, dp, caplog): sleep(0.1) assert len(caplog.records) == 1 rec = caplog.records[-1] - assert 'processing the job' in rec.getMessage() - assert 'uncaught error was raised while handling' in rec.getMessage() + assert 'An error was raised and an uncaught' in rec.getMessage() caplog.clear() with caplog.at_level(logging.ERROR): job.run(dp) assert len(caplog.records) == 1 rec = caplog.records[-1] - assert 'processing the job' in rec.getMessage() assert 'uncaught error was raised while handling' in rec.getMessage() caplog.clear()