Properly throw MsalServiceError exception

Author: rayluo

msal/exceptions.py

@@ -27,12 +27,26 @@
 
 class MsalError(Exception):
  # Define the template in Unicode to accommodate possible Unicode variables
- msg = u'An unspecified error'
+ msg = u'An unspecified error' # Keeping for backward compatibility
 
- def __init__(self, *args, **kwargs):
- super(MsalError, self).__init__(self.msg.format(**kwargs), *args)
- self.kwargs = kwargs
 
 class MsalServiceError(MsalError):
- msg = u"{error}: {error_description}"
+ msg = u"{error}: {error_description}" # Keeping for backward compatibility
+ def __init__(
+ self,
+ *args,
+ error: str, error_description: str, # Historically required, keeping them for now
+ # 1. We can't simply remove them, or else it will be a breaking change
+ # 2. We may change them to optional without breaking anyone. However,
+ # such a change will be a one-way change, because once being optional,
+ # we will never be able to change them (back) to be required.
+ # 3. Since they were required and already exist anyway,
+ # now we just keep them required "for now",
+ # just in case that we would use them again.
+ # There is no plan to do #1; and we keep option #2 open; we go with #3.
+ **kwargs,
+ ):
+ super().__init__(*args, **kwargs)
+ self._error = error
+ self._error_description = error_description
 

msal/throttled_http_client.py

@@ -66,7 +66,10 @@ def __init__(self, raw_response):
  # self.raise_for_status = raw_response.raise_for_status
  def raise_for_status(self):
  if self.status_code >= 400:
- raise MsalServiceError("HTTP Error: {}".format(self.status_code))
+ raise MsalServiceError(
+ "HTTP Error: {}".format(self.status_code),
+ error=None, error_description=None, # Historically required, keeping them for now
+ )
 
 
 class ThrottledHttpClientBase(object):

tests/test_throttled_http_client.py

@@ -6,6 +6,7 @@
 
 from msal.throttled_http_client import (
  ThrottledHttpClientBase, ThrottledHttpClient, NormalizedResponse)
+from msal.exceptions import MsalServiceError
 
 from tests import unittest
 from tests.http_client import MinimalResponse as _MinimalResponse
@@ -65,6 +66,26 @@ class CloseMethodCalled(Exception):
  pass
 
 
+class NormalizedResponseTestCase(unittest.TestCase):
+ def test_pickled_minimal_response_should_contain_signature(self):
+ self.assertIn(MinimalResponse.SIGNATURE, pickle.dumps(MinimalResponse(
+ status_code=200, headers={}, text="foo")))
+
+ def test_normalized_response_should_not_contain_signature(self):
+ response = NormalizedResponse(MinimalResponse(
+ status_code=200, headers={}, text="foo"))
+ self.assertNotIn(
+ MinimalResponse.SIGNATURE, pickle.dumps(response),
+ "A pickled object should not contain undesirable data")
+ self.assertEqual(response.text, "foo", "Should return the same response text")
+
+ def test_normalized_response_raise_for_status_should_raise(self):
+ response = NormalizedResponse(MinimalResponse(
+ status_code=400, headers={}, text="foo"))
+ with self.assertRaises(MsalServiceError):
+ response.raise_for_status()
+
+
 class ThrottledHttpClientBaseTestCase(unittest.TestCase):
 
  def assertCleanPickle(self, obj):
@@ -77,10 +98,6 @@ def assertValidResponse(self, response):
  self.assertIsInstance(response, NormalizedResponse)
  self.assertCleanPickle(response)
 
- def test_pickled_minimal_response_should_contain_signature(self):
- self.assertIn(MinimalResponse.SIGNATURE, pickle.dumps(MinimalResponse(
- status_code=200, headers={}, text="foo")))
-
  def test_throttled_http_client_base_response_should_tolerate_headerless_response(self):
  http_client = ThrottledHttpClientBase(DummyHttpClientWithoutResponseHeaders(
  status_code=200, response_text="foo"))