Skip to content

Commit

Permalink
Handle two exceptions that are causing patron_activity sync to fail (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathangreen authored Oct 23, 2024
1 parent c572cbf commit 4043a57
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 3 deletions.
33 changes: 31 additions & 2 deletions src/palace/manager/celery/tasks/patron_activity.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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:
Expand Down Expand Up @@ -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}) "
Expand Down
51 changes: 50 additions & 1 deletion tests/manager/celery/tasks/test_patron_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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!")
Expand Down

0 comments on commit 4043a57

Please sign in to comment.