Refactor CirculationExceptions (#1694)

api/admin/exceptions.py

@@ -1,19 +1,14 @@
-from typing import Any
-
 from api.admin.problem_details import ADMIN_NOT_AUTHORIZED
-from core.util.problem_detail import ProblemDetail
-
+from core.util.problem_detail import BaseProblemDetailException, ProblemDetail
 
-class AdminNotAuthorized(Exception):
-    status_code = 403
 
-    def __init__(self, *args: Any) -> None:
-        self.message = None
-        if len(args) > 0:
-            self.message = args[0]
-        super().__init__(*args)
+class AdminNotAuthorized(BaseProblemDetailException):
+    def __init__(self, message: str | None = None) -> None:
+        self.message = message
+        super().__init__(message)
 
-    def as_problem_detail_document(self, debug=False) -> ProblemDetail:
+    @property
+    def problem_detail(self) -> ProblemDetail:
         return (
             ADMIN_NOT_AUTHORIZED.detailed(self.message)
             if self.message

api/admin/routes.py

@@ -23,8 +23,8 @@
 from api.routes import allows_library, has_library, library_route
 from core.app_server import ensure_pydantic_after_problem_detail, returns_problem_detail
 from core.util.problem_detail import (
+    BaseProblemDetailException,
     ProblemDetail,
-    ProblemDetailException,
     ProblemDetailModel,
 )
 
@@ -95,8 +95,8 @@ def returns_json_or_response_or_problem_detail(f):
     def decorated(*args, **kwargs):
         try:
             v = f(*args, **kwargs)
-        except ProblemDetailException as ex:
-            # A ProblemError is the same as a ProblemDetail
+        except BaseProblemDetailException as ex:
+            # A ProblemDetailException just needs to be converted to a ProblemDetail.
             v = ex.problem_detail
         if isinstance(v, ProblemDetail):
             return v.response

api/axis.py

@@ -33,12 +33,31 @@
     LoanInfo,
     PatronActivityCirculationAPI,
 )
-from api.circulation_exceptions import *
+from api.circulation_exceptions import (
+    AlreadyCheckedOut,
+    AlreadyOnHold,
+    CannotFulfill,
+    CannotHold,
+    CannotLoan,
+    CurrentlyAvailable,
+    InvalidInputException,
+    LibraryAuthorizationFailedException,
+    LibraryInvalidInputException,
+    NoAcceptableFormat,
+    NoActiveLoan,
+    NoAvailableCopies,
+    NotFoundOnRemote,
+    NotOnHold,
+    PatronAuthorizationFailedException,
+    PatronLoanLimitReached,
+    RemoteInitiatedServerError,
+)
 from api.selftest import HasCollectionSelfTests, SelfTestResult
 from api.web_publication_manifest import FindawayManifest, SpineItem
 from core.analytics import Analytics
 from core.config import CannotLoadConfiguration
 from core.coverage import BibliographicCoverageProvider, CoverageFailure
+from core.exceptions import IntegrationException
 from core.integration.settings import (
     ConfigurationFormItem,
     ConfigurationFormItemType,
@@ -74,6 +93,7 @@
     Subject,
 )
 from core.monitor import CollectionMonitor, IdentifierSweepMonitor, TimelineMonitor
+from core.problem_details import INTEGRATION_ERROR
 from core.service.container import Services
 from core.util.datetime_helpers import datetime_utc, strptime_utc, utc_now
 from core.util.flask_util import Response
@@ -382,8 +402,8 @@ def checkin(self, patron: Patron, pin: str, licensepool: LicensePool) -> None:
             CheckinResponseParser(licensepool.collection).process_first(
                 response.content
             )
-        except etree.XMLSyntaxError as e:
-            raise RemoteInitiatedServerError(response.content, self.label())
+        except etree.XMLSyntaxError:
+            raise RemoteInitiatedServerError(response.text, self.label())
 
     def _checkin(self, title_id: str | None, patron_id: str | None) -> RequestsResponse:
         """Make a request to the EarlyCheckInTitle endpoint."""
@@ -424,8 +444,8 @@ def checkout(
             if loan_info is None:
                 raise CannotLoan()
             return loan_info
-        except etree.XMLSyntaxError as e:
-            raise RemoteInitiatedServerError(response.content, self.label())
+        except etree.XMLSyntaxError:
+            raise RemoteInitiatedServerError(response.text, self.label())
 
     def _checkout(
         self, title_id: str | None, patron_id: str | None, internal_format: str

api/bibliotheca.py

@@ -30,7 +30,21 @@
     LoanInfo,
     PatronActivityCirculationAPI,
 )
-from api.circulation_exceptions import *
+from api.circulation_exceptions import (
+    AlreadyCheckedOut,
+    AlreadyOnHold,
+    CannotHold,
+    CannotLoan,
+    CannotReleaseHold,
+    CurrentlyAvailable,
+    NoAvailableCopies,
+    NoLicenses,
+    NotCheckedOut,
+    NotOnHold,
+    PatronHoldLimitReached,
+    PatronLoanLimitReached,
+    RemoteInitiatedServerError,
+)
 from api.selftest import HasCollectionSelfTests, SelfTestResult
 from api.web_publication_manifest import FindawayManifest, SpineItem
 from core.analytics import Analytics
@@ -79,6 +93,7 @@
 from core.util import base64
 from core.util.datetime_helpers import datetime_utc, strptime_utc, to_utc, utc_now
 from core.util.http import HTTP
+from core.util.problem_detail import BaseProblemDetailException
 from core.util.xmlparser import XMLParser, XMLProcessor
 
 
@@ -894,23 +909,7 @@ def date_from_subtag(self, tag, key, required=True):
         return self.parse_date(value)
 
 
-class BibliothecaException(Exception):
-    pass
-
-
-class WorkflowException(BibliothecaException):
-    def __init__(self, actual_status, statuses_that_would_work):
-        self.actual_status = actual_status
-        self.statuses_that_would_work = statuses_that_would_work
-
-    def __str__(self):
-        return "Book status is {}, must be: {}".format(
-            self.actual_status,
-            ", ".join(self.statuses_that_would_work),
-        )
-
-
-class ErrorParser(BibliothecaParser[Exception]):
+class ErrorParser(BibliothecaParser[BaseProblemDetailException]):
     """Turn an error document from the Bibliotheca web service into a CheckoutException"""
 
     wrong_status = re.compile(
@@ -930,13 +929,20 @@ class ErrorParser(BibliothecaParser[Exception]):
     def xpath_expression(self) -> str:
         return "//Error"
 
-    def process_first(self, string: str | bytes) -> Exception:
+    def process_first(self, string: str | bytes) -> BaseProblemDetailException:
         try:
             return_val = super().process_first(string)
         except Exception as e:
             # The server sent us an error with an incorrect or
             # nonstandard syntax.
-            return RemoteInitiatedServerError(string, BibliothecaAPI.SERVICE_NAME)
+            if isinstance(string, bytes):
+                try:
+                    debug = string.decode("utf-8")
+                except UnicodeDecodeError:
+                    debug = "Unreadable error message (Unicode decode error)."
+            else:
+                debug = string
+            return RemoteInitiatedServerError(debug, BibliothecaAPI.SERVICE_NAME)
 
         if return_val is None:
             # We were not able to interpret the result as an error.
@@ -950,7 +956,7 @@ def process_first(self, string: str | bytes) -> Exception:
 
     def process_one(
         self, error_tag: _Element, namespaces: dict[str, str] | None
-    ) -> Exception:
+    ) -> BaseProblemDetailException:
         message = self.text_of_optional_subtag(error_tag, "Message")
         if not message:
             return RemoteInitiatedServerError(
@@ -982,7 +988,7 @@ def process_one(
 
         m = self.wrong_status.search(message)
         if not m:
-            return BibliothecaException(message)
+            return RemoteInitiatedServerError(message, BibliothecaAPI.SERVICE_NAME)
         actual, expected = m.groups()
         expected = expected.split(",")
 
@@ -1010,7 +1016,7 @@ def process_one(
         if "CAN_LOAN" in expected:
             return CannotLoan(message)
 
-        return BibliothecaException(message)
+        return RemoteInitiatedServerError(message, BibliothecaAPI.SERVICE_NAME)
 
 
 class PatronCirculationParser(XMLParser):

api/circulation.py

@@ -16,7 +16,25 @@
 from pydantic import PositiveInt
 from sqlalchemy.orm import Query
 
-from api.circulation_exceptions import *
+from api.circulation_exceptions import (
+    AlreadyCheckedOut,
+    AlreadyOnHold,
+    CannotFulfill,
+    CannotRenew,
+    CannotReturn,
+    CurrentlyAvailable,
+    DeliveryMechanismConflict,
+    DeliveryMechanismError,
+    DeliveryMechanismMissing,
+    NoAcceptableFormat,
+    NoActiveLoan,
+    NoAvailableCopies,
+    NoLicenses,
+    NotCheckedOut,
+    NotOnHold,
+    PatronHoldLimitReached,
+    PatronLoanLimitReached,
+)
 from api.util.patron import PatronUtility
 from core.analytics import Analytics
 from core.integration.base import HasLibraryIntegrationConfiguration
@@ -45,6 +63,7 @@
 from core.model.integration import IntegrationConfiguration
 from core.util.datetime_helpers import utc_now
 from core.util.log import LoggerMixin
+from core.util.problem_detail import ProblemDetail
 
 
 class CirculationInfo: