Handle two exceptions causing sync_patron_activity job to fail (PP-1838)

src/palace/manager/celery/tasks/patron_activity.py

@@ -1,6 +1,7 @@
 from celery import shared_task
 
 from palace.manager.api.circulation import PatronActivityCirculationAPI
+from palace.manager.api.circulation_exceptions import PatronAuthorizationFailedException
 from palace.manager.celery.task import Task
 from palace.manager.service.celery.celery import QueueNames
 from palace.manager.service.integration_registry.license_providers import (
@@ -11,9 +12,11 @@
 from palace.manager.sqlalchemy.model.collection import Collection
 from palace.manager.sqlalchemy.model.patron import Patron
 from palace.manager.sqlalchemy.util import get_one
+from palace.manager.util.backoff import exponential_backoff
+from palace.manager.util.http import RemoteIntegrationException
 
 
-@shared_task(queue=QueueNames.high, bind=True)
+@shared_task(queue=QueueNames.high, bind=True, max_retries=4)
 def sync_patron_activity(
     task: Task, collection_id: int, patron_id: int, pin: str | None, force: bool = False
 ) -> None:
@@ -68,7 +71,33 @@ def sync_patron_activity(
                 )
                 return
 
-            api.sync_patron_activity(patron, pin)
+            try:
+                api.sync_patron_activity(patron, pin)
+            except PatronAuthorizationFailedException:
+                patron_activity_status.fail()
+                task.log.exception(
+                    "Patron activity sync task failed due to PatronAuthorizationFailedException. "
+                    "Marking patron activity as failed."
+                )
+                return
+            except RemoteIntegrationException as e:
+                # This may have been a transient network error with the remote integration. Attempt to retry.
+                retries = task.request.retries
+                if retries < task.max_retries:
+                    wait_time = exponential_backoff(retries)
+                    patron_activity_status.clear()
+                    task.log.exception(
+                        f"Patron activity sync task failed ({e}). Retrying in {wait_time} seconds."
+                    )
+                    raise task.retry(countdown=wait_time)
+
+                # We've reached the max number of retries. Mark the status as failed, but don't fail
+                # the task itself, since this is likely an error that is outside our control.
+                patron_activity_status.fail()
+                task.log.exception(
+                    f"Patron activity sync task failed ({e}). Max retries exceeded."
+                )
+                return
 
             task.log.info(
                 f"Patron activity sync for patron '{patron.authorization_identifier}' (id: {patron_id}) "

tests/manager/celery/tasks/test_patron_activity.py

@@ -3,6 +3,7 @@
 import pytest
 
 from palace.manager.api.circulation import HoldInfo, LoanInfo
+from palace.manager.api.circulation_exceptions import PatronAuthorizationFailedException
 from palace.manager.celery.task import Task
 from palace.manager.celery.tasks.patron_activity import sync_patron_activity
 from palace.manager.service.integration_registry.license_providers import (
@@ -13,6 +14,7 @@
     PatronActivity,
     PatronActivityStatus,
 )
+from palace.manager.util.http import RemoteIntegrationException
 from tests.fixtures.celery import CeleryFixture
 from tests.fixtures.database import DatabaseTransactionFixture
 from tests.fixtures.redis import RedisFixture
@@ -127,7 +129,54 @@ def test_collection_not_found(
         assert task_status.state == PatronActivityStatus.State.FAILED
         assert task_status.task_id == task.id
 
-    def test_exception(
+    def test_patron_authorization_failed_exception(
+        self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture
+    ):
+        sync_task_fixture.mock_collection_api.patron_activity = MagicMock(
+            side_effect=PatronAuthorizationFailedException()
+        )
+
+        sync_patron_activity.apply_async(
+            (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin")
+        ).wait()
+
+        task_status = sync_task_fixture.redis_record.status()
+        assert task_status is not None
+        assert task_status.state == PatronActivityStatus.State.FAILED
+
+        assert (
+            "Patron activity sync task failed due to PatronAuthorizationFailedException"
+            in caplog.text
+        )
+
+    @patch("palace.manager.celery.tasks.patron_activity.exponential_backoff")
+    def test_remote_integration_exception(
+        self,
+        mock_backoff: MagicMock,
+        sync_task_fixture: SyncTaskFixture,
+        caplog: pytest.LogCaptureFixture,
+    ):
+        # Make sure our backoff function doesn't delay the test.
+        mock_backoff.return_value = 0
+
+        mock_activity = create_autospec(
+            sync_task_fixture.mock_collection_api.patron_activity,
+            side_effect=RemoteIntegrationException("http://test.com", "boom!"),
+        )
+        sync_task_fixture.mock_collection_api.patron_activity = mock_activity
+
+        sync_patron_activity.apply_async(
+            (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin")
+        ).wait()
+
+        task_status = sync_task_fixture.redis_record.status()
+        assert task_status is not None
+        assert task_status.state == PatronActivityStatus.State.FAILED
+
+        assert mock_activity.call_count == 5
+        assert "Max retries exceeded" in caplog.text
+
+    def test_other_exception(
         self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture
     ):
         sync_task_fixture.mock_registry.from_collection.side_effect = Exception("Boom!")