Skip to content

Pass job to error handler where possible #2570

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Pass job to error handler where possible #2570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions telegram/ext/callbackcontext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class CallbackContext(Generic[UD, CD, BD]):
that raised the error. Only present when the raising function was run asynchronously
using :meth:`telegram.ext.Dispatcher.run_async`.
job (:class:`telegram.ext.Job`): Optional. The job which originated this callback.
Only present when passed to the callback of :class:`telegram.ext.Job`.
Only present when passed to the callback of :class:`telegram.ext.Job` or in error
handlers if the error is caused by a `repeating` job.

"""

Expand Down Expand Up @@ -231,6 +232,7 @@ def from_error(
dispatcher: 'Dispatcher',
async_args: Union[List, Tuple] = None,
async_kwargs: Dict[str, object] = None,
job: 'Job' = None,
) -> CC:
"""
Constructs an instance of :class:`telegram.ext.CallbackContext` to be passed to the error
Expand All @@ -244,12 +246,15 @@ def from_error(
error (:obj:`Exception`): The error.
dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher associated with this
context.
async_args (List[:obj:`object`]): Optional. Positional arguments of the function that
async_args (List[:obj:`object`], optional): Positional arguments of the function that
raised the error. Pass only when the raising function was run asynchronously using
:meth:`telegram.ext.Dispatcher.run_async`.
async_kwargs (Dict[:obj:`str`, :obj:`object`]): Optional. Keyword arguments of the
async_kwargs (Dict[:obj:`str`, :obj:`object`], optional): Keyword arguments of the
function that raised the error. Pass only when the raising function was run
asynchronously using :meth:`telegram.ext.Dispatcher.run_async`.
job (:class:`telegram.ext.Job`, optional): The job associated with the error.

.. versionadded:: 13.6.1

Returns:
:class:`telegram.ext.CallbackContext`
Expand All @@ -258,6 +263,7 @@ def from_error(
self.error = error
self.async_args = async_args
self.async_kwargs = async_kwargs
self.job = job
return self

@classmethod
Expand Down
18 changes: 15 additions & 3 deletions telegram/ext/dispatcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@

if TYPE_CHECKING:
from telegram import Bot
from telegram.ext import JobQueue
from telegram.ext import JobQueue, Job

DEFAULT_GROUP: int = 0

Expand Down Expand Up @@ -784,7 +784,11 @@ 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
self,
update: Optional[object],
error: Exception,
promise: Promise = None,
job: 'Job' = None,
) -> None:
"""Dispatches an error.

Expand All @@ -793,6 +797,9 @@ def dispatch_error(
error (:obj:`Exception`): The error that was raised.
promise (:class:`telegram.utils.Promise`, optional): The promise whose pooled function
raised the error.
job (:class:`telegram.ext.Job`, optional): The job that caused the error.

.. versionadded:: 13.6.1

"""
async_args = None if not promise else promise.args
Expand All @@ -802,7 +809,12 @@ def dispatch_error(
for callback, run_async in self.error_handlers.items(): # pylint: disable=W0621
if self.use_context:
context = self.context_types.context.from_error(
update, error, self, async_args=async_args, async_kwargs=async_kwargs
update,
error,
self,
async_args=async_args,
async_kwargs=async_kwargs,
job=job,
)
if run_async:
self.run_async(callback, update, context, update=update)
Expand Down
6 changes: 4 additions & 2 deletions telegram/ext/jobqueue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ def _update_persistence(self, _: JobEvent) -> None:

def _dispatch_error(self, event: JobEvent) -> None:
try:
self._dispatcher.dispatch_error(None, event.exception)
aps_job = self.scheduler.get_job(event.job_id)
job = Job._from_aps_job(aps_job, self) if aps_job else None # pylint: disable=W0212
self._dispatcher.dispatch_error(None, event.exception, job=job)
# Errors should not stop the thread.
except Exception:
self.logger.exception(
Expand Down Expand Up @@ -591,7 +593,7 @@ def run(self, dispatcher: 'Dispatcher') -> None:
self.callback(dispatcher.bot, self) # type: ignore[arg-type,call-arg]
except Exception as exc:
try:
dispatcher.dispatch_error(None, exc)
dispatcher.dispatch_error(None, exc, job=self)
# Errors should not stop the thread.
except Exception:
dispatcher.logger.exception(
Expand Down
21 changes: 15 additions & 6 deletions tests/test_jobqueue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def error_handler(self, bot, update, error):
self.received_error = str(error)

def error_handler_context(self, update, context):
self.received_error = str(context.error)
self.received_error = (str(context.error), context.job)

def error_handler_raise_error(self, *args):
raise Exception('Failing bigly')
Expand Down Expand Up @@ -466,15 +466,24 @@ def test_dispatch_error(self, job_queue, dp):
job.run(dp)
assert self.received_error is None

def test_dispatch_error_context(self, job_queue, cdp):
@pytest.mark.parametrize('repeating', [True, False])
def test_dispatch_error_context(self, job_queue, cdp, repeating):
cdp.add_error_handler(self.error_handler_context)

job = job_queue.run_once(self.job_with_exception, 0.05)
sleep(0.1)
assert self.received_error == 'Test Error'
if repeating:
job = job_queue.run_repeating(self.job_with_exception, 0.1, last=0.5)
else:
job = job_queue.run_once(self.job_with_exception, 0.1)
sleep(0.15)

assert self.received_error[0] == 'Test Error'
# For non-repeating jobs, APS already dropped the jobs before the error is dispatched
assert self.received_error[1] == (job if repeating else None)

self.received_error = None
job.run(cdp)
assert self.received_error == 'Test Error'
assert self.received_error[0] == 'Test Error'
assert self.received_error[1] is job

# Remove handler
cdp.remove_error_handler(self.error_handler_context)
Expand Down