Raise appropriate exception when error_body is dict | Fix exception raised during handling of another exception (Azure#18147)

* Raise appropriate exception when error_body is dict

* Raise appropriate exception when error_body is dict

* Raise appropriate exception when error_body is dict

* cleaned up and accounted for missing error code

* Cleaned up - fixed py2 and fixed error during handling

* py2 check

* fixed errors and added more changes

* python 2 issue

* ci

* ci

* fixing ci

* fixed ci

* fixed ci

* linter

* linter

* linter

* fixed ci

* raise exception when there is no response in storage error

Author: tasherif-msft

sdk/storage/azure-storage-blob/azure/storage/blob/_shared/response_handlers.py

@@ -3,12 +3,12 @@
 # Licensed under the MIT License. See License.txt in the project root for
 # license information.
 # --------------------------------------------------------------------------
-
 from typing import (  # pylint: disable=unused-import
     Union, Optional, Any, Iterable, Dict, List, Type, Tuple,
     TYPE_CHECKING
 )
 import logging
+from xml.etree.ElementTree import Element
 
 from azure.core.pipeline.policies import ContentDecodePolicy
 from azure.core.exceptions import (
@@ -22,7 +22,6 @@
 from .parser import _to_utc_datetime
 from .models import StorageErrorCode, UserDelegationKey, get_enum_value
 
-
 if TYPE_CHECKING:
     from datetime import datetime
     from azure.core.exceptions import AzureError
@@ -83,29 +82,46 @@ def return_context_and_deserialized(response, deserialized, response_headers):
     return response.http_response.location_mode, deserialized
 
 
-def process_storage_error(storage_error):
-    # If storage_error is one of the two then it has already been processed and serialized to the specific exception.
-    if isinstance(storage_error, (PartialBatchErrorException, ClientAuthenticationError)):
-        raise storage_error
+def process_storage_error(storage_error):   # pylint:disable=too-many-statements
     raise_error = HttpResponseError
+    serialized = False
+    if not storage_error.response:
+        raise storage_error
+    # If it is one of those three then it has been serialized prior by the generated layer.
+    if isinstance(storage_error, (PartialBatchErrorException,
+                                  ClientAuthenticationError, ResourceNotFoundError, ResourceExistsError)):
+        serialized = True
     error_code = storage_error.response.headers.get('x-ms-error-code')
     error_message = storage_error.message
     additional_data = {}
+    error_dict = {}
     try:
         error_body = ContentDecodePolicy.deserialize_from_http_generics(storage_error.response)
-        if error_body:
-            for info in error_body.iter():
-                if info.tag.lower() == 'code':
-                    error_code = info.text
-                elif info.tag.lower() == 'message':
-                    error_message = info.text
-                else:
-                    additional_data[info.tag] = info.text
+        # If it is an XML response
+        if isinstance(error_body, Element):
+            error_dict = {
+                child.tag.lower(): child.text
+                for child in error_body
+            }
+        # If it is a JSON response
+        elif isinstance(error_body, dict):
+            error_dict = error_body.get('error', {})
+        elif not error_code:
+            _LOGGER.warning(
+                'Unexpected return type % from ContentDecodePolicy.deserialize_from_http_generics.', type(error_body))
+            error_dict = {'message': str(error_body)}
+
+        # If we extracted from a Json or XML response
+        if error_dict:
+            error_code = error_dict.get('code')
+            error_message = error_dict.get('message')
+            additional_data = {k: v for k, v in error_dict.items() if k not in {'code', 'message'}}
     except DecodeError:
         pass
 
     try:
-        if error_code:
+        # This check would be unnecessary if we have already serialized the error
+        if error_code and not serialized:
             error_code = StorageErrorCode(error_code)
             if error_code in [StorageErrorCode.condition_not_met,
                               StorageErrorCode.blob_overwritten]:
@@ -137,17 +153,30 @@ def process_storage_error(storage_error):
         # Got an unknown error code
         pass
 
+    # Error message should include all the error properties
     try:
         error_message += "\nErrorCode:{}".format(error_code.value)
     except AttributeError:
         error_message += "\nErrorCode:{}".format(error_code)
     for name, info in additional_data.items():
         error_message += "\n{}:{}".format(name, info)
 
-    error = raise_error(message=error_message, response=storage_error.response)
+    # No need to create an instance if it has already been serialized by the generated layer
+    if serialized:
+        storage_error.message = error_message
+        error = storage_error
+    else:
+        error = raise_error(message=error_message, response=storage_error.response)
+    # Ensure these properties are stored in the error instance as well (not just the error message)
     error.error_code = error_code
     error.additional_info = additional_data
-    error.raise_with_traceback()
+    # error.args is what's surfaced on the traceback - show error message in all cases
+    error.args = (error.message,)
+    try:
+        # `from None` prevents us from double printing the exception (suppresses generated layer error context)
+        exec("raise error from None")   # pylint: disable=exec-used # nosec
+    except SyntaxError:
+        raise error
 
 
 def parse_to_internal_user_delegation_key(service_user_delegation_key):

sdk/storage/azure-storage-file-datalake/azure/storage/filedatalake/_deserialize.py

@@ -3,11 +3,11 @@
 # Licensed under the MIT License. See License.txt in the project root for
 # license information.
 # --------------------------------------------------------------------------
-
 import logging
 from typing import (  # pylint: disable=unused-import
     TYPE_CHECKING
 )
+from xml.etree.ElementTree import Element
 
 from azure.core.pipeline.policies import ContentDecodePolicy
 from azure.core.exceptions import HttpResponseError, DecodeError, ResourceModifiedError, ClientAuthenticationError, \
@@ -111,26 +111,46 @@ def deserialize_metadata(response, obj, headers):  # pylint: disable=unused-argu
     return {k[10:]: v for k, v in raw_metadata.items()}
 
 
-def process_storage_error(storage_error):
+def process_storage_error(storage_error):   # pylint:disable=too-many-statements
     raise_error = HttpResponseError
+    serialized = False
+    if not storage_error.response:
+        raise storage_error
+    # If it is one of those three then it has been serialized prior by the generated layer.
+    if isinstance(storage_error, (ResourceNotFoundError, ClientAuthenticationError, ResourceExistsError)):
+        serialized = True
     error_code = storage_error.response.headers.get('x-ms-error-code')
     error_message = storage_error.message
     additional_data = {}
+    error_dict = {}
     try:
         error_body = ContentDecodePolicy.deserialize_from_http_generics(storage_error.response)
-        if error_body:
-            for info in error_body:
-                if info == 'code':
-                    error_code = error_body[info]
-                elif info == 'message':
-                    error_message = error_body[info]
-                else:
-                    additional_data[info] = error_body[info]
+        # If it is an XML response
+        if isinstance(error_body, Element):
+            error_dict = {
+                child.tag.lower(): child.text
+                for child in error_body
+            }
+        # If it is a JSON response
+        elif isinstance(error_body, dict):
+            error_dict = error_body.get('error', {})
+        elif not error_code:
+            _LOGGER.warning(
+                'Unexpected return type % from ContentDecodePolicy.deserialize_from_http_generics.', type(error_body))
+            error_dict = {'message': str(error_body)}
+
+        # If we extracted from a Json or XML response
+        if error_dict:
+            error_code = error_dict.get('code')
+            error_message = error_dict.get('message')
+            additional_data = {k: v for k, v in error_dict.items() if k not in {'code', 'message'}}
+
     except DecodeError:
         pass
 
     try:
-        if error_code:
+        # This check would be unnecessary if we have already serialized the error.
+        if error_code and not serialized:
             error_code = StorageErrorCode(error_code)
             if error_code in [StorageErrorCode.condition_not_met]:
                 raise_error = ResourceModifiedError
@@ -166,15 +186,27 @@ def process_storage_error(storage_error):
         # Got an unknown error code
         pass
 
+    # Error message should include all the error properties
     try:
         error_message += "\nErrorCode:{}".format(error_code.value)
     except AttributeError:
         error_message += "\nErrorCode:{}".format(error_code)
     for name, info in additional_data.items():
         error_message += "\n{}:{}".format(name, info)
 
-    error = raise_error(message=error_message, response=storage_error.response,
-                        continuation_token=storage_error.continuation_token)
+    # No need to create an instance if it has already been serialized by the generated layer
+    if serialized:
+        storage_error.message = error_message
+        error = storage_error
+    else:
+        error = raise_error(message=error_message, response=storage_error.response)
+    # Ensure these properties are stored in the error instance as well (not just the error message)
     error.error_code = error_code
     error.additional_info = additional_data
-    raise error
+    # error.args is what's surfaced on the traceback - show error message in all cases
+    error.args = (error.message,)
+    try:
+        # `from None` prevents us from double printing the exception (suppresses generated layer error context)
+        exec("raise error from None")   # pylint: disable=exec-used # nosec
+    except SyntaxError:
+        raise error

sdk/storage/azure-storage-file-datalake/azure/storage/filedatalake/_shared/response_handlers.py

@@ -3,12 +3,12 @@
 # Licensed under the MIT License. See License.txt in the project root for
 # license information.
 # --------------------------------------------------------------------------
-
 from typing import (  # pylint: disable=unused-import
     Union, Optional, Any, Iterable, Dict, List, Type, Tuple,
     TYPE_CHECKING
 )
 import logging
+from xml.etree.ElementTree import Element
 
 from azure.core.pipeline.policies import ContentDecodePolicy
 from azure.core.exceptions import (
@@ -83,26 +83,46 @@ def return_context_and_deserialized(response, deserialized, response_headers):
     return response.http_response.location_mode, deserialized
 
 
-def process_storage_error(storage_error):
+def process_storage_error(storage_error):   # pylint:disable=too-many-statements
     raise_error = HttpResponseError
+    serialized = False
+    if not storage_error.response:
+        raise storage_error
+    # If it is one of those three then it has been serialized prior by the generated layer.
+    if isinstance(storage_error, (PartialBatchErrorException,
+                                  ClientAuthenticationError, ResourceNotFoundError, ResourceExistsError)):
+        serialized = True
     error_code = storage_error.response.headers.get('x-ms-error-code')
     error_message = storage_error.message
     additional_data = {}
+    error_dict = {}
     try:
         error_body = ContentDecodePolicy.deserialize_from_http_generics(storage_error.response)
-        if error_body:
-            for info in error_body.iter():
-                if info.tag.lower() == 'code':
-                    error_code = info.text
-                elif info.tag.lower() == 'message':
-                    error_message = info.text
-                else:
-                    additional_data[info.tag] = info.text
+        # If it is an XML response
+        if isinstance(error_body, Element):
+            error_dict = {
+                child.tag.lower(): child.text
+                for child in error_body
+            }
+        # If it is a JSON response
+        elif isinstance(error_body, dict):
+            error_dict = error_body.get('error', {})
+        elif not error_code:
+            _LOGGER.warning(
+                'Unexpected return type % from ContentDecodePolicy.deserialize_from_http_generics.', type(error_body))
+            error_dict = {'message': str(error_body)}
+
+        # If we extracted from a Json or XML response
+        if error_dict:
+            error_code = error_dict.get('code')
+            error_message = error_dict.get('message')
+            additional_data = {k: v for k, v in error_dict.items() if k not in {'code', 'message'}}
     except DecodeError:
         pass
 
     try:
-        if error_code:
+        # This check would be unnecessary if we have already serialized the error
+        if error_code and not serialized:
             error_code = StorageErrorCode(error_code)
             if error_code in [StorageErrorCode.condition_not_met,
                               StorageErrorCode.blob_overwritten]:
@@ -134,17 +154,30 @@ def process_storage_error(storage_error):
         # Got an unknown error code
         pass
 
+    # Error message should include all the error properties
     try:
         error_message += "\nErrorCode:{}".format(error_code.value)
     except AttributeError:
         error_message += "\nErrorCode:{}".format(error_code)
     for name, info in additional_data.items():
         error_message += "\n{}:{}".format(name, info)
 
-    error = raise_error(message=error_message, response=storage_error.response)
+    # No need to create an instance if it has already been serialized by the generated layer
+    if serialized:
+        storage_error.message = error_message
+        error = storage_error
+    else:
+        error = raise_error(message=error_message, response=storage_error.response)
+    # Ensure these properties are stored in the error instance as well (not just the error message)
     error.error_code = error_code
     error.additional_info = additional_data
-    error.raise_with_traceback()
+    # error.args is what's surfaced on the traceback - show error message in all cases
+    error.args = (error.message,)
+    try:
+        # `from None` prevents us from double printing the exception (suppresses generated layer error context)
+        exec("raise error from None")   # pylint: disable=exec-used # nosec
+    except SyntaxError:
+        raise error
 
 
 def parse_to_internal_user_delegation_key(service_user_delegation_key):