Refactor BadResponseException

src/palace/manager/api/odl/api.py

@@ -63,7 +63,7 @@
 from palace.manager.sqlalchemy.util import get_one
 from palace.manager.util import base64
 from palace.manager.util.datetime_helpers import utc_now
-from palace.manager.util.http import BadResponseException
+from palace.manager.util.http import BadResponseException, RemoteIntegrationException
 
 
 class OPDS2WithODLApi(
@@ -261,16 +261,18 @@ def get_license_status_document(self, loan: Loan) -> dict[str, Any]:
                 f"status code {response.status_code}. Expected 2XX. Response headers: {header_string}. "
                 f"Response content: {response_string}."
             )
-            raise BadResponseException(url, "License Status Document request failed.")
+            raise BadResponseException(
+                url, "License Status Document request failed.", response
+            )
 
         try:
             status_doc = json.loads(response.content)
         except ValueError as e:
             raise BadResponseException(
-                url, "License Status Document was not valid JSON."
+                url, "License Status Document was not valid JSON.", response
             )
         if status_doc.get("status") not in self.STATUS_VALUES:
-            raise BadResponseException(
+            raise RemoteIntegrationException(
                 url, "License Status Document had an unknown status value."
             )
         return status_doc  # type: ignore[no-any-return]
@@ -958,7 +960,7 @@ def update_loan(self, loan: Loan, status_doc: dict[str, Any] | None = None) -> N
         # We already check that the status is valid in get_license_status_document,
         # but if the document came from a notification it hasn't been checked yet.
         if status not in self.STATUS_VALUES:
-            raise BadResponseException(
+            raise RemoteIntegrationException(
                 str(loan.license.checkout_url),
                 "The License Status Document had an unknown status value.",
             )

src/palace/manager/api/overdrive.py

@@ -562,10 +562,10 @@ def get(
         if status_code == 401:
             if exception_on_401:
                 # This is our second try. Give up.
-                raise BadResponseException.from_response(
+                raise BadResponseException(
                     url,
                     "Something's wrong with the Overdrive OAuth Bearer Token!",
-                    (status_code, headers, content),
+                    response,
                 )
             else:
                 # Refresh the token and try again.

src/palace/manager/core/opds2_import.py

@@ -10,6 +10,7 @@
 
 import webpub_manifest_parser.opds2.ast as opds2_ast
 from flask_babel import lazy_gettext as _
+from requests import Response
 from sqlalchemy.orm import Session
 from uritemplate import URITemplate
 from webpub_manifest_parser.core import ManifestParserFactory, ManifestParserResult
@@ -1157,18 +1158,16 @@ class OPDS2ImportMonitor(OPDSImportMonitor):
     PROTOCOL = OPDS2API.label()
     MEDIA_TYPE = OPDS2MediaTypesRegistry.OPDS_FEED.key, "application/json"
 
-    def _verify_media_type(
-        self, url: str, status_code: int, headers: Mapping[str, str], feed: bytes
-    ) -> None:
+    def _verify_media_type(self, url: str, resp: Response) -> None:
         # Make sure we got an OPDS feed, and not an error page that was
         # sent with a 200 status code.
-        media_type = headers.get("content-type")
+        media_type = resp.headers.get("content-type")
         if not media_type or not any(x in media_type for x in self.MEDIA_TYPE):
             message = "Expected {} OPDS 2.0 feed, got {}".format(
                 self.MEDIA_TYPE, media_type
             )
 
-            raise BadResponseException(url, message=message, status_code=status_code)
+            raise BadResponseException(url, message=message, response=resp)
 
     def _get_accept_header(self) -> str:
         return "{}, {};q=0.9, */*;q=0.1".format(

src/palace/manager/core/opds_import.py

@@ -1855,17 +1855,15 @@ def identifier_needs_import(
                 return True
         return False
 
-    def _verify_media_type(
-        self, url: str, status_code: int, headers: Mapping[str, str], feed: bytes
-    ) -> None:
+    def _verify_media_type(self, url: str, resp: Response) -> None:
         # Make sure we got an OPDS feed, and not an error page that was
         # sent with a 200 status code.
-        media_type = headers.get("content-type")
+        media_type = resp.headers.get("content-type")
         if not media_type or not any(
             x in media_type for x in (OPDSFeed.ATOM_LIKE_TYPES)
         ):
             message = "Expected Atom feed, got %s" % media_type
-            raise BadResponseException(url, message=message, status_code=status_code)
+            raise BadResponseException(url, message=message, response=resp)
 
     def follow_one_link(
         self, url: str, do_get: Callable[..., Response] | None = None
@@ -1881,10 +1879,8 @@ def follow_one_link(
         get = do_get or self._get
         resp = get(url, {})
         feed = resp.content
-        status_code = resp.status_code
-        headers = resp.headers
 
-        self._verify_media_type(url, status_code, headers, feed)
+        self._verify_media_type(url, resp)
 
         new_data = self.feed_contains_new_data(feed)
 

src/palace/manager/util/http.py

@@ -29,7 +29,7 @@
 )
 
 if TYPE_CHECKING:
-    from palace.manager.sqlalchemy.model.resource import HttpResponseTuple
+    pass
 
 
 class RemoteIntegrationException(IntegrationException, BaseProblemDetailException):
@@ -99,55 +99,28 @@ def __init__(
         self,
         url_or_service: str,
         message: str,
+        response: Response,
         debug_message: str | None = None,
-        status_code: int | None = None,
     ):
         """Indicate that a remote integration has failed.
 
         `param url_or_service` The name of the service that failed
            (e.g. "Overdrive"), or the specific URL that had the problem.
         """
+        if debug_message is None:
+            debug_message = (
+                f"Status code: {response.status_code}\nContent: {response.text}"
+            )
+
         super().__init__(url_or_service, message, debug_message)
         # to be set to 500, etc.
-        self.status_code = status_code
-
-    @classmethod
-    def from_response(
-        cls, url: str, message: str, response: HttpResponseTuple | Response
-    ) -> Self:
-        """Helper method to turn a `requests` Response object into
-        a BadResponseException.
-        """
-        if isinstance(response, tuple):
-            # The response has been unrolled into a (status_code,
-            # headers, body) 3-tuple.
-            status_code, _, content_bytes = response
-            # The HTTP content response is a bytestring that we want to
-            # convert to unicode for the debug message.
-            if content_bytes:
-                content = content_bytes.decode("utf-8")
-            else:
-                content = ""
-        else:
-            status_code = response.status_code
-            content = response.text
-
-        return cls(
-            url,
-            message,
-            status_code=status_code,
-            debug_message="Status code: %s\nContent: %s"
-            % (
-                status_code,
-                content,
-            ),
-        )
+        self.response = response
 
     @classmethod
     def bad_status_code(cls, url: str, response: Response) -> Self:
         """The response is bad because the status code is wrong."""
         message = cls.BAD_STATUS_CODE_MESSAGE % response.status_code
-        return cls.from_response(
+        return cls(
             url,
             message,
             response,
@@ -399,9 +372,9 @@ def _process_response(
             raise BadResponseException(
                 url,
                 error_message % code,
-                status_code=response.status_code,
                 debug_message="Response content: %s"
                 % cls._decode_response_content(response, url),
+                response=response,
             )
         return response
 

tests/manager/api/odl/test_api.py

@@ -44,7 +44,7 @@
 from palace.manager.sqlalchemy.model.work import Work
 from palace.manager.sqlalchemy.util import create
 from palace.manager.util.datetime_helpers import datetime_utc, utc_now
-from palace.manager.util.http import BadResponseException
+from palace.manager.util.http import BadResponseException, RemoteIntegrationException
 from tests.fixtures.database import DatabaseTransactionFixture
 from tests.fixtures.odl import OPDS2WithODLApiFixture
 
@@ -277,7 +277,7 @@ def test_get_license_status_document_errors(
             200, content=json.dumps(dict(status="unknown"))
         )
         pytest.raises(
-            BadResponseException,
+            RemoteIntegrationException,
             opds2_with_odl_api_fixture.api.get_license_status_document,
             loan,
         )

tests/manager/util/test_http.py

@@ -394,14 +394,14 @@ def test_with_debug_message(self):
 
 
 class TestBadResponseException:
-    def test_from_response(self):
+    def test__init__(self):
         response = MockRequestsResponse(102, content="nonsense")
-        exc = BadResponseException.from_response(
+        exc = BadResponseException(
             "http://url/", "Terrible response, just terrible", response
         )
 
-        # the status code gets set on the exception
-        assert exc.status_code == 102
+        # the response gets set on the exception
+        assert exc.response is response
 
         # Turn the exception into a problem detail document, and it's full
         # of useful information.
@@ -418,7 +418,7 @@ def test_from_response(self):
         )
         assert problem_detail.status_code == 502
 
-    def test_bad_status_code(object):
+    def test_bad_status_code(self):
         response = MockRequestsResponse(500, content="Internal Server Error!")
         exc = BadResponseException.bad_status_code("http://url/", response)
         doc = exc.problem_detail
@@ -434,11 +434,12 @@ def test_bad_status_code(object):
         )
 
     def test_problem_detail(self):
+        response = MockRequestsResponse(401, content="You are not authorized!")
         exception = BadResponseException(
             "http://url/",
             "What even is this",
             debug_message="some debug info",
-            status_code=401,
+            response=response,
         )
         document = exception.problem_detail
         assert 502 == document.status_code
@@ -451,7 +452,7 @@ def test_problem_detail(self):
             "Bad response from http://url/: What even is this\n\nsome debug info"
             == document.debug_message
         )
-        assert exception.status_code == 401
+        assert exception.response is response
 
 
 class TestRequestTimedOut: