Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Mar 25, 2021

Description

We are seeing countless users complaining they have stumbled on Quoting issues with PowerShell.

All issues received: https://github.com/Azure/azure-cli/issues?q=is%3Aissue+label%3A%22Shell+-+PowerShell%22

Changes

This PR refines the error message of JSON parsing error:

  • Echo the JSON received by CLI
  • Provide recommendation for all shells
  • Provide recommendation especially for PowerShell
  • Use the new InvalidArgumentValueError to replace CLIError

Testing Guide

Before:

# In powershell.exe or pwsh.exe
> az monitor diagnostic-settings create --logs '{"category":"QueryRuntimeStatistics"}'
Expecting property name enclosed in double quotes: line 1 column 2 (char 1)

After:

# In powershell.exe or pwsh.exe
> az monitor diagnostic-settings create --logs '{"category":"QueryRuntimeStatistics"}'
Failed to parse JSON: {category:QueryRuntimeStatistics}
Error detail: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)
The JSON may have been parsed by the shell. See https://docs.microsoft.com/cli/azure/use-cli-effectively#quoting-issues
PowerShell requires additional quoting rules. See https://github.com/Azure/azure-cli/blob/dev/doc/quoting-issues-with-powershell.md

# dependencies for specific OSes
if not sys.platform.startswith('cygwin'):
DEPENDENCIES.append('psutil~=5.7')
DEPENDENCIES.append('psutil~=5.8')
Copy link
Member Author

Choose a reason for hiding this comment

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

Use psutil 5.8 in order to get Python 3.9 support.

except SyntaxError:
raise CLIError(json_ex)
except ValueError as ex:
except Exception as ex:
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to distinguish between SyntaxError and ValueError.

@yonzhan yonzhan added this to the S185 milestone Mar 25, 2021
@yonzhan
Copy link
Collaborator

yonzhan commented Mar 25, 2021

Core

@jiasli jiasli changed the title [Core] Provide error recommendation for JSON parsing error [Core] Provide recommendation for JSON parsing error Mar 25, 2021
@jiasli
Copy link
Member Author

jiasli commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jiasli jiasli requested a review from yonzhan March 26, 2021 06:57
Copy link
Collaborator

@yonzhan yonzhan left a comment

Choose a reason for hiding this comment

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

LGTM

@jiasli jiasli requested a review from qwordy April 1, 2021 06:41
@jiasli jiasli merged commit ad30d55 into Azure:dev Apr 1, 2021
@jiasli jiasli deleted the json-error branch April 1, 2021 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants