diff --git a/django/tasks/backends/immediate.py b/django/tasks/backends/immediate.py index 283cc4e6a4..d70dc55463 100644 --- a/django/tasks/backends/immediate.py +++ b/django/tasks/backends/immediate.py @@ -7,7 +7,7 @@ from asgiref.sync import async_to_sync from django.db import transaction from django.tasks.signals import task_enqueued, task_finished from django.tasks.task import ResultStatus, TaskResult -from django.tasks.utils import exception_to_dict, get_random_id, json_normalize +from django.tasks.utils import get_exception_traceback, get_random_id, json_normalize from django.utils import timezone from .base import BaseTaskBackend @@ -46,10 +46,9 @@ class ImmediateBackend(BaseTaskBackend): raise object.__setattr__(task_result, "finished_at", timezone.now()) - try: - object.__setattr__(task_result, "_exception_data", exception_to_dict(e)) - except Exception: - logger.exception("Task id=%s unable to save exception", task_result.id) + + object.__setattr__(task_result, "_traceback", get_exception_traceback(e)) + object.__setattr__(task_result, "_exception_class", type(e)) object.__setattr__(task_result, "status", ResultStatus.FAILED) diff --git a/django/tasks/task.py b/django/tasks/task.py index 1b9458d9d2..6ffd7101a5 100644 --- a/django/tasks/task.py +++ b/django/tasks/task.py @@ -1,7 +1,7 @@ from dataclasses import dataclass, field, replace from datetime import datetime, timedelta from inspect import iscoroutinefunction -from typing import Any, Callable, Dict, Optional +from typing import Any, Callable, Dict, Optional, Type from asgiref.sync import async_to_sync, sync_to_async @@ -10,7 +10,7 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ from .exceptions import ResultDoesNotExist -from .utils import exception_from_dict, get_module_path, json_normalize +from .utils import get_module_path, json_normalize DEFAULT_TASK_BACKEND_ALIAS = "default" DEFAULT_QUEUE_NAME = "default" @@ -19,7 +19,8 @@ MAX_PRIORITY = 100 DEFAULT_PRIORITY = 0 TASK_REFRESH_ATTRS = { - "_exception_data", + "_exception_class", + "_traceback", "_return_value", "finished_at", "started_at", @@ -214,27 +215,10 @@ class TaskResult: backend: str """The name of the backend the task will run on""" + _exception_class: Optional[Type[BaseException]] = field(init=False, default=None) + _traceback: Optional[str] = field(init=False, default=None) + _return_value: Optional[Any] = field(init=False, default=None) - _exception_data: Optional[Dict[str, Any]] = field(init=False, default=None) - - @property - def exception(self): - return ( - exception_from_dict(self._exception_data) - if self.status == ResultStatus.FAILED and self._exception_data is not None - else None - ) - - @property - def traceback(self): - """ - Return the string representation of the traceback of the task if it failed - """ - return ( - self._exception_data["exc_traceback"] - if self.status == ResultStatus.FAILED and self._exception_data is not None - else None - ) @property def return_value(self): @@ -244,14 +228,32 @@ class TaskResult: If the task didn't succeed, an exception is raised. This is to distinguish against the task returning None. """ - if self.status == ResultStatus.FAILED: - raise ValueError("Task failed") - - elif self.status != ResultStatus.SUCCEEDED: + if not self.is_finished: raise ValueError("Task has not finished yet") return self._return_value + @property + def exception_class(self): + """The exception raised by the task function""" + if not self.is_finished: + raise ValueError("Task has not finished yet") + + return self._exception_class + + @property + def traceback(self): + """The traceback of the exception if the task failed""" + if not self.is_finished: + raise ValueError("Task has not finished yet") + + return self._traceback + + @property + def is_finished(self): + """Has the task finished?""" + return self.status in {ResultStatus.FAILED, ResultStatus.SUCCEEDED} + def refresh(self): """ Reload the cached task data from the task store diff --git a/django/tasks/utils.py b/django/tasks/utils.py index a92eae36e6..38f0eff4c5 100644 --- a/django/tasks/utils.py +++ b/django/tasks/utils.py @@ -6,7 +6,6 @@ from functools import wraps from traceback import format_exception from django.utils.crypto import RANDOM_STRING_CHARS -from django.utils.module_loading import import_string def is_module_level_function(func): @@ -52,27 +51,14 @@ def retry(*, retries=3, backoff_delay=0.1): return wrapper +def get_exception_traceback(exc): + return "".join(format_exception(type(exc), exc, exc.__traceback__)) + + def get_module_path(val): return f"{val.__module__}.{val.__qualname__}" -def exception_to_dict(exc): - return { - "exc_type": get_module_path(type(exc)), - "exc_args": json_normalize(exc.args), - "exc_traceback": "".join(format_exception(type(exc), exc, exc.__traceback__)), - } - - -def exception_from_dict(exc_data): - exc_class = import_string(exc_data["exc_type"]) - - if not inspect.isclass(exc_class) or not issubclass(exc_class, BaseException): - raise TypeError(f"{type(exc_class)} is not an exception") - - return exc_class(*exc_data["exc_args"]) - - def get_random_id(): """ Return a random string for use as a task id. diff --git a/docs/ref/tasks.txt b/docs/ref/tasks.txt index 27758690bc..a6e1cd16bd 100644 --- a/docs/ref/tasks.txt +++ b/docs/ref/tasks.txt @@ -183,15 +183,20 @@ Attributes of ``TaskResult`` cannot be modified. The backend the result is from. -.. attribute:: TaskResult.exception +.. attribute:: TaskResult.exception_class - The exception raised when executing the task, or ``None`` if no exception - was raised. + The exception class raised when executing the task. + + If the task has not finished, ``ValueError`` is raised. If the task finished + successfully, the exception class is ``None``. .. attribute:: TaskResult.traceback The exception traceback from the raised exception when the task failed. + If the task has not finished, ``ValueError`` is raised. If the task finished + successfully, the traceback is ``None``. + .. attribute:: TaskResult.return_value The return value from the task function. @@ -206,6 +211,10 @@ Attributes of ``TaskResult`` cannot be modified. The ``async`` variant of :meth:`TaskResult.refresh`. +.. attribute:: TaskResult.is_finished + + Whether the task has finished (successfully or not). + Backends ======== diff --git a/docs/topics/tasks.txt b/docs/topics/tasks.txt index 24cbf6b4df..e3b15c1dea 100644 --- a/docs/topics/tasks.txt +++ b/docs/topics/tasks.txt @@ -323,15 +323,13 @@ Exceptions ---------- If the task doesn't succeed, and instead raises an exception, either -as part of the task or as part of running it, the exception instance is saved -to the :attr:`django.tasks.TaskResult.exception` attribute:: +as part of the task or as part of running it, the exception class is saved +to the :attr:`django.tasks.TaskResult.exception_class` attribute:: - assert isinstance(result.exception, ValueError) + assert result.exception_class == ValueError -As part of the serialization process for exceptions, some information is lost. +Note that this is just the type of exception, and contains no other values. The traceback information is reduced to a string which you can use to help debugging:: print(result.traceback) - -If the exception could not be serialized, ``exception`` is ``None``. diff --git a/tests/tasks/test_dummy_backend.py b/tests/tasks/test_dummy_backend.py index bd35dff0d0..ce5786aa8e 100644 --- a/tests/tasks/test_dummy_backend.py +++ b/tests/tasks/test_dummy_backend.py @@ -32,6 +32,7 @@ class DummyBackendTestCase(SimpleTestCase): result = cast(Task, task).enqueue(1, two=3) self.assertEqual(result.status, ResultStatus.NEW) + self.assertFalse(result.is_finished) self.assertIsNone(result.started_at) self.assertIsNone(result.finished_at) with self.assertRaisesMessage(ValueError, "Task has not finished yet"): @@ -48,6 +49,7 @@ class DummyBackendTestCase(SimpleTestCase): result = await cast(Task, task).aenqueue() self.assertEqual(result.status, ResultStatus.NEW) + self.assertFalse(result.is_finished) self.assertIsNone(result.started_at) self.assertIsNone(result.finished_at) with self.assertRaisesMessage(ValueError, "Task has not finished yet"): @@ -118,6 +120,15 @@ class DummyBackendTestCase(SimpleTestCase): self.assertIn("enqueued", captured_logs.output[0]) self.assertIn(result.id, captured_logs.output[0]) + def test_exceptions(self): + result = test_tasks.noop_task.enqueue() + + with self.assertRaisesMessage(ValueError, "Task has not finished yet"): + result.exception_class + + with self.assertRaisesMessage(ValueError, "Task has not finished yet"): + result.traceback + class DummyBackendTransactionTestCase(TransactionTestCase): available_apps = [] diff --git a/tests/tasks/test_immediate_backend.py b/tests/tasks/test_immediate_backend.py index baf68c9c34..0b424ca89d 100644 --- a/tests/tasks/test_immediate_backend.py +++ b/tests/tasks/test_immediate_backend.py @@ -30,6 +30,7 @@ class ImmediateBackendTestCase(SimpleTestCase): result = cast(Task, task).enqueue(1, two=3) self.assertEqual(result.status, ResultStatus.SUCCEEDED) + self.assertTrue(result.is_finished) self.assertIsNotNone(result.started_at) self.assertIsNotNone(result.finished_at) self.assertGreaterEqual(result.started_at, result.enqueued_at) @@ -45,6 +46,7 @@ class ImmediateBackendTestCase(SimpleTestCase): result = await cast(Task, task).aenqueue() self.assertEqual(result.status, ResultStatus.SUCCEEDED) + self.assertTrue(result.is_finished) self.assertIsNotNone(result.started_at) self.assertIsNotNone(result.finished_at) self.assertGreaterEqual(result.started_at, result.enqueued_at) @@ -80,11 +82,12 @@ class ImmediateBackendTestCase(SimpleTestCase): # assert result self.assertEqual(result.status, ResultStatus.FAILED) + self.assertTrue(result.is_finished) 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.exception, exception) + self.assertEqual(result.exception_class, exception) self.assertTrue( result.traceback and result.traceback.endswith(f"{exception.__name__}: {message}\n") @@ -114,7 +117,8 @@ class ImmediateBackendTestCase(SimpleTestCase): self.assertGreaterEqual(result.finished_at, result.started_at) self.assertIsNone(result._return_value) - self.assertIsNone(result.traceback) + self.assertEqual(result.exception_class, ValueError) + self.assertIn('ValueError(ValueError("This task failed"))', result.traceback) self.assertEqual(result.task, test_tasks.complex_exception) self.assertEqual(result.args, []) diff --git a/tests/tasks/test_utils.py b/tests/tasks/test_utils.py index 9f6fc263ec..6c263fa7be 100644 --- a/tests/tasks/test_utils.py +++ b/tests/tasks/test_utils.py @@ -1,13 +1,8 @@ import datetime -import hashlib -import optparse import subprocess -from typing import List from unittest.mock import Mock -from django.core.exceptions import ImproperlyConfigured from django.tasks import utils -from django.tasks.exceptions import InvalidTaskError from django.test import SimpleTestCase from . import tasks as test_tasks @@ -79,87 +74,27 @@ class RetryTestCase(SimpleTestCase): self.assertFalse(utils.retry()(lambda: False)()) -class ExceptionSerializationTestCase(SimpleTestCase): - def test_serialize_exceptions(self): - for exc in [ - ValueError(10), - SyntaxError("Wrong"), - ImproperlyConfigured("It's wrong"), - InvalidTaskError(""), - SystemExit(), - ]: - with self.subTest(exc): - data = utils.exception_to_dict(exc) - self.assertEqual(utils.json_normalize(data), data) - self.assertEqual( - set(data.keys()), {"exc_type", "exc_args", "exc_traceback"} - ) - exception = utils.exception_from_dict(data) - self.assertIsInstance(exception, type(exc)) - self.assertEqual(exception.args, exc.args) +class ExceptionTracebackTestCase(SimpleTestCase): + def test_literal_exception(self): + self.assertEqual( + utils.get_exception_traceback(ValueError("Failure")), + "ValueError: Failure\n", + ) - # Check that the exception traceback contains a minimal traceback - msg = str(exc.args[0]) if exc.args else "" - traceback = data["exc_traceback"] - self.assertIn(exc.__class__.__name__, traceback) - self.assertIn(msg, traceback) - - def test_serialize_full_traceback(self): + def test_exception(self): try: - # Using optparse to generate an error because: - # - it's pure python - # - it's easy to trip down - # - it's unlikely to change ever - optparse.OptionParser(option_list=[1]) # type: ignore - except Exception as e: - traceback = utils.exception_to_dict(e)["exc_traceback"] - # The test is willingly fuzzy to ward against changes in the - # traceback formatting - self.assertIn("traceback", traceback.lower()) - self.assertIn("line", traceback.lower()) - self.assertIn(optparse.__file__, traceback) - self.assertTrue( - traceback.endswith("TypeError: not an Option instance: 1\n") - ) + 1 / 0 + except ZeroDivisionError as e: + traceback = utils.get_exception_traceback(e) + self.assertIn("ZeroDivisionError: division by zero", traceback) + else: + self.fail("ZeroDivisionError not raised") - def test_serialize_traceback_from_c_module(self): + def test_complex_exception(self) -> None: try: - # Same as test_serialize_full_traceback, but uses hashlib - # because it's in C, not in Python - hashlib.md5(1) # type: ignore - except Exception as e: - traceback = utils.exception_to_dict(e)["exc_traceback"] - self.assertIn("traceback", traceback.lower()) - self.assertTrue( - traceback.endswith( - "TypeError: object supporting the buffer API required\n" - ) - ) - self.assertIn("hashlib.md5(1)", traceback) - - def test_cannot_deserialize_non_exception(self): - serialized_exceptions: List[utils.SerializedExceptionDict] = [ - { - "exc_type": "subprocess.check_output", - "exc_args": ["exit", "1"], - "exc_traceback": "", - }, - {"exc_type": "True", "exc_args": [], "exc_traceback": ""}, - {"exc_type": "math.pi", "exc_args": [], "exc_traceback": ""}, - {"exc_type": __name__, "exc_args": [], "exc_traceback": ""}, - { - "exc_type": utils.get_module_path(type(self)), - "exc_args": [], - "exc_traceback": "", - }, - { - "exc_type": utils.get_module_path(Mock), - "exc_args": [], - "exc_traceback": "", - }, - ] - - for data in serialized_exceptions: - with self.subTest(data): - with self.assertRaises((TypeError, ImportError)): - utils.exception_from_dict(data) + {}[datetime.datetime.now()] + except KeyError as e: + traceback = utils.get_exception_traceback(e) + self.assertIn("KeyError: datetime.datetime", traceback) + else: + self.fail("KeyError not raised")