diff --git a/msal/exceptions.py b/msal/exceptions.py index 5e9ee15..a6da67b 100644 --- a/msal/exceptions.py +++ b/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 diff --git a/msal/throttled_http_client.py b/msal/throttled_http_client.py index 2e79a60..7c64fbf 100644 --- a/msal/throttled_http_client.py +++ b/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): diff --git a/tests/test_throttled_http_client.py b/tests/test_throttled_http_client.py index fecb98d..6fbd0ed 100644 --- a/tests/test_throttled_http_client.py +++ b/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"))