{Error Improvement} Adjust error types (#15865)

* adjust error types

* adjust error handling guideline

* refine error handling guidelines

* replace UnknownError

* Update .github/pull_request_template.md

Co-authored-by: Yishi Wang <yishiwang@microsoft.com>

Co-authored-by: Yishi Wang <yishiwang@microsoft.com>

.github/pull_request_template.md

@@ -17,3 +17,5 @@ This checklist is used to make sure that common guidelines for a pull request ar
 - [ ] The PR title and description has followed the guideline in [Submitting Pull Requests](https://github.com/Azure/azure-cli/tree/dev/doc/authoring_command_modules#submitting-pull-requests).
 
 - [ ] I adhere to the [Command Guidelines](https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md).
+
+- [ ] I adhere to the [Error Handling Guidelines](https://github.com/Azure/azure-cli/blob/dev/doc/error_handling_guidelines.md).

doc/error_handling_guidelines.md

@@ -17,14 +17,14 @@ For the new commands authoring, here are what need to be done to get onboard. __
 The newly designed error types are provided in [azure/cli/core/azclierror.py](https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/azclierror.py), where the error types are defined in three layers with their structures shown below. `AzCLIError` is the base layer for all the newly defined error types. The second layer includes the main categories (`UserFault`, `ClientError`, `ServiceError`), which can be shown to users and also can be used for Telemetry analysis. __The third layer includes the specific error types, which are the ones command group authors can use when raising errors.__
 
 ```
-| -- AzCLIError                                 # [Base Layer]: DO NOT use it in command groups
-|    | -- UserFault                             # [Second Layer]: DO NOT use it in command groups
-|    |    | -- CommandNotFoundError             # [Third Layer]: Can be used in command groups
+| -- AzCLIError                             # [Base Layer]: DO NOT use it in command groups
+|    | -- UserFault                         # [Second Layer]: DO NOT use it in command groups
+|    |    | -- CommandNotFoundError         # [Third Layer]: Can be used in command groups
 |    |    | -- UnrecognizedArgumentError
 |    |    | -- RequiredArgumentMissingError
 |    |    | -- MutuallyExclusiveArgumentError
 |    |    | -- InvalidArgumentValueError
-|    |    | -- ArgumentParseError               # fallback of argument related errors
+|    |    | -- ArgumentUsageError               # fallback of argument related errors
 |    |    | -- BadRequestError
 |    |    | -- UnauthorizedError
 |    |    | -- ForbiddenError
@@ -35,8 +35,7 @@ The newly designed error types are provided in [azure/cli/core/azclierror.py](ht
 |    |    | -- ValidationError                  # fallback of validation related errors
 |    |    | -- FileOperationError
 |    |    | -- ManualInterrupt
-|    |    | -- ... More ...
-|    |    | -- UnknownError                     # fallback of UserFault related errors
+|    |    | -- UnclassifiedUserFault            # fallback of UserFault related errors
 |    | -- ClientError
 |    |    | -- CLIInternalError
 |    | -- ServiceError
@@ -45,8 +44,8 @@ The newly designed error types are provided in [azure/cli/core/azclierror.py](ht
 
 To summarize, here is a list of rules for command group authors to select a proper error type.
 - __DO NOT__ use the error types defined in the base layer and the second layer
+- Avoid using the fallback error types unless you can not find a specific one for your case
 - Consider defining a new error type if you can not find a proper one when it is a general error
-- Avoid using the fallback error types unless you can not find a proper one and the error is not general enough for a new error type
 
 ### Apply the Error Type
 
@@ -99,6 +98,7 @@ Below are the specific DOs and DON'Ts when writing the error messages. PRs viola
 
 __DOs__
 - Use the capital letter ahead of an error message.
+- Provide actionable message with argument suggestion. (e.g, Instead of using `resource group is missing, please provide a resource group name`, use `resource group is missing, please provide a resource group name by --resource-group`)
 
 __DON'Ts__
 - Do not control the style of an error message. (e.g, the unnecessary `'\n'` and the colorization.)

src/azure-cli-core/azure/cli/core/_profile.py

@@ -1349,7 +1349,7 @@ def _login_exception_handler(ex):
     from requests.exceptions import InvalidURL
     if isinstance(ex, InvalidURL):
         import traceback
-        from azure.cli.core.azclierror import UnknownError
+        from azure.cli.core.azclierror import UnclassifiedUserFault
         logger.debug('Invalid url when acquiring token\n%s', traceback.format_exc())
-        raise UnknownError(error_msg='Invalid url when acquiring token',
-                           recommendation='Please make sure the cloud is registered with valid url')
+        raise UnclassifiedUserFault(error_msg='Invalid url when acquiring token',
+                                    recommendation='Please make sure the cloud is registered with valid url')

src/azure-cli-core/azure/cli/core/azclierror.py

@@ -92,6 +92,14 @@ class ClientError(AzCLIError):
         telemetry.set_failure(self.error_msg)
         if self.exception_trace:
             telemetry.set_exception(self.exception_trace, '')
+
+
+class UnknownError(AzCLIError):
+    """ Unclear errors, could not know who should be responsible for the errors.
+    DO NOT raise this error class in your codes. """
+    def send_telemetry(self):
+        super().send_telemetry()
+        telemetry.set_failure(self.error_msg)
 # endregion
 
 
@@ -125,8 +133,8 @@ class InvalidArgumentValueError(UserFault):
     pass
 
 
-class ArgumentParseError(UserFault):
-    """ Fallback of the argument parsing related errors.
+class ArgumentUsageError(UserFault):
+    """ Fallback of the argument usage related errors.
     Avoid using this class unless the error can not be classified
     into the Argument related specific error types. """
     pass
@@ -199,7 +207,7 @@ class ValidationError(UserFault):
     pass
 
 
-class UnknownError(UserFault):
+class UnclassifiedUserFault(UserFault):
     """ Fallback of the UserFault related error types.
     Avoid using this class unless the error can not be classified into
     the UserFault related specific error types.

src/azure-cli-core/azure/cli/core/parser.py

@@ -20,7 +20,7 @@ from azure.cli.core.command_recommender import CommandRecommender
 from azure.cli.core.azclierror import UnrecognizedArgumentError
 from azure.cli.core.azclierror import RequiredArgumentMissingError
 from azure.cli.core.azclierror import InvalidArgumentValueError
-from azure.cli.core.azclierror import ArgumentParseError
+from azure.cli.core.azclierror import ArgumentUsageError
 from azure.cli.core.azclierror import CommandNotFoundError
 from azure.cli.core.azclierror import ValidationError
 
@@ -165,7 +165,7 @@ class AzCliCommandParser(CLICommandParser):
         recommender.set_help_examples(self.get_examples(self.prog))
         recommendation = recommender.recommend_a_command()
 
-        az_error = ArgumentParseError(message)
+        az_error = ArgumentUsageError(message)
         if 'unrecognized arguments' in message:
             az_error = UnrecognizedArgumentError(message)
         elif 'arguments are required' in message:

src/azure-cli-core/azure/cli/core/tests/test_profile.py

@@ -1050,7 +1050,7 @@ class TestProfile(unittest.TestCase):
     @mock.patch('azure.cli.core._profile._get_authorization_code', autospec=True)
     def test_find_subscriptions_with_invalid_authority_url(self, _get_authorization_code_mock, mock_auth_context):
         from requests.exceptions import InvalidURL
-        from azure.cli.core.azclierror import UnknownError
+        from azure.cli.core.azclierror import UnclassifiedUserFault
 
         def mock_acquire(*args, **kwargs):
             raise InvalidURL(request='http://some.unknown.endpoints')
@@ -1065,11 +1065,11 @@ class TestProfile(unittest.TestCase):
         }
 
         finder = SubscriptionFinder(cli, lambda _, _1, _2: mock_auth_context, None, lambda _: None)
-        with self.assertRaisesRegexp(UnknownError, 'Invalid url when acquiring token'):
+        with self.assertRaisesRegexp(UnclassifiedUserFault, 'Invalid url when acquiring token'):
             finder.find_from_user_account(self.user1, 'bar', None, 'http://goo-resource')
-        with self.assertRaisesRegexp(UnknownError, 'Invalid url when acquiring token'):
+        with self.assertRaisesRegexp(UnclassifiedUserFault, 'Invalid url when acquiring token'):
             finder.find_through_authorization_code_flow(None, 'https://management.core.windows.net/', 'https:/some_aad_point/common')
-        with self.assertRaisesRegexp(UnknownError, 'Invalid url when acquiring token'):
+        with self.assertRaisesRegexp(UnclassifiedUserFault, 'Invalid url when acquiring token'):
             finder.find_through_interactive_flow(None, 'https://management.core.windows.net/')
 
     @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True)

src/azure-cli-core/azure/cli/core/util.py

@@ -96,8 +96,8 @@ def handle_exception(ex):  # pylint: disable=too-many-locals, too-many-statement
             az_error = azclierror.ValidationError(error_msg)
 
         elif isinstance(ex, CLIError):
-            # TODO: Fine-grained analysis here for Unknown error
-            az_error = azclierror.UnknownError(error_msg)
+            # TODO: Fine-grained analysis here
+            az_error = azclierror.UnclassifiedUserFault(error_msg)
 
         elif isinstance(ex, AzureError):
             if extract_common_error_message(ex):

src/azure-cli/azure/cli/command_modules/network/custom.py

@@ -2046,7 +2046,7 @@ def import_zone(cmd, resource_group_name, zone_name, file_name):
     logger.warning("In the future, zone name will be case insensitive.")
     RecordSet = cmd.get_models('RecordSet', resource_type=ResourceType.MGMT_NETWORK_DNS)
 
-    from azure.cli.core.azclierror import FileOperationError, UnknownError
+    from azure.cli.core.azclierror import FileOperationError, UnclassifiedUserFault
     try:
         file_text = read_file_content(file_name)
     except FileNotFoundError:
@@ -2056,7 +2056,7 @@ def import_zone(cmd, resource_group_name, zone_name, file_name):
     except PermissionError:
         raise FileOperationError("Permission denied: " + str(file_name))
     except OSError as e:
-        raise UnknownError(e)
+        raise UnclassifiedUserFault(e)
 
     zone_obj = parse_zone_file(file_text, zone_name)
 

src/azure-cli/azure/cli/command_modules/privatedns/custom.py

@@ -33,7 +33,7 @@ def import_zone(cmd, resource_group_name, private_zone_name, file_name):
     import sys
     from azure.mgmt.privatedns.models import RecordSet
 
-    from azure.cli.core.azclierror import FileOperationError, UnknownError
+    from azure.cli.core.azclierror import FileOperationError, UnclassifiedUserFault
     try:
         file_text = read_file_content(file_name)
     except FileNotFoundError:
@@ -43,7 +43,7 @@ def import_zone(cmd, resource_group_name, private_zone_name, file_name):
     except PermissionError:
         raise FileOperationError("Permission denied: " + str(file_name))
     except OSError as e:
-        raise UnknownError(e)
+        raise UnclassifiedUserFault(e)
 
     zone_obj = parse_zone_file(file_text, private_zone_name)
     origin = private_zone_name