Swallow and log any BaseException in ImmediateBackend other than KeyboardInterrupt (#81)

Fixes #50

Author: jhthompson

django_tasks/backends/immediate.py

@@ -37,14 +37,26 @@ def enqueue(
         try:
             result = json_normalize(calling_task_func(*args, **kwargs))
             status = ResultStatus.COMPLETE
-        except Exception as e:
+        except BaseException as e:
             try:
                 result = exception_to_dict(e)
             except Exception:
                 logger.exception("Task id=%s unable to save exception", result_id)
                 result = None
+
+            # Use `.exception` to integrate with error monitoring tools (eg Sentry)
+            logger.exception(
+                "Task id=%s path=%s state=%s",
+                result_id,
+                task.module_path,
+                ResultStatus.FAILED,
+            )
             status = ResultStatus.FAILED
 
+            # If the user tried to terminate, let them
+            if isinstance(e, KeyboardInterrupt):
+                raise
+
         task_result = TaskResult[T](
             task=task,
             id=result_id,

tests/tasks.py

@@ -22,8 +22,18 @@ def calculate_meaning_of_life() -> int:
 
 
 @task()
-def failing_task() -> None:
-    raise ValueError("This task failed")
+def failing_task_value_error() -> None:
+    raise ValueError("This task failed due to ValueError")
+
+
+@task()
+def failing_task_system_exit() -> None:
+    raise SystemExit("This task failed due to SystemExit")
+
+
+@task()
+def failing_task_keyboard_interrupt() -> None:
+    raise KeyboardInterrupt("This task failed due to KeyboardInterrupt")
 
 
 @task()

tests/tests/test_database_backend.py

@@ -283,7 +283,7 @@ def test_run_enqueued_task(self) -> None:
     def test_batch_processes_all_tasks(self) -> None:
         for _ in range(3):
             test_tasks.noop_task.enqueue()
-        test_tasks.failing_task.enqueue()
+        test_tasks.failing_task_value_error.enqueue()
 
         self.assertEqual(DBTaskResult.objects.ready().count(), 4)
 
@@ -329,7 +329,7 @@ def test_process_all_queues(self) -> None:
         self.assertEqual(DBTaskResult.objects.ready().count(), 0)
 
     def test_failing_task(self) -> None:
-        result = test_tasks.failing_task.enqueue()
+        result = test_tasks.failing_task_value_error.enqueue()
         self.assertEqual(DBTaskResult.objects.ready().count(), 1)
 
         with self.assertNumQueries(8):
@@ -346,7 +346,11 @@ def test_failing_task(self) -> None:
 
         self.assertIsInstance(result.result, ValueError)
         assert result.traceback  # So that mypy knows the next line is allowed
-        self.assertTrue(result.traceback.endswith("ValueError: This task failed\n"))
+        self.assertTrue(
+            result.traceback.endswith(
+                "ValueError: This task failed due to ValueError\n"
+            )
+        )
 
         self.assertEqual(DBTaskResult.objects.ready().count(), 0)
 
@@ -374,7 +378,7 @@ def test_complex_exception(self) -> None:
         self.assertEqual(DBTaskResult.objects.ready().count(), 0)
 
     def test_doesnt_process_different_backend(self) -> None:
-        result = test_tasks.failing_task.enqueue()
+        result = test_tasks.failing_task_value_error.enqueue()
 
         self.assertEqual(DBTaskResult.objects.ready().count(), 1)
 

tests/tests/test_immediate_backend.py

@@ -50,19 +50,57 @@ async def test_enqueue_task_async(self) -> None:
                 self.assertEqual(result.kwargs, {})
 
     def test_catches_exception(self) -> None:
-        result = default_task_backend.enqueue(test_tasks.failing_task, [], {})
+        test_data = [
+            (
+                test_tasks.failing_task_value_error,  # task function
+                ValueError,  # expected exception
+                "This task failed due to ValueError",  # expected message
+            ),
+            (
+                test_tasks.failing_task_system_exit,
+                SystemExit,
+                "This task failed due to SystemExit",
+            ),
+        ]
+        for task, exception, message in test_data:
+            with self.subTest(task), self.assertLogs(
+                "django_tasks.backends.immediate", level="ERROR"
+            ) as captured_logs:
+                result = default_task_backend.enqueue(task, [], {})
+
+                # assert logging
+                self.assertEqual(len(captured_logs.output), 1)
+                self.assertIn(message, captured_logs.output[0])
+
+                # assert result
+                self.assertEqual(result.status, ResultStatus.FAILED)
+                self.assertIsNotNone(result.started_at)
+                self.assertIsNotNone(result.finished_at)
+                self.assertGreaterEqual(result.started_at, result.enqueued_at)
+                self.assertGreaterEqual(result.finished_at, result.started_at)
+                self.assertIsInstance(result.result, exception)
+                self.assertTrue(
+                    result.traceback.endswith(f"{exception.__name__}: {message}\n")
+                )
+                self.assertIsNone(result.get_result())
+                self.assertEqual(result.task, task)
+                self.assertEqual(result.args, [])
+                self.assertEqual(result.kwargs, {})
 
-        self.assertEqual(result.status, ResultStatus.FAILED)
-        self.assertIsNotNone(result.started_at)
-        self.assertIsNotNone(result.finished_at)
-        self.assertGreaterEqual(result.started_at, result.enqueued_at)
-        self.assertGreaterEqual(result.finished_at, result.started_at)
-        self.assertIsInstance(result.result, ValueError)
-        self.assertTrue(result.traceback.endswith("ValueError: This task failed\n"))
-        self.assertIsNone(result.get_result())
-        self.assertEqual(result.task, test_tasks.failing_task)
-        self.assertEqual(result.args, [])
-        self.assertEqual(result.kwargs, {})
+    def test_throws_keyboard_interrupt(self) -> None:
+        with self.assertRaises(KeyboardInterrupt):
+            with self.assertLogs(
+                "django_tasks.backends.immediate", level="ERROR"
+            ) as captured_logs:
+                default_task_backend.enqueue(
+                    test_tasks.failing_task_keyboard_interrupt, [], {}
+                )
+
+        # assert logging
+        self.assertEqual(len(captured_logs.output), 1)
+        self.assertIn(
+            "This task failed due to KeyboardInterrupt", captured_logs.output[0]
+        )
 
     def test_complex_exception(self) -> None:
         with self.assertLogs("django_tasks.backends.immediate", level="ERROR"):
@@ -134,7 +172,7 @@ def test_cannot_pass_run_after(self) -> None:
             "Backend does not support run_after",
         ):
             default_task_backend.validate_task(
-                test_tasks.failing_task.using(run_after=timezone.now())
+                test_tasks.failing_task_value_error.using(run_after=timezone.now())
             )
 
     def test_meaning_of_life_view(self) -> None: