[http-client-python] refine exception handling logic

packages/http-client-python/emitter/src/http.ts

@@ -379,7 +379,7 @@ function emitHttpResponse(
     headers: response.headers.map((x) => emitHttpResponseHeader(context, x)),
     statusCodes:
       typeof statusCodes === "object"
-        ? [(statusCodes as HttpStatusCodeRange).start]
+        ? [[(statusCodes as HttpStatusCodeRange).start, (statusCodes as HttpStatusCodeRange).end]]
         : statusCodes === "*"
           ? ["default"]
           : [statusCodes],

packages/http-client-python/generator/pygen/codegen/__init__.py

@@ -241,6 +241,16 @@ def _validate_code_model_options(self) -> None:
         if not self.options_retriever.is_azure_flavor and self.options_retriever.tracing:
             raise ValueError("Can only have tracing turned on for Azure SDKs.")
 
+    @staticmethod
+    def sort_exceptions(yaml_data: Dict[str, Any]) -> None:
+        for client in yaml_data["clients"]:
+            for group in client["operationGroups"]:
+                for operation in group["operations"]:
+                    if not operation.get("exceptions"):
+                        continue
+                    # sort exceptions by status code, first single status code, then range, then default
+                    operation["exceptions"] = sorted(operation["exceptions"], key=lambda x: 3 if x["statusCodes"][0] == "default" else (1 if isinstance(x["statusCodes"][0], int) else 2))
+
     @staticmethod
     def remove_cloud_errors(yaml_data: Dict[str, Any]) -> None:
         for client in yaml_data["clients"]:
@@ -315,6 +325,8 @@ def process(self) -> bool:
         self._validate_code_model_options()
         options = self._build_code_model_options()
         yaml_data = self.get_yaml()
+
+        self.sort_exceptions(yaml_data)
 
         if self.options_retriever.azure_arm:
             self.remove_cloud_errors(yaml_data)

packages/http-client-python/generator/pygen/codegen/models/model_type.py

@@ -348,7 +348,7 @@ def serialization_type(self) -> str:
 
     @property
     def instance_check_template(self) -> str:
-        return "isinstance({}, _model_base.Model)"
+        return "isinstance({}, " + f"_models.{self.name})"
 
     def imports(self, **kwargs: Any) -> FileImport:
         file_import = super().imports(**kwargs)

packages/http-client-python/generator/pygen/codegen/models/operation.py

@@ -9,6 +9,7 @@
     List,
     Any,
     Optional,
+    Tuple,
     Union,
     TYPE_CHECKING,
     Generic,
@@ -201,17 +202,11 @@ def default_error_deserialization(self) -> Optional[str]:
         exception_schema = default_exceptions[0].type
         if isinstance(exception_schema, ModelType):
             return exception_schema.type_annotation(skip_quote=True)
-        # in this case, it's just an AnyType
-        return "'object'"
+        return None
 
     @property
     def non_default_errors(self) -> List[Response]:
-        return [e for e in self.exceptions if "default" not in e.status_codes]
-
-    @property
-    def non_default_error_status_codes(self) -> List[Union[str, int]]:
-        """Actually returns all of the status codes from exceptions (besides default)"""
-        return list(chain.from_iterable([error.status_codes for error in self.non_default_errors]))
+        return [e for e in self.exceptions if "default" not in e.status_codes and e.type and isinstance(e.type, ModelType)]
 
     def _imports_shared(self, async_mode: bool, **kwargs: Any) -> FileImport:  # pylint: disable=unused-argument
         file_import = FileImport(self.code_model)
@@ -344,19 +339,7 @@ def imports(  # pylint: disable=too-many-branches, disable=too-many-statements
             file_import.add_submodule_import("exceptions", error, ImportType.SDKCORE)
         if self.code_model.options["azure_arm"]:
             file_import.add_submodule_import("azure.mgmt.core.exceptions", "ARMErrorFormat", ImportType.SDKCORE)
-        if self.non_default_errors:
-            file_import.add_submodule_import(
-                "typing",
-                "Type",
-                ImportType.STDLIB,
-            )
         file_import.add_mutable_mapping_import()
-        if self.non_default_error_status_codes:
-            file_import.add_submodule_import(
-                "typing",
-                "cast",
-                ImportType.STDLIB,
-            )
 
         if self.has_kwargs_to_pop_with_default(
             self.parameters.kwargs_to_pop, ParameterLocation.HEADER  # type: ignore
@@ -436,7 +419,7 @@ def imports(  # pylint: disable=too-many-branches, disable=too-many-statements
             elif any(r.type for r in self.responses):
                 file_import.add_submodule_import(f"{relative_path}_model_base", "_deserialize", ImportType.LOCAL)
             if self.default_error_deserialization or self.non_default_errors:
-                file_import.add_submodule_import(f"{relative_path}_model_base", "_deserialize", ImportType.LOCAL)
+                file_import.add_submodule_import(f"{relative_path}_model_base", "_failsafe_deserialize", ImportType.LOCAL)
         return file_import
 
     def get_response_from_status(self, status_code: Optional[Union[str, int]]) -> ResponseType:

packages/http-client-python/generator/pygen/codegen/models/response.py

@@ -54,7 +54,7 @@ def __init__(
         type: Optional[BaseType] = None,
     ) -> None:
         super().__init__(yaml_data=yaml_data, code_model=code_model)
-        self.status_codes: List[Union[int, str]] = yaml_data["statusCodes"]
+        self.status_codes: List[Union[int, str, List[int]]] = yaml_data["statusCodes"]
         self.headers = headers or []
         self.type = type
         self.nullable = yaml_data.get("nullable")

packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py

@@ -61,14 +61,6 @@ def _all_same(data: List[List[str]]) -> bool:
     return len(data) > 1 and all(sorted(data[0]) == sorted(data[i]) for i in range(1, len(data)))
 
 
-def _need_type_ignore(builder: OperationType) -> bool:
-    for e in builder.non_default_errors:
-        for status_code in e.status_codes:
-            if status_code in (401, 404, 409, 304):
-                return True
-    return False
-
-
 def _xml_config(send_xml: bool, content_types: List[str]) -> str:
     if not (send_xml and "xml" in str(content_types)):
         return ""
@@ -999,20 +991,80 @@ def handle_error_response(self, builder: OperationType) -> List[str]:
         elif isinstance(builder.stream_value, str):  # _stream is not sure, so we need to judge it
             retval.append("    if _stream:")
             retval.extend([f"    {l}" for l in response_read])
-        type_ignore = "  # type: ignore" if _need_type_ignore(builder) else ""
         retval.append(
-            f"    map_error(status_code=response.status_code, response=response, error_map=error_map){type_ignore}"
+            f"    map_error(status_code=response.status_code, response=response, error_map=error_map)"
         )
         error_model = ""
+        if builder.non_default_errors and self.code_model.options["models_mode"]:
+            error_model = ", model=error"
+            condition = "if"
+            retval.append("    error = None")
+            for e in builder.non_default_errors:
+                # single status code
+                if isinstance(e.status_codes[0], int):
+                    for status_code in e.status_codes:
+                        retval.append(f"    {condition} response.status_code == {status_code}:")
+                        if self.code_model.options["models_mode"] == "dpg":
+                            retval.append(f"        error = _failsafe_deserialize({e.type.type_annotation(is_operation_file=True, skip_quote=True)},  response.json())")
+                        else:
+                            retval.append(
+                                f"        error = self._deserialize.failsafe_deserialize({e.type.type_annotation(is_operation_file=True, skip_quote=True)}, "
+                                "pipeline_response)"
+                            )
+                        # add build-in error type
+                        # TODO: we should decide whether need to this wrapper for customized error type
+                        if status_code == 401:
+                            retval.append(
+                                "        raise ClientAuthenticationError(response=response{}{})".format(
+                                    error_model,
+                                    (", error_format=ARMErrorFormat" if self.code_model.options["azure_arm"] else ""),
+                                )
+                            )
+                        elif status_code == 404:
+                            retval.append(
+                                "        raise ResourceNotFoundError(response=response{}{})".format(
+                                    error_model,
+                                    (", error_format=ARMErrorFormat" if self.code_model.options["azure_arm"] else ""),
+                                )
+                            )
+                        elif status_code == 409:
+                            retval.append(
+                                "        raise ResourceExistsError(response=response{}{})".format(
+                                    error_model,
+                                    (", error_format=ARMErrorFormat" if self.code_model.options["azure_arm"] else ""),
+                                )
+                            )
+                        elif status_code == 304:
+                            retval.append(
+                                "        raise ResourceNotModifiedError(response=response{}{})".format(
+                                    error_model,
+                                    (", error_format=ARMErrorFormat" if self.code_model.options["azure_arm"] else ""),
+                                )
+                            )
+                # ranged status code only exist in typespec and will not have multiple status codes
+                else:
+                    retval.append(f"    {condition} {e.status_codes[0][0]} <= response.status_code <= {e.status_codes[0][1]}:")
+                    if self.code_model.options["models_mode"] == "dpg":
+                        retval.append(f"        error = _failsafe_deserialize({e.type.type_annotation(is_operation_file=True, skip_quote=True)},  response.json())")
+                    else:
+                        retval.append(
+                            f"        error = self._deserialize.failsafe_deserialize({e.type.type_annotation(is_operation_file=True, skip_quote=True)}, "
+                            "pipeline_response)"
+                        )
+                condition = "elif"
+        # default error handling
         if builder.default_error_deserialization and self.code_model.options["models_mode"]:
+            error_model = ", model=error"
+            indent = "        " if builder.non_default_errors else "    "
+            if builder.non_default_errors:
+                retval.append("    else:")
             if self.code_model.options["models_mode"] == "dpg":
-                retval.append(f"    error = _deserialize({builder.default_error_deserialization},  response.json())")
+                retval.append(f"{indent}error = _failsafe_deserialize({builder.default_error_deserialization},  response.json())")
             else:
                 retval.append(
-                    f"    error = self._deserialize.failsafe_deserialize({builder.default_error_deserialization}, "
+                    f"{indent}error = self._deserialize.failsafe_deserialize({builder.default_error_deserialization}, "
                     "pipeline_response)"
                 )
-            error_model = ", model=error"
         retval.append(
             "    raise HttpResponseError(response=response{}{})".format(
                 error_model,
@@ -1085,60 +1137,28 @@ def handle_response(self, builder: OperationType) -> List[str]:
             retval.append("return 200 <= response.status_code <= 299")
         return retval
 
+    def _need_specific_error_map(self, code: int, builder: OperationType) -> bool:
+        for non_default_error in builder.non_default_errors:
+            # single status code
+            if code in non_default_error.status_codes:
+                return False
+            # ranged status code
+            if isinstance(non_default_error.status_codes[0], list) and non_default_error.status_codes[0][0] <= code <= non_default_error.status_codes[0][1]:
+                return False
+        return True
+
     def error_map(self, builder: OperationType) -> List[str]:
         retval = ["error_map: MutableMapping = {"]
-        if builder.non_default_errors:
-            if not 401 in builder.non_default_error_status_codes:
+        if builder.non_default_errors and self.code_model.options["models_mode"]:
+            # TODO: we should decide whether to add the build-in error map when there is a customized default error type
+            if self._need_specific_error_map(401, builder):
                 retval.append("    401: ClientAuthenticationError,")
-            if not 404 in builder.non_default_error_status_codes:
+            if self._need_specific_error_map(404, builder):
                 retval.append("    404: ResourceNotFoundError,")
-            if not 409 in builder.non_default_error_status_codes:
+            if self._need_specific_error_map(409, builder):
                 retval.append("    409: ResourceExistsError,")
-            if not 304 in builder.non_default_error_status_codes:
+            if self._need_specific_error_map(304, builder):
                 retval.append("    304: ResourceNotModifiedError,")
-            for e in builder.non_default_errors:
-                error_model_str = ""
-                if isinstance(e.type, ModelType):
-                    if self.code_model.options["models_mode"] == "msrest":
-                        error_model_str = (
-                            f", model=self._deserialize(" f"_models.{e.type.serialization_type}, response)"
-                        )
-                    elif self.code_model.options["models_mode"] == "dpg":
-                        error_model_str = f", model=_deserialize(_models.{e.type.name}, response.json())"
-                error_format_str = ", error_format=ARMErrorFormat" if self.code_model.options["azure_arm"] else ""
-                for status_code in e.status_codes:
-                    if status_code == 401:
-                        retval.append(
-                            "    401:  cast(Type[HttpResponseError], "
-                            "lambda response: ClientAuthenticationError(response=response"
-                            f"{error_model_str}{error_format_str})),"
-                        )
-                    elif status_code == 404:
-                        retval.append(
-                            "    404:  cast(Type[HttpResponseError], "
-                            "lambda response: ResourceNotFoundError(response=response"
-                            f"{error_model_str}{error_format_str})),"
-                        )
-                    elif status_code == 409:
-                        retval.append(
-                            "    409:  cast(Type[HttpResponseError], "
-                            "lambda response: ResourceExistsError(response=response"
-                            f"{error_model_str}{error_format_str})),"
-                        )
-                    elif status_code == 304:
-                        retval.append(
-                            "    304:  cast(Type[HttpResponseError], "
-                            "lambda response: ResourceNotModifiedError(response=response"
-                            f"{error_model_str}{error_format_str})),"
-                        )
-                    elif not error_model_str and not error_format_str:
-                        retval.append(f"    {status_code}: HttpResponseError,")
-                    else:
-                        retval.append(
-                            f"    {status_code}: cast(Type[HttpResponseError], "
-                            "lambda response: HttpResponseError(response=response"
-                            f"{error_model_str}{error_format_str})),"
-                        )
         else:
             retval.append(
                 "    401: ClientAuthenticationError, 404: ResourceNotFoundError, 409: ResourceExistsError, "

packages/http-client-python/generator/pygen/codegen/templates/model_base.py.jinja2

@@ -892,6 +892,23 @@ def _deserialize(
     return _deserialize_with_callable(deserializer, value)
 
 
+def _failsafe_deserialize(
+  deserializer: typing.Any,
+  value: typing.Any,
+  module: typing.Optional[str] = None,
+  rf: typing.Optional["_RestField"] = None,
+  format: typing.Optional[str] = None,
+) -> typing.Any:
+  try:
+      return _deserialize(deserializer, value, module, rf, format)
+  except DeserializationError:
+      _LOGGER.warning(
+          "Ran into a deserialization error. Ignoring since this is failsafe deserialization",
+          exc_info=True
+      )
+      return None
+
+
 class _RestField:
     def __init__(
         self,