Properly parse errorMessage key in error response (#1526)

* consistent response errors parsing

* properly parse errorMessage message in error responses

jira/resilientsession.py

@@ -3,7 +3,7 @@
 import logging
 import random
 import time
-from typing import Any, Dict, Optional, Union
+from typing import Any, Dict, List, Optional, Union
 
 from requests import Response, Session
 from requests.exceptions import ConnectionError
@@ -78,49 +78,66 @@ def raise_on_error(resp: Optional[Response], **kwargs) -> TypeGuard[Response]:
     return True  # if no exception was raised, we have a valid Response
 
 
-def parse_error_msg(resp: Response) -> str:
-    """Parse a Jira Error message from the Response.
+def parse_errors(resp: Response) -> List[str]:
+    """Parse a Jira Error messages from the Response.
 
     https://developer.atlassian.com/cloud/jira/platform/rest/v2/intro/#status-codes
 
     Args:
         resp (Response): The Jira API request's response.
 
     Returns:
-        str: The error message parsed from the Response. An empty string if no error.
+        List[str]: The error messages list parsed from the Response. An empty list if no error.
     """
     resp_data: Dict[str, Any] = {}  # json parsed from the response
-    parsed_error = ""  # error message parsed from the response
-
+    parsed_errors: List[str] = []  # error messages parsed from the response
     if resp.status_code == 403 and "x-authentication-denied-reason" in resp.headers:
-        parsed_error = resp.headers["x-authentication-denied-reason"]
+        return [resp.headers["x-authentication-denied-reason"]]
     elif resp.text:
         try:
             resp_data = resp.json()
         except ValueError:
-            parsed_error = resp.text
+            return [resp.text]
 
     if "message" in resp_data:
         # Jira 5.1 errors
-        parsed_error = resp_data["message"]
+        parsed_errors = [resp_data["message"]]
+    elif "errorMessage" in resp_data:
+        # Sometimes Jira returns `errorMessage` as a message error key
+        # for example for the "Service temporary unavailable" error
+        parsed_errors = [resp_data["errorMessage"]]
     elif "errorMessages" in resp_data:
         # Jira 5.0.x error messages sometimes come wrapped in this array
         # Sometimes this is present but empty
         error_messages = resp_data["errorMessages"]
         if len(error_messages) > 0:
             if isinstance(error_messages, (list, tuple)):
-                parsed_error = "\n".join(error_messages)
+                parsed_errors = list(error_messages)
             else:
-                parsed_error = error_messages
+                parsed_errors = [error_messages]
     elif "errors" in resp_data:
         resp_errors = resp_data["errors"]
         if len(resp_errors) > 0 and isinstance(resp_errors, dict):
             # Catching only 'errors' that are dict. See https://github.com/pycontribs/jira/issues/350
             # Jira 6.x error messages are found in this array.
-            error_list = resp_errors.values()
-            parsed_error = ", ".join(error_list)
+            parsed_errors = [str(err) for err in resp_errors.values()]
+
+    return parsed_errors
+
 
-    return parsed_error
+def parse_error_msg(resp: Response) -> str:
+    """Parse a Jira Error messages from the Response and join them by comma.
+
+    https://developer.atlassian.com/cloud/jira/platform/rest/v2/intro/#status-codes
+
+    Args:
+        resp (Response): The Jira API request's response.
+
+    Returns:
+        str: The error message parsed from the Response. An empty str if no error.
+    """
+    errors = parse_errors(resp)
+    return ", ".join(errors)
 
 
 class ResilientSession(Session):

jira/resources.py

@@ -12,7 +12,7 @@
 from requests import Response
 from requests.structures import CaseInsensitiveDict
 
-from jira.resilientsession import ResilientSession
+from jira.resilientsession import ResilientSession, parse_errors
 from jira.utils import json_loads, threaded_requests
 
 if TYPE_CHECKING:
@@ -68,35 +68,6 @@ class AnyLike:
 logging.getLogger("jira").addHandler(logging.NullHandler())
 
 
-def get_error_list(r: Response) -> List[str]:
-    error_list = []
-    if r.status_code >= 400:
-        if r.status_code == 403 and "x-authentication-denied-reason" in r.headers:
-            error_list = [r.headers["x-authentication-denied-reason"]]
-        elif r.text:
-            try:
-                response: Dict[str, Any] = json_loads(r)
-                if "message" in response:
-                    # Jira 5.1 errors
-                    error_list = [response["message"]]
-                elif "errorMessages" in response and len(response["errorMessages"]) > 0:
-                    # Jira 5.0.x error messages sometimes come wrapped in this array
-                    # Sometimes this is present but empty
-                    errorMessages = response["errorMessages"]
-                    if isinstance(errorMessages, (list, tuple)):
-                        error_list = list(errorMessages)
-                    else:
-                        error_list = [errorMessages]
-                elif "errors" in response and len(response["errors"]) > 0:
-                    # Jira 6.x error messages are found in this array.
-                    error_list = response["errors"].values()
-                else:
-                    error_list = [r.text]
-            except ValueError:
-                error_list = [r.text]
-    return error_list
-
-
 class Resource:
     """Models a URL-addressable resource in the Jira REST API.
 
@@ -358,7 +329,7 @@ def update(
         r = self._session.put(self.self + querystring, data=json.dumps(data))
         if "autofix" in self._options and r.status_code == 400:
             user = None
-            error_list = get_error_list(r)
+            error_list = parse_errors(r)
             logging.error(error_list)
             if "The reporter specified is not a user." in error_list:
                 if "reporter" not in data["fields"]:

tests/test_resilientsession.py

@@ -6,6 +6,7 @@
 
 import jira.resilientsession
 from jira.exceptions import JIRAError
+from jira.resilientsession import parse_error_msg, parse_errors
 from tests.conftest import JiraTestCase
 
 
@@ -100,6 +101,31 @@ def test_status_codes_retries(
     assert mocked_sleep_method.call_count == expected_number_of_sleep_invocations
 
 
+errors_parsing_test_data = [
+    (403, {"x-authentication-denied-reason": "err1"}, "", ["err1"]),
+    (500, {}, "err1", ["err1"]),
+    (500, {}, '{"message": "err1"}', ["err1"]),
+    (500, {}, '{"errorMessages": "err1"}', ["err1"]),
+    (500, {}, '{"errorMessages": ["err1", "err2"]}', ["err1", "err2"]),
+    (500, {}, '{"errors": {"code1": "err1", "code2": "err2"}}', ["err1", "err2"]),
+]
+
+
+@pytest.mark.parametrize(
+    "status_code,headers,content,expected_errors",
+    errors_parsing_test_data,
+)
+def test_error_parsing(status_code, headers, content, expected_errors):
+    mocked_response: Response = Response()
+    mocked_response.status_code = status_code
+    mocked_response.headers.update(headers)
+    mocked_response._content = content.encode("utf-8")
+    errors = parse_errors(mocked_response)
+    assert errors == expected_errors
+    error_msg = parse_error_msg(mocked_response)
+    assert error_msg == ", ".join(expected_errors)
+
+
 def test_passthrough_class():
     # GIVEN: The passthrough class and a dict of request args
     passthrough_class = jira.resilientsession.PassthroughRetryPrepare()