From 87bacb650eb33fb6bde4d9fcb96b0f455eeb9757 Mon Sep 17 00:00:00 2001 From: Selwin Ong Date: Fri, 1 Jul 2022 09:27:17 +0700 Subject: [PATCH] Jobs that are run synchronously should always raise an exception (#1671) --- rq/queue.py | 1 + tests/test_callbacks.py | 30 +++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/rq/queue.py b/rq/queue.py index 18ce35c..c8f5abf 100644 --- a/rq/queue.py +++ b/rq/queue.py @@ -595,6 +595,7 @@ nd job.set_status(JobStatus.FAILED) if job.failure_callback: job.failure_callback(job, self.connection, *sys.exc_info()) + raise else: if job.success_callback: job.success_callback(job, self.connection, job.result) diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index 8d8edc7..d115a6a 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -1,4 +1,5 @@ from datetime import timedelta +from uuid import uuid4 from tests import RQTestCase from tests.fixtures import div_by_zero, erroneous_callback, save_exception, save_result, say_hello @@ -59,7 +60,13 @@ class SyncJobCallback(RQTestCase): job.result ) - job = queue.enqueue(div_by_zero, on_success=save_result) + # Callback is not executed when job fails + job_id = str(uuid4()) + try: + job = queue.enqueue(div_by_zero, on_success=save_result, job_id=job_id) + except TypeError: + pass + job = Job.fetch(id=job_id) self.assertEqual(job.get_status(), JobStatus.FAILED) self.assertFalse(self.testconn.exists('success_callback:%s' % job.id)) @@ -67,15 +74,32 @@ class SyncJobCallback(RQTestCase): """queue.enqueue* methods with on_failure is persisted correctly""" queue = Queue(is_async=False) - job = queue.enqueue(div_by_zero, on_failure=save_exception) + job_id = str(uuid4()) + try: + job = queue.enqueue(div_by_zero, on_failure=save_exception, job_id=job_id) + except: + pass + job = Job.fetch(id=job_id) self.assertEqual(job.get_status(), JobStatus.FAILED) self.assertIn('div_by_zero', self.testconn.get('failure_callback:%s' % job.id).decode()) - job = queue.enqueue(div_by_zero, on_success=save_result) + # If there's no failure callback, exception should be raised + job_id = str(uuid4()) + with self.assertRaises(TypeError): + job = queue.enqueue(div_by_zero, on_success=save_result, job_id=job_id) + job = Job.fetch(id=job_id) self.assertEqual(job.get_status(), JobStatus.FAILED) self.assertFalse(self.testconn.exists('failure_callback:%s' % job.id)) + # If failure callback is specified, exception is raised after callback is executed + job_id = str(uuid4()) + with self.assertRaises(TypeError): + job = queue.enqueue(div_by_zero, on_failure=save_exception, job_id=job_id) + job = Job.fetch(id=job_id) + self.assertEqual(job.get_status(), JobStatus.FAILED) + self.assertTrue(self.testconn.exists('failure_callback:%s' % job.id)) + class WorkerCallbackTestCase(RQTestCase): def test_success_callback(self):