Skip to content

Conversation

@houk-ms
Copy link
Contributor

@houk-ms houk-ms commented Aug 20, 2020

Description

This PR works to improve the error messages of CLI, which includes the output messages shown to users, messages for Telemetry records, and new error categories. These changes are interdependent to some extent, so it's not very easy to split it into smaller PRs. Beg your patience and understanding for it may be a little large to review. The main changes are:

1. Introduce AzCLIError to replace CLIError

Backgrounds

Currently, CLIError is widely used in both CLI core and command groups. Working as a wrapper of the service-defined error types, CLIError accepts only an error message, which brings a side-effect that we can not distinguish the different error types and categorize them.

For example, CLIError can be caused by resource not found which is an userfault, and it can also be caused by service failure which is a service error. So when we want to differentiate UserFault from Service-Side-Error to do some analysis using telemetry, CLIError will block us. Explore here to see more CLIError messages.

This PR Bings

This PR introduces the new AzCLIError, while it will not change the original workflow of CLIError. You can use the AzCLIError to replace your CLIError anywhere in your codes. And by using AzCLIError, you need to specify

  1. [Required] An error type, defined below in section 2
  2. [Required] An error message
  3. [Optional] A recommendation to fix the error

Another benefit AzCLIError brings is that we can provide the same error message for both CLI users and Telemetry records. Previously, there are many cases that we print error messages for users, while the error message details are not recorded in Telemetry, and even the entire record is missing. Here is an example.

2. Define new error types for better error categorizing

As mentioned in section 1, these new error types can help us differentiate UserFault, Client-Side-Error and Service-Side-Error. The new defined error types are:

# userfaults
  CommandNotFoundError
  ArgumentParseError
  ValidationError
  ManualInterrupt

# service side error
  ServiceError

# client side error
  ClientError

# unexpected error
  UnexpectedError

3. Improve Telemetry

  • One record for one command.

This PR sets the exception info into the UserTask type record, which means the UserTask record will have all the needed info for a command execution, no matter the comand fails or not. The PR now doesn't change the original workflow of Fault type record, which will be removed from Telemetry records after we confirm the UserTask record work properly.

  • Fix the record missing issue.

When a command fails with a 404 error, there is no Telemetry record for the command. This PR fixes the issue.

4. New error messages

  • New error messages format

As we discussed in previous meeting, the new error messages are applied in this PR. These message changes are mainly for userfaults, like CommandNotFountError and ArgumentParseError. The pattern is

ErrorType: ErrorMSG
Try this: ...recommend a command...
Still stuck? ...reference...
  • Recommend a command when user's command fails

This PR provides the logic to recommend a command for users combining both examples from help files and aladdin recommendations. It makes sure the recommended command has the same command name with user's input, otherwise, they will not be recommended. If there are multiple recommendations, we compare them and find the most similar one with the user's original inputs.

Testing Guide

Type a command which can raise an CommandNotFoundError or ArgumentParseError, you will see the new error message formats. A few examples are shown here.

  • az grup show
CommandNotFoundError: 'grup' is misspelled or not recognized by the system.
Did you mean 'group' ?
Try this: 'az group show --resource-group myresourcegroup'
Still stuck? Run 'az --help' to view all commands or go to 'https://docs.microsoft.com/en-us/cli/azure/reference-index?view=azure-cli-latest' to learn more
  • az vm create -g rg
ArgumentParseError: the following arguments are required: --name/-n
Try this: 'az vm create -g MyResourceGroup -n MyVm --image MyImage'
Still stuck? Run 'az vm create --help' to view all commands or go to 'https://docs.microsoft.com/en-us/cli/azure/reference-index?view=azure-cli-latest' to learn more
  • az group get -g rg
CommandNotFoundError: 'get' is misspelled or not recognized by the system.
Try this: 'az group list'
Still stuck? Run 'az group --help' to view all commands or go to 'https://docs.microsoft.com/en-us/cli/azure/reference-index?view=azure-cli-latest' to learn more
  • az grup get -g rg
CommandNotFoundError: 'grup' is misspelled or not recognized by the system.
Did you mean 'group' ?
Still stuck? Run 'az --help' to view all commands or go to 'https://docs.microsoft.com/en-us/cli/azure/reference-index?view=azure-cli-latest' to learn more

@yonzhan
Copy link
Collaborator

yonzhan commented Aug 20, 2020

Core

@yonzhan yonzhan requested a review from zhoxing-ms August 20, 2020 07:14
@houk-ms houk-ms requested a review from bgklein as a code owner August 20, 2020 09:29
@mmyyrroonn
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@houk-ms
Copy link
Contributor Author

houk-ms commented Sep 11, 2020

May I ask whether the customer's adoption of the commands recommended by ai-did-you-mean-this will be collected?
I personally think that if there is a feedback mechanism to analyze user adoption, it can help us better optimize the recommendation effect.

Not yet now. Maybe we can discuss with Aladdin team to see if that can help them improve the service and consider then add it in next release if necessary.

Copy link
Contributor

@haroldrandom haroldrandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Network LGTM

Copy link
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general. Recommend to use replacer for unexpected request

@houk-ms
Copy link
Contributor Author

houk-ms commented Sep 15, 2020

All the changes made on test files are removed. All the changes made on Azure Pipeline are removed.

Now the method to deal with test issues is:

Disabling aladdin requests in all the testing environments, which is decied by CLI core whether current instance is DummyCLI. With this method, aladdin api will not be called when running the test cases, so the requests will never be recorded in recording files.

@houk-ms houk-ms changed the title {Core} Error message improvement [Core] Error message improvement Sep 15, 2020
@houk-ms houk-ms merged commit 4055456 into Azure:dev Sep 15, 2020
@houk-ms
Copy link
Contributor Author

houk-ms commented Oct 26, 2020

#15314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.