Support pickling of SlackException

Exception classes that call the `BaseException` `__init__` method
(possibly via `Exception`) with a different number of arguments than
the `__init__` method of the derived exception class takes cannot be
pickled and unpickled. Since arq uses pickle to record the exception
of failed jobs, and we may want such exceptions to be derived from
`SlackException` so that we can easily report them to Slack, this is
an annoying limitation. The restriction is due to the properties of
the default `__reduce__` method on `BaseException` and probably cannot
be changed in Python.

Work around this by using the pattern recommended at the end of the
discussion in python/cpython#44791, namely
avoid calling `BaseException.__init__` entirely. This means we can
no longer rely on the default `__str__` behavior and must implement
`__str__`.

Add a test that `SlackException` and classes derived from it with
different numbers of constructor arguments can be pickled and unpickled
without loss of the information that goes into `to_slack`.

changelog.d/20240607_112935_rra_DM_44720.md

@@ -0,0 +1,3 @@
+### New features
+
+- Support pickling of `SlackException` so that subclasses of it can be thrown by arq workers and unpickled correctly when retrieving results.

pyproject.toml

@@ -263,6 +263,7 @@ select = ["ALL"]
     "PT012",   # way too aggressive about limiting pytest.raises blocks
     "S101",    # tests should use assert
     "S106",    # tests are allowed to hard-code dummy passwords
+    "S301",    # one test verifies that a class can be pickled
     "SLF001",  # tests are allowed to access private members
 ]
 

src/safir/slack/blockkit.py

@@ -234,9 +234,19 @@ def to_slack(self) -> dict[str, Any]:
 class SlackException(Exception):
     """Parent class of exceptions that can be reported to Slack.
 
-    Intended to be subclassed.  Subclasses may wish to override the
+    Intended to be subclassed. Subclasses may wish to override the
     ``to_slack`` method.
 
+    Attributes
+    ----------
+    message
+        Error message represented by this exception.
+    user
+        Username associated with the exception.
+    failed_at
+        When the failure occurred. Defaults to the time the exception was
+        created.
+
     Parameters
     ----------
     message
@@ -254,12 +264,24 @@ def __init__(
         *,
         failed_at: datetime | None = None,
     ) -> None:
+        # Do not call the parent Exception constructor here, because calling
+        # it with a different number of arguments than the constructor
+        # argument of derived exceptions breaks pickling. See the end of
+        # https://github.com/python/cpython/issues/44791. This requires
+        # implementing __str__ rather than relying on the default behavior.
+        #
+        # Arguably, this is a bug in the __reduce__ method of BaseException
+        # and its interaction with constructors, but it appears to be hard to
+        # fix. See https://github.com/python/cpython/issues/76877.
+        self.message = message
         self.user = user
         if failed_at:
             self.failed_at = failed_at
         else:
             self.failed_at = current_datetime(microseconds=True)
-        super().__init__(message)
+
+    def __str__(self) -> str:
+        return self.message
 
     def to_slack(self) -> SlackMessage:
         """Format the exception as a Slack message.
@@ -366,12 +388,12 @@ def __init__(
         status: int | None = None,
         body: str | None = None,
     ) -> None:
+        super().__init__(message, user, failed_at=failed_at)
         self.message = message
         self.method = method
         self.url = url
         self.status = status
         self.body = body
-        super().__init__(message, user, failed_at=failed_at)
 
     def __str__(self) -> str:
         result = self.message

tests/slack/blockkit_test.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import pickle
 from unittest.mock import ANY
 
 import pytest
@@ -294,6 +295,31 @@ class SomeError(SlackException):
     ]
 
 
+class SlackSubclassException(SlackException):
+    """Subclsas for testing pickling."""
+
+    def __init__(self) -> None:
+        super().__init__("Some error", "username")
+
+
+def test_exception_pickling() -> None:
+    """Test that a `SlackException` can be pickled and unpickled.
+
+    Errors that may be raised by backend workers in an arq queue must support
+    pickling so that they can be passed correctly to other workers or the
+    frontend.
+    """
+    exc = SlackException("some message", "username")
+    pickled_exc = pickle.loads(pickle.dumps(exc))
+    assert exc.to_slack() == pickled_exc.to_slack()
+
+    # Try the same with a derived class with a different number of constructor
+    # arguments.
+    subexc = SlackSubclassException()
+    pickled_subexc = pickle.loads(pickle.dumps(subexc))
+    assert subexc.to_slack() == pickled_subexc.to_slack()
+
+
 @pytest.mark.asyncio
 async def test_web_exception(
     respx_mock: respx.Router, mock_slack: MockSlackWebhook