Fix various problems in minecraft.authentication and its tests:

- Return value of _make_request() is treated as a requests.Request, when it is in fact a requests.Response.
- Some tests in test_authentication use assertRaises() incorrectly, resulting in testing code that never gets run.
- Other miscellaneous errors exposed by the above changes.

Additionally:
- YggdrasilError instances now have fields with specific error information, and _raise_from_response() populates them. (This will be useful for later changes.)

minecraft/authentication.py

@@ -110,11 +110,11 @@ class AuthenticationToken(object):
             "password": password
         }
 
-        req = _make_request(AUTH_SERVER, "authenticate", payload)
+        res = _make_request(AUTH_SERVER, "authenticate", payload)
 
-        _raise_from_request(req)
+        _raise_from_response(res)
 
-        json_resp = req.json()
+        json_resp = res.json()
 
         self.username = username
         self.access_token = json_resp["accessToken"]
@@ -145,13 +145,13 @@ class AuthenticationToken(object):
         if self.client_token is None:
             raise ValueError("'client_token' is not set!")
 
-        req = _make_request(AUTH_SERVER,
+        res = _make_request(AUTH_SERVER,
                             "refresh", {"accessToken": self.access_token,
                                         "clientToken": self.client_token})
 
-        _raise_from_request(req)
+        _raise_from_response(res)
 
-        json_resp = req.json()
+        json_resp = res.json()
 
         self.access_token = json_resp["accessToken"]
         self.client_token = json_resp["clientToken"]
@@ -177,12 +177,12 @@ class AuthenticationToken(object):
         if self.access_token is None:
             raise ValueError("'access_token' not set!")
 
-        req = _make_request(AUTH_SERVER, "validate",
+        res = _make_request(AUTH_SERVER, "validate",
                             {"accessToken": self.access_token})
 
         # Validate returns 204 to indicate success
         # http://wiki.vg/Authentication#Response_3
-        if req.status_code == 204:
+        if res.status_code == 204:
             return True
 
     @staticmethod
@@ -202,10 +202,10 @@ class AuthenticationToken(object):
         Raises:
             minecraft.exceptions.YggdrasilError
         """
-        req = _make_request(AUTH_SERVER, "signout",
+        res = _make_request(AUTH_SERVER, "signout",
                             {"username": username, "password": password})
 
-        if _raise_from_request(req) is None:
+        if _raise_from_response(res) is None:
             return True
 
     def invalidate(self):
@@ -219,12 +219,12 @@ class AuthenticationToken(object):
         Raises:
             :class:`minecraft.exceptions.YggdrasilError`
         """
-        req = _make_request(AUTH_SERVER, "invalidate",
+        res = _make_request(AUTH_SERVER, "invalidate",
                             {"accessToken": self.access_token,
                              "clientToken": self.client_token})
 
-        if req.status_code != 204:
+        if res.status_code != 204:
-            _raise_from_request(req)
+            _raise_from_response(res)
         return True
 
     def join(self, server_id):
@@ -246,13 +246,13 @@ class AuthenticationToken(object):
             err = "AuthenticationToken hasn't been authenticated yet!"
             raise YggdrasilError(err)
 
-        req = _make_request(SESSION_SERVER, "join",
+        res = _make_request(SESSION_SERVER, "join",
                             {"accessToken": self.access_token,
                              "selectedProfile": self.profile.to_dict(),
                              "serverId": server_id})
 
-        if req.status_code != 204:
+        if res.status_code != 204:
-            _raise_from_request(req)
+            _raise_from_response(res)
         return True
 
 
@@ -268,31 +268,39 @@ def _make_request(server, endpoint, data):
     Returns:
         A `requests.Request` object.
     """
-    req = requests.post(server + "/" + endpoint, data=json.dumps(data),
+    res = requests.post(server + "/" + endpoint, data=json.dumps(data),
                         headers=HEADERS)
-    return req
+    return res
 
 
-def _raise_from_request(req):
+def _raise_from_response(res):
     """
     Raises an appropriate `YggdrasilError` based on the `status_code` and
     `json` of a `requests.Request` object.
     """
-    if req.status_code == requests.codes['ok']:
+    if res.status_code == requests.codes['ok']:
         return None
 
+    exception = YggdrasilError()
+    exception.status_code = res.status_code
+
     try:
-        json_resp = req.json()
+        json_resp = res.json()
-
+        if not ("error" in json_resp and "errorMessage" in json_resp):
-        if "error" not in json_resp and "errorMessage" not in json_resp:
+            raise ValueError
-            raise YggdrasilError("Malformed error message.")
+    except ValueError:
-
+        message = "[{status_code}] Malformed error message: '{response_text}'"
+        message = message.format(status_code=str(res.status_code),
+                                 response_text=res.text)
+        exception.args = (message,)
+    else:
         message = "[{status_code}] {error}: '{error_message}'"
-        message = message.format(status_code=str(req.status_code),
+        message = message.format(status_code=str(res.status_code),
                                  error=json_resp["error"],
                                  error_message=json_resp["errorMessage"])
-    except ValueError as e:
+        exception.args = (message,)
-        message = "Unknown requests error. Status code: {}. Error: {}"
+        exception.yggdrasil_error = json_resp["error"]
-        message.format(str(req.status_code), e)
+        exception.yggdrasil_message = json_resp["errorMessage"]
+        exception.yggdrasil_cause = json_resp.get("cause")
 
-    raise YggdrasilError(message)
+    raise exception

minecraft/exceptions.py

@@ -7,6 +7,23 @@ class YggdrasilError(Exception):
     """
     Base `Exception` for the Yggdrasil authentication service.
     """
+    def __init__(
+        self,
+        message=None,
+        status_code=None,
+        yggdrasil_error=None,
+        yggdrasil_message=None,
+        yggdrasil_cause=None,
+        *args
+    ):
+        if message is not None:
+            args = (message,) + args
+        super(YggdrasilError, self).__init__(*args)
+
+        self.status_code = status_code
+        self.yggdrasil_error = yggdrasil_error
+        self.yggdrasil_message = yggdrasil_message
+        self.yggdrasil_cause = yggdrasil_cause
 
 
 class VersionMismatch(Exception):

tests/test_authentication.py

@@ -1,7 +1,7 @@
 from minecraft.authentication import Profile
 from minecraft.authentication import AuthenticationToken
 from minecraft.authentication import _make_request
-from minecraft.authentication import _raise_from_request
+from minecraft.authentication import _raise_from_response
 from minecraft.exceptions import YggdrasilError
 
 import requests
@@ -179,11 +179,11 @@ class AuthenticateAuthenticationToken(unittest.TestCase):
         a = AuthenticationToken()
 
         # We assume these aren't actual, valid credentials.
-        with self.assertRaises(YggdrasilError) as e:
+        with self.assertRaises(YggdrasilError) as cm:
             a.authenticate("Billy", "The Goat")
 
-            err = "Invalid Credentials. Invalid username or password."
+        err = "Invalid credentials. Invalid username or password."
-            self.assertEqual(e.error, err)
+        self.assertEqual(cm.exception.yggdrasil_message, err)
 
     @unittest.skipIf(should_skip_cred_test(),
                      "Need credentials to perform test.")
@@ -196,8 +196,8 @@ class AuthenticateAuthenticationToken(unittest.TestCase):
 
 class MakeRequest(unittest.TestCase):
     def test_make_request_http_method(self):
-        req = _make_request(AUTHSERVER, "authenticate", {"Billy": "Bob"})
+        res = _make_request(AUTHSERVER, "authenticate", {"Billy": "Bob"})
-        self.assertEqual(req.request.method, "POST")
+        self.assertEqual(res.request.method, "POST")
 
     def test_make_request_json_dump(self):
         data = {"Marie": "McGee",
@@ -208,63 +208,72 @@ class MakeRequest(unittest.TestCase):
                 "Listly": ["listling1", 2, "listling 3"]
                 }
 
-        req = _make_request(AUTHSERVER, "authenticate", data)
+        res = _make_request(AUTHSERVER, "authenticate", data)
-        self.assertEqual(req.request.body, json.dumps(data))
+        self.assertEqual(res.request.body, json.dumps(data))
 
     def test_make_request_url(self):
         URL = "https://authserver.mojang.com/authenticate"
-        req = _make_request(AUTHSERVER, "authenticate", {"Darling": "Diary"})
+        res = _make_request(AUTHSERVER, "authenticate", {"Darling": "Diary"})
-        self.assertEqual(req.request.url, URL)
+        self.assertEqual(res.request.url, URL)
 
 
 class RaiseFromRequest(unittest.TestCase):
     def test_raise_from_erroneous_request(self):
-        err_req = requests.Request()
+        err_res = mock.NonCallableMock(requests.Response)
-        err_req.status_code = 401
+        err_res.status_code = 401
-        err_req.json = mock.MagicMock(
+        err_res.json = mock.MagicMock(
             return_value={"error": "ThisIsAnException",
                           "errorMessage": "Went wrong."})
+        err_res.text = json.dumps(err_res.json())
 
-        with self.assertRaises(YggdrasilError) as e:
+        with self.assertRaises(YggdrasilError) as cm:
-            _raise_from_request(err_req)
+            _raise_from_response(err_res)
-            self.assertEqual(e, "[401]) ThisIsAnException: Went Wrong.")
+
+        message = "[401] ThisIsAnException: 'Went wrong.'"
+        self.assertEqual(str(cm.exception), message)
 
     def test_raise_invalid_json(self):
-        err_req = requests.Request()
+        err_res = mock.NonCallableMock(requests.Response)
-        err_req.status_code = 401
+        err_res.status_code = 401
-        err_req.json = mock.MagicMock(
+        err_res.json = mock.MagicMock(
             side_effect=ValueError("no json could be decoded")
         )
+        err_res.text = "{sample invalid json}"
 
-        with self.assertRaises(YggdrasilError) as e:
+        with self.assertRaises(YggdrasilError) as cm:
-            _raise_from_request(err_req)
+            _raise_from_response(err_res)
-            self.assertTrue("Unknown requests error" in e)
 
-    def test_raise_from_erroneous_request_without_error(self):
+        message_start = "[401] Malformed error message"
-        err_req = requests.Request()
+        self.assertTrue(str(cm.exception).startswith(message_start))
-        err_req.status_code = 401
-        err_req.json = mock.MagicMock(return_value={"goldfish": "are pretty."})
 
-        with self.assertRaises(YggdrasilError) as e:
+    def test_raise_from_erroneous_response_without_error(self):
-            _raise_from_request(err_req)
+        err_res = mock.NonCallableMock(requests.Response)
+        err_res.status_code = 401
+        err_res.json = mock.MagicMock(return_value={"goldfish": "are pretty."})
+        err_res.text = json.dumps(err_res.json())
 
-            self.assertEqual(e, "Malformed error message.")
+        with self.assertRaises(YggdrasilError) as cm:
+            _raise_from_response(err_res)
 
-    def test_raise_from_healthy_request(self):
+        message_start = "[401] Malformed error message"
-        req = requests.Request()
+        self.assertTrue(str(cm.exception).startswith(message_start))
-        req.status_code = 200
-        req.json = mock.MagicMock(return_value={"vegetables": "are healthy."})
 
-        self.assertIs(_raise_from_request(req), None)
+    def test_raise_from_healthy_response(self):
+        res = mock.NonCallableMock(requests.Response)
+        res.status_code = 200
+        res.json = mock.MagicMock(return_value={"vegetables": "are healthy."})
+        res.text = json.dumps(res.json())
+
+        self.assertIs(_raise_from_response(res), None)
 
 
 class NormalConnectionProcedure(unittest.TestCase):
     def test_login_connect_and_logout(self):
         a = AuthenticationToken()
 
-        successful_req = requests.Request()
+        successful_res = mock.NonCallableMock(requests.Response)
-        successful_req.status_code = 200
+        successful_res.status_code = 200
-        successful_req.json = mock.MagicMock(
+        successful_res.json = mock.MagicMock(
             return_value={"accessToken": "token",
                           "clientToken": "token",
                           "selectedProfile": {
@@ -272,33 +281,35 @@ class NormalConnectionProcedure(unittest.TestCase):
                               "name": "asdf"
                           }}
         )
+        successful_res.text = json.dumps(successful_res.json())
 
-        error_req = requests.Request()
+        error_res = mock.NonCallableMock(requests.Response)
-        error_req.status_code = 400
+        error_res.status_code = 400
-        error_req.json = mock.MagicMock(
+        error_res.json = mock.MagicMock(
             return_value={
                 "error": "invalid request",
                 "errorMessage": "invalid request"
             }
         )
+        error_res.text = json.dumps(error_res.json())
 
         def mocked_make_request(server, endpoint, data):
             if endpoint == "authenticate":
-                return successful_req
+                return successful_res
             if endpoint == "refresh" and data["accessToken"] == "token":
-                return successful_req
+                return successful_res
             if (endpoint == "validate" and data["accessToken"] == "token") \
                     or endpoint == "join":
-                r = requests.Request()
+                r = requests.Response()
                 r.status_code = 204
                 r.raise_for_status = mock.MagicMock(return_value=None)
                 return r
             if endpoint == "signout":
-                return successful_req
+                return successful_res
             if endpoint == "invalidate":
-                return successful_req
+                return successful_res
 
-            return error_req
+            return error_res
 
         # Test a successful sequence of events
         with mock.patch("minecraft.authentication._make_request",
@@ -327,7 +338,7 @@ class NormalConnectionProcedure(unittest.TestCase):
 
         # Failures
         with mock.patch("minecraft.authentication._make_request",
-                        return_value=error_req) as _make_request_mock:
+                        return_value=error_res) as _make_request_mock:
             self.assertFalse(a.authenticated)
 
             a.client_token = "token"

tests/test_exceptions.py

@@ -9,7 +9,7 @@ class RaiseYggdrasilError(unittest.TestCase):
             raise YggdrasilError
 
     def test_raise_yggdrasil_error_message(self):
-        with self.assertRaises(YggdrasilError) as e:
+        with self.assertRaises(YggdrasilError) as cm:
             raise YggdrasilError("Error!")
 
-        self.assertEqual(str(e.exception), "Error!")
+        self.assertEqual(str(cm.exception), "Error!")