Reusable function to extract error message from responses and … (#144)

doc/source/api.rst

@@ -60,3 +60,6 @@ Utilities
 
 .. automodule:: osctiny.utils.mapping
     :members:
+
+.. automodule:: osctiny.utils.xml
+    :members:

osctiny/osc.py

@@ -20,8 +20,6 @@
 from urllib.parse import quote, parse_qs, urlparse
 import warnings
 
-# pylint: disable=no-name-in-module
-from lxml.objectify import fromstring, makeparser
 from requests import Session, Request
 from requests.auth import HTTPBasicAuth
 from requests.cookies import RequestsCookieJar, cookiejar_from_dict
@@ -41,6 +39,7 @@
 from .utils.backports import cached_property
 from .utils.conf import BOOLEAN_PARAMS, get_credentials, get_cookie_jar
 from .utils.errors import OscError
+from .utils.xml import get_xml_parser, get_objectified_xml
 
 
 THREAD_LOCAL = threading.local()
@@ -216,11 +215,11 @@ def cookies(self, value: typing.Union[CookieJar, dict]):
     def parser(self):
         """
         Explicit parser instance
-        """
-        if not hasattr(THREAD_LOCAL, "parser"):
-            THREAD_LOCAL.parser = makeparser(huge_tree=True)
 
-        return THREAD_LOCAL.parser
+        .. versionchanged:: 0.8.0
+            Content moved to :py:fun:`osctiny.utils.xml.get_xml_parser`
+        """
+        return get_xml_parser()
 
     def request(self, url, method="GET", stream=False, data=None, params=None,
                 raise_for_status=True, timeout=None):
@@ -465,25 +464,8 @@ def get_objectified_xml(self, response):
 
             Allow ``response`` to be a string
 
-        :param response: An API response or XML string
-        :rtype response: :py:class:`requests.Response`
-        :return: :py:class:`lxml.objectify.ObjectifiedElement`
+        .. versionchanged:: 0.8.0
+
+            Content moved to :py:fun:`osctiny.utils.xml.get_objectified_xml`
         """
-        if isinstance(response, str):
-            text = response
-        else:
-            text = response.text
-
-        try:
-            return fromstring(text, self.parser)
-        except ValueError:
-            # Just in case OBS returns a Unicode string with encoding
-            # declaration
-            if isinstance(text, str) and \
-                    "encoding=" in text:
-                return fromstring(
-                    re.sub(r'encoding="[^"]+"', "", text)
-                )
-
-            # This might be something else
-            raise
+        return get_objectified_xml(response=response)

osctiny/tests/test_utils.py

@@ -10,17 +10,19 @@
 import sys
 from tempfile import mkstemp
 from types import GeneratorType
+import warnings
 
 from dateutil.parser import parse
 from pytz import _UTC, timezone
-from requests import Response
+from requests import Response, HTTPError
 import responses
 
 from ..osc import Osc, THREAD_LOCAL
 from ..utils.auth import HttpSignatureAuth
 from ..utils.changelog import ChangeLog, Entry
 from ..utils.conf import get_config_path, get_credentials
 from ..utils.mapping import Mappable
+from ..utils.errors import get_http_error_details
 
 sys.path.append(os.path.dirname(__file__))
 
@@ -489,3 +491,65 @@ def test_handle_401(self, *_):
                                      "Basic realm=\"Use your developer account\", "})
             response = self.osc.session.get("https://api.example.com/hello-world")
             self.do_assertions(response, True)
+
+
+class TestError(TestCase):
+    url = "http://example.com"
+    @property
+    def osc(self) -> Osc:
+        return Osc(url=self.url, username="nemo", password="password")
+
+    @responses.activate
+    def test_get_http_error_details(self):
+        status = 400
+        summary = "Bla Bla Bla"
+        responses.add(
+            responses.GET,
+            "http://example.com",
+            body=f"""<status code="foo"><summary>{summary}</summary></status>""",
+            status=status
+        )
+
+        response = self.osc.session.get(self.url)
+
+        with self.subTest("Response"):
+            self.assertEqual(response.status_code, status)
+            self.assertEqual(get_http_error_details(response), summary)
+
+        with self.subTest("Exception"):
+            try:
+                response.raise_for_status()
+            except HTTPError as error:
+                self.assertEqual(get_http_error_details(error), summary)
+            else:
+                self.fail("No exception was raised")
+
+    @responses.activate
+    def test_get_http_error_details__bad_response(self):
+        status = 502
+        responses.add(
+            responses.GET,
+            "http://example.com",
+            body=f"""Bad Gateway HTML message""",
+            status=status
+        )
+
+        response = self.osc.session.get(self.url)
+
+        with self.subTest("Response"):
+            self.assertEqual(response.status_code, status)
+            with warnings.catch_warnings(record=True) as emitted_warnings:
+                self.assertIn("Server replied with:", get_http_error_details(response))
+                self.assertEqual(len(emitted_warnings), 1)
+                self.assertIn("Start tag expected", str(emitted_warnings[-1].message))
+
+        with self.subTest("Exception"):
+            try:
+                response.raise_for_status()
+            except HTTPError as error:
+                with warnings.catch_warnings(record=True) as emitted_warnings:
+                    self.assertIn("Server replied with:", get_http_error_details(error))
+                    self.assertEqual(len(emitted_warnings), 1)
+                    self.assertIn("Start tag expected", str(emitted_warnings[-1].message))
+            else:
+                self.fail("No exception was raised")

osctiny/utils/errors.py

@@ -1,10 +1,42 @@
 """
-Base classes for osc-tiny specific exceptions
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Exception base classes and utilities
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 """
+import typing
+from warnings import warn
+
+from requests import HTTPError, Response
+
+from .xml import get_objectified_xml
+
+
+def get_http_error_details(error: typing.Union[HTTPError, Response]) -> str:
+    """
+    Extract user-friendly error message from exception
+
+    .. versionadded:: 0.8.0
+    """
+    if isinstance(error, HTTPError):
+        response = error.response
+    elif isinstance(error, Response):
+        response = error
+    else:
+        raise TypeError("Expected a Response of HTTPError instance!")
+
+    try:
+        xml_obj = get_objectified_xml(response)
+    except Exception as error2:
+        warn(message=f"Failed to extract error message due to another error: {error2}",
+             category=RuntimeWarning)
+    else:
+        summary = xml_obj.find("summary")
+        if summary is not None:
+            return summary.text
+
+    return f"Server replied with: {response.status_code} {response.reason}"
 
 
 class OscError(Exception):
     """
-    Base class for expcetions to be raised by ``osctiny``
+    Base class for exceptions to be raised by ``osctiny``
     """

osctiny/utils/xml.py

@@ -0,0 +1,74 @@
+"""
+XML parsing
+^^^^^^^^^^^
+
+.. versionadded:: 0.8.0
+"""
+import re
+import threading
+import typing
+
+from lxml.etree import XMLParser
+from lxml.objectify import fromstring, makeparser, ObjectifiedElement
+from requests import Response
+
+
+THREAD_LOCAL = threading.local()
+
+
+def get_xml_parser() -> XMLParser:
+    """
+    Get a parser object
+
+    .. versionchanged:: 0.8.0
+
+        Carved out from the ``Osc`` class
+    """
+    if not hasattr(THREAD_LOCAL, "parser"):
+        THREAD_LOCAL.parser = makeparser(huge_tree=True)
+
+    return THREAD_LOCAL.parser
+
+
+def get_objectified_xml(response: typing.Union[Response, str]) -> ObjectifiedElement:
+    """
+    Return API response as an XML object
+
+    .. versionchanged:: 0.1.6
+
+        Allow parsing of "huge" XML inputs
+
+    .. versionchanged:: 0.2.4
+
+        Allow ``response`` to be a string
+
+    .. versionchanged:: 0.8.0
+
+        Carved out from ``Osc`` class
+
+    :param response: An API response or XML string
+    :rtype response: :py:class:`requests.Response`
+    :return: :py:class:`lxml.objectify.ObjectifiedElement`
+    """
+    if isinstance(response, str):
+        text = response
+    elif isinstance(response, Response):
+        text = response.text
+    else:
+        raise TypeError(f"Expected a string or response object. Got  {type(response)} instead.")
+
+    parser = get_xml_parser()
+
+    try:
+        return fromstring(text, parser)
+    except ValueError:
+        # Just in case OBS returns a Unicode string with encoding
+        # declaration
+        if isinstance(text, str) and \
+                "encoding=" in text:
+            return fromstring(
+                re.sub(r'encoding="[^"]+"', "", text)
+            )
+
+        # This might be something else
+        raise