Repository: Azure/msrestazure-for-python

Issue:
CloudError parsing should be resilient if input type is string
In so (messy) scenario, we don't receive a dict (from a JSON), but a string. We should be robust to that and print the while string as the error message:
```python
msrest.http_logger : b'"{\\"error\\":{\\"code\\":\\"ResourceGroupNotFound\\",\\"message\\":\\"Resource group \'res_grp\' could not be found.\\"}}"'
'str' object has no attribute 'get'
Traceback (most recent call last):
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\cli\main.py", line 36, in main
    cmd_result = APPLICATION.execute(args)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\cli\core\application.py", line 212, in execute
    result = expanded_arg.func(params)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\cli\core\commands\__init__.py", line 377, in __call__
    return self.handler(*args, **kwargs)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\cli\core\commands\__init__.py", line 620, in _execute_command
    reraise(*sys.exc_info())
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\six.py", line 693, in reraise
    raise value
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\cli\core\commands\__init__.py", line 614, in _execute_command
    return list(result)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\msrest\paging.py", line 109, in __next__
    self.advance_page()
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\msrest\paging.py", line 95, in advance_page
    self._response = self._get_next(self.next_link)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\azure\mgmt\compute\v2017_03_30\operations\disks_operations.py", line 441, in internal_paging
    exp = CloudError(response)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\msrestazure\azure_exceptions.py", line 136, in __init__
    self._build_error_data(response)
  File "C:\Users\lmazuel\Git\AzureCli\lib\site-packages\msrestazure\azure_exceptions.py", line 156, in _build_error_data
    data = data.get('error', data)
AttributeError: 'str' object has no attribute 'get'
```

Discussion:

Patch:
diff --git a/msrestazure/azure_exceptions.py b/msrestazure/azure_exceptions.py
index bb85333..5b4792c 100644
--- a/msrestazure/azure_exceptions.py
+++ b/msrestazure/azure_exceptions.py
@@ -30,6 +30,15 @@ from msrest.exceptions import ClientException
 from msrest.serialization import Deserializer
 from msrest.exceptions import DeserializationError
 
+class CloudErrorRoot(object):
+    """Just match the "error" key at the root of a OdataV4 JSON.
+    """
+    _validation = {}
+    _attribute_map = {
+        'error': {'key': 'error', 'type': 'CloudErrorData'},
+    }
+    def __init__(self, error):
+        self.error = error
 
 class CloudErrorData(object):
     """Cloud Error Data object, deserialized from error data returned
@@ -47,7 +56,7 @@ class CloudErrorData(object):
 
     def __init__(self, *args, **kwargs):
         self.error = kwargs.get('error')
-        self._message = kwargs.get('message')
+        self.message = kwargs.get('message')
         self.request_id = None
         self.error_time = None
         self.target = kwargs.get('target')
@@ -122,7 +131,10 @@ class CloudError(ClientException):
     """
 
     def __init__(self, response, error=None, *args, **kwargs):
-        self.deserializer = Deserializer({'CloudErrorData': CloudErrorData})
+        self.deserializer = Deserializer({
+            'CloudErrorRoot': CloudErrorRoot,
+            'CloudErrorData': CloudErrorData
+        })
         self.error = None
         self.message = None
         self.response = response
@@ -149,13 +161,7 @@ class CloudError(ClientException):
 
     def _build_error_data(self, response):
         try:
-            data = response.json()
-        except ValueError:
-            data = response
-        else:
-            data = data.get('error', data)
-        try:
-            self.error = self.deserializer(CloudErrorData(), data)
+            self.error = self.deserializer('CloudErrorRoot', response).error
         except DeserializationError:
             self.error = None
         else:
@@ -178,7 +184,10 @@ class CloudError(ClientException):
         except ValueError:
             message = "none"
         else:
-            message = data.get("message", self._get_state(data))
+            try:
+                message = data.get("message", self._get_state(data))
+            except AttributeError: # data is not a dict, but is a requests.Response parsable as JSON
+                message = str(response.text)
         try:
             response.raise_for_status()
         except RequestException as err:


Test patch:
diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py
index 2506a9c..45a4770 100644
--- a/tests/test_exceptions.py
+++ b/tests/test_exceptions.py
@@ -125,20 +125,6 @@ class TestCloudException(unittest.TestCase):
         response.headers = {"content-type": "application/json; charset=utf8"}
         response.reason = 'BadRequest'
 
-        message = {
-            'code': '500',
-            'message': {'value': 'Bad Request\nRequest:34875\nTime:1999-12-31T23:59:59-23:59'},
-            'values': {'invalid_attribute':'data'}
-            }
-
-        response.text = json.dumps(message)
-        response.json = lambda: json.loads(response.text)
-
-        error = CloudError(response)
-        self.assertEqual(error.message, 'Bad Request')
-        self.assertEqual(error.status_code, 400)
-        self.assertIsInstance(error.error, CloudErrorData)
-
         message = { 'error': {
             'code': '500',
             'message': {'value': 'Bad Request\nRequest:34875\nTime:1999-12-31T23:59:59-23:59'},
@@ -146,6 +132,7 @@ class TestCloudException(unittest.TestCase):
             }}
 
         response.text = json.dumps(message)
+        response.json = lambda: json.loads(response.text)
         error = CloudError(response)
         self.assertEqual(error.message, 'Bad Request')
         self.assertEqual(error.status_code, 400)
@@ -175,9 +162,9 @@ class TestCloudException(unittest.TestCase):
 
         response.text = '{\r\n  "odata.metadata":"https://account.region.batch.azure.com/$metadata#Microsoft.Azure.Batch.Protocol.Entities.Container.errors/@Element","code":"InvalidHeaderValue","message":{\r\n    "lang":"en-US","value":"The value for one of the HTTP headers is not in the correct format.\\nRequestId:5f4c1f05-603a-4495-8e80-01f776310bbd\\nTime:2016-01-04T22:12:33.9245931Z"\r\n  },"values":[\r\n    {\r\n      "key":"HeaderName","value":"Content-Type"\r\n    },{\r\n      "key":"HeaderValue","value":"application/json; odata=minimalmetadata; charset=utf-8"\r\n    }\r\n  ]\r\n}'
         error = CloudError(response)
-        self.assertIsInstance(error.error, CloudErrorData)
+        self.assertIn("The value for one of the HTTP headers is not in the correct format", error.message)
 
-        response.text = '{"code":"Conflict","message":"The maximum number of Free ServerFarms allowed in a Subscription is 10.","target":null,"details":[{"message":"The maximum number of Free ServerFarms allowed in a Subscription is 10."},{"code":"Conflict"},{"errorentity":{"code":"Conflict","message":"The maximum number of Free ServerFarms allowed in a Subscription is 10.","extendedCode":"59301","messageTemplate":"The maximum number of {0} ServerFarms allowed in a Subscription is {1}.","parameters":["Free","10"],"innerErrors":null}}],"innererror":null}'
+        response.text = '{"error":{"code":"Conflict","message":"The maximum number of Free ServerFarms allowed in a Subscription is 10.","target":null,"details":[{"message":"The maximum number of Free ServerFarms allowed in a Subscription is 10."},{"code":"Conflict"},{"errorentity":{"code":"Conflict","message":"The maximum number of Free ServerFarms allowed in a Subscription is 10.","extendedCode":"59301","messageTemplate":"The maximum number of {0} ServerFarms allowed in a Subscription is {1}.","parameters":["Free","10"],"innerErrors":null}}],"innererror":null}}'
         error = CloudError(response)
         self.assertIsInstance(error.error, CloudErrorData)
         self.assertEqual(error.error.error, "Conflict")
@@ -199,6 +186,11 @@ class TestCloudException(unittest.TestCase):
         self.assertIsInstance(error.error, CloudErrorData)
         self.assertEqual(error.error.error, "BadArgument")
 
+        # See https://github.com/Azure/msrestazure-for-python/issues/54
+        response.text = '"{\\"error\\": {\\"code\\": \\"ResourceGroupNotFound\\", \\"message\\": \\"Resource group \'res_grp\' could not be found.\\"}}"'
+        error = CloudError(response)
+        self.assertIn(response.text, error.message)
+
 
 if __name__ == '__main__':
     unittest.main()