Skip to content

[Core] Provide recommendation for JSON parsing error#17445

Merged
jiasli merged 1 commit intoAzure:devfrom
jiasli:json-error
Apr 1, 2021
Merged

[Core] Provide recommendation for JSON parsing error#17445
jiasli merged 1 commit intoAzure:devfrom
jiasli:json-error

Conversation

@jiasli
Copy link
Copy Markdown
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

jiasli
jiasli commented Mar 25, 2021
View reviewed changes
src/azure-cli-core/setup.py
# dependencies for specific OSes
if not sys.platform.startswith('cygwin'):
DEPENDENCIES.append('psutil~=5.7')
DEPENDENCIES.append('psutil~=5.8')
Copy link
Copy Markdown
Member Author

@jiasli jiasli Mar 25, 2021

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.

jiasli
jiasli commented Mar 25, 2021
View reviewed changes
src/azure-cli-core/azure/cli/core/util.py
except SyntaxError:
raise CLIError(json_ex)
except ValueError as ex:
except Exception as ex:
Copy link
Copy Markdown
Member Author

@jiasli jiasli Mar 25, 2021

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
Copy Markdown
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 jiasli mentioned this pull request Mar 26, 2021
Closed
@jiasli
Copy link
Copy Markdown
Member Author

jiasli commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 26, 2021

Azure Pipelines successfully started running 2 pipeline(s).

@jiasli jiasli requested a review from yonzhan March 26, 2021 06:57
yonzhan
yonzhan approved these changes Mar 26, 2021
View reviewed changes
Copy link
Copy Markdown
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
@jiasli jiasli mentioned this pull request Aug 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evelyn-ys evelyn-ys evelyn-ys approved these changes

@yonzhan yonzhan yonzhan approved these changes

@fengzhou-msft fengzhou-msft Awaiting requested review from fengzhou-msft

@houk-ms houk-ms Awaiting requested review from houk-ms

@jsntcy jsntcy Awaiting requested review from jsntcy

@Juliehzl Juliehzl Awaiting requested review from Juliehzl

+1 more reviewer

@qwordy qwordy qwordy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jiasli jiasli

Labels

None yet

Projects

None yet

Milestone

S185

Development

Successfully merging this pull request may close these issues.

4 participants

@jiasli @yonzhan @qwordy @evelyn-ys