Fix unhandled exception in overdrive patron auth (#2188)

src/palace/manager/api/overdrive.py

@@ -8,6 +8,7 @@
 import re
 import urllib.parse
 from collections.abc import Iterable
+from json import JSONDecodeError
 from threading import RLock
 from typing import Any, NamedTuple
 from urllib.parse import quote, urlsplit, urlunsplit
@@ -961,21 +962,36 @@ def refresh_patron_access_token(
             # refuse to issue a token.
             payload["password_required"] = "false"
             payload["password"] = "[ignore]"
-        response = self.token_post(
-            self.PATRON_TOKEN_ENDPOINT, payload, is_fulfillment=is_fulfillment
-        )
-        if response.status_code == 200:
-            self._update_credential(credential, response.json())
-        elif response.status_code == 400:
-            response = response.json()
-            message = response["error"]
-            error = response.get("error_description")
-            if error:
-                message += "/" + error
-            debug = message
-            if error == "Requested record not found":
-                debug = "The patron failed Overdrive's cross-check against the library's ILS."
-            raise PatronAuthorizationFailedException(message, debug)
+        try:
+            response = self.token_post(
+                self.PATRON_TOKEN_ENDPOINT,
+                payload,
+                is_fulfillment=is_fulfillment,
+                allowed_response_codes=["2xx"],
+            )
+        except BadResponseException as e:
+            try:
+                response_data = e.response.json()
+            except JSONDecodeError:
+                self.log.exception(
+                    f"Error parsing Overdrive response. "
+                    f"Status code: {e.response.status_code}. Response: {e.response.content}"
+                )
+                response_data = {}
+            error_code = response_data.get("error")
+            error_description = response_data.get(
+                "error_description", "Failed to authenticate with Overdrive"
+            )
+            debug_message = (
+                f"refresh_patron_access_token failed. Status code: '{e.response.status_code}'. "
+                f"Error: '{error_code}'. Description: '{error_description}'."
+            )
+            self.log.info(debug_message + f" Response: '{e.response.text}'")
+            raise PatronAuthorizationFailedException(
+                error_description, debug_message
+            ) from e
+
+        self._update_credential(credential, response.json())
         return credential
 
     def checkout(

tests/files/overdrive/patron_token_failed.json

@@ -0,0 +1 @@
+{"error":"unauthorized_client","error_description":"Invalid Library Card: 123456.  Not a valid card."}

tests/manager/api/test_overdrive.py

@@ -31,6 +31,7 @@
     FulfilledOnIncompatiblePlatform,
     NoAcceptableFormat,
     NoAvailableCopies,
+    PatronAuthorizationFailedException,
     PatronHoldLimitReached,
     PatronLoanLimitReached,
 )
@@ -2130,6 +2131,40 @@ def test_refresh_patron_access_token(
         assert "false" == payload["password_required"]
         assert "[ignore]" == payload["password"]
 
+    def test_test_refresh_patron_access_token_failure(
+        self, overdrive_api_fixture: OverdriveAPIFixture
+    ) -> None:
+        db = overdrive_api_fixture.db
+        patron = db.patron()
+        patron.authorization_identifier = "barcode"
+        credential = db.credential(patron=patron)
+
+        # Test with a real 400 response we've seen from overdrive
+        data, raw = overdrive_api_fixture.sample_json("patron_token_failed.json")
+        overdrive_api_fixture.api.access_token_response = MockRequestsResponse(
+            400, content=raw
+        )
+        with pytest.raises(
+            PatronAuthorizationFailedException, match="Invalid Library Card"
+        ):
+            overdrive_api_fixture.api.refresh_patron_access_token(
+                credential, patron, "a pin"
+            )
+
+        # Test with a fictional 403 response that doesn't contain valid json - we've never
+        # seen this come back from overdrive, this test is just to make sure we can handle
+        # unexpected responses back from OD API.
+        overdrive_api_fixture.api.access_token_response = MockRequestsResponse(
+            403, content="garbage { json"
+        )
+        with pytest.raises(
+            PatronAuthorizationFailedException,
+            match="Failed to authenticate with Overdrive",
+        ):
+            overdrive_api_fixture.api.refresh_patron_access_token(
+                credential, patron, "a pin"
+            )
+
     def test_refresh_patron_access_token_is_fulfillment(
         self, overdrive_api_fixture: OverdriveAPIFixture
     ):
@@ -2148,7 +2183,10 @@ def test_refresh_patron_access_token_is_fulfillment(
         od_api = OverdriveAPI(db.session, overdrive_api_fixture.collection)
         od_api._server_nickname = OverdriveConstants.TESTING_SERVERS
         # but mock the request methods
-        od_api._do_post = MagicMock()
+        post_response, _ = overdrive_api_fixture.sample_json("patron_token.json")
+        od_api._do_post = MagicMock(
+            return_value=MockRequestsResponse(200, content=post_response)
+        )
         od_api._do_get = MagicMock()
         response_credential = od_api.refresh_patron_access_token(
             credential, patron, "a pin", is_fulfillment=True