Skip to content

{AD} Show error code for Microsoft Graph errors#29941

Draft
jiasli wants to merge 1 commit intoAzure:devfrom
jiasli:graph-error
Draft

{AD} Show error code for Microsoft Graph errors#29941
jiasli wants to merge 1 commit intoAzure:devfrom
jiasli:graph-error

Conversation

@jiasli
Copy link
Copy Markdown
Member

@jiasli jiasli commented Sep 20, 2024

Related command
az ad
az role

Description
The current Graph client only shows error.message property. Sometimes, error.code also contains valuable information. For example, in #28734, the error code is more clear than the error message:

{"error":{"code":"CannotUpdateLockedServicePrincipalProperty","message":"Property passwordCredentials is invalid."...

ARM clients show error code in parentheses and then the error message. They are also repeated in Code: and Message: lines:

> az group show --name non-exist
(ResourceGroupNotFound) Resource group 'non-exist' could not be found.
Code: ResourceGroupNotFound
Message: Resource group 'non-exist' could not be found.

> az storage account create -g cli-test-rg -n 11
(AccountNameInvalid) 11 is not a valid storage account name. Storage account name must be between 3 and 24 characters in length and use numbers and lower-case letters only.
Code: AccountNameInvalid
Message: 11 is not a valid storage account name. Storage account name must be between 3 and 24 characters in length and use numbers and lower-case letters only.

This PR aligns Graph client with ARM client but without Code: and Message: lines:

> az ad sp credential reset --id 92375e31-0eae-4019-8207-0698ce16d144
(CannotUpdateLockedServicePrincipalProperty) Property passwordCredentials is invalid.

> az ad sp credential reset --id 92375e31-0eae-4019-8207-0698ce16d145
(Request_ResourceNotFound) Resource '92375e31-0eae-4019-8207-0698ce16d145' does not exist or one of its queried reference-property objects are not present.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Sep 20, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9

@azure-client-tools-bot-prd
Copy link
Copy Markdown

Hi @jiasli,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Sep 20, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Sep 20, 2024

AD

@microsoft-github-policy-service microsoft-github-policy-service bot added the Auto-Assign Auto assign by bot label Sep 20, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group label Sep 20, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad label Sep 20, 2024
@jiasli
Copy link
Copy Markdown
Member Author

jiasli commented Sep 20, 2024

I am hesitating on Code: and Message: lines as they are simply repetitions of the first line and gives no more information, such as

> az group show --name non-exist
(ResourceGroupNotFound) Resource group 'non-exist' could not be found.
Code: ResourceGroupNotFound
Message: Resource group 'non-exist' could not be found.

ARM clients' implementation is from azure.core.exceptions.ODataV4Format:

https://github.com/Azure/azure-sdk-for-python/blob/9da614d54c97660b7f14854a4bce3704242507eb/sdk/core/azure-core/azure/core/exceptions.py#L248-L270

    def __str__(self) -> str:
        return "({}) {}\n{}".format(self.code, self.message, self.message_details())


    def message_details(self) -> str:
        """Return a detailed string of the error.

        :return: A string with the details of the error.
        :rtype: str
        """
        error_str = "Code: {}".format(self.code)
        error_str += "\nMessage: {}".format(self.message)
        if self.target:
            error_str += "\nTarget: {}".format(self.target)


        if self.details:
            error_str += "\nException Details:"
            for error_obj in self.details:
                # Indent for visibility
                error_str += "\n".join("\t" + s for s in str(error_obj).splitlines())


        if self.innererror:
            error_str += "\nInner error: {}".format(json.dumps(self.innererror, indent=4))
        return error_str

I guess the reason is because besides code and message, Azure Core also handles target, details and innererror.

Even though Graph error also contains similar properties, such as error.details.target and error.innerError:

# az ad sp credential reset --id 92375e31-0eae-4019-8207-0698ce16d144 --verbose
{
    "error": {
        "code": "CannotUpdateLockedServicePrincipalProperty",
        "message": "Property passwordCredentials is invalid.",
        "details": [
            {
                "code": "GenericError",
                "message": "Property passwordCredentials is invalid.",
                "target": "passwordCredentials",
                "blockedWord": "",
                "prefix": "",
                "suffix": ""
            }
        ],
        "innerError": {
            "date": "2024-09-20T08:01:41",
            "request-id": "fbbd7cdb-8181-4bcf-8bca-7ed896035ee2",
            "client-request-id": "fbbd7cdb-8181-4bcf-8bca-7ed896035ee2"
        }
    }
}

It may not be guaranteed that the error body of every API is the same, so I tend to only show the top level error.code and error.message, and not complicate things too much. The detailed information is in --verbose and --debug log after all.

err = ex.response.json()['error']
code = err['code']
message = err['message']
raise GraphError(f"({code}) {message}", ex.response) from ex
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will code and message always exist? Should we use err.get('Code', '')?

Copy link
Copy Markdown
Member Author

@jiasli jiasli Sep 23, 2024

Choose a reason for hiding this comment

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

Will code and message always exist?

A good catch! I can't guarantee this. Using .get() is definitely more robust.

But, we can't use err.get('code', ''). If code does exist but is null, it will be rendered as 'None'. https://docs.python.org/3.12/library/stdtypes.html#dict.get

It's better to use err.get('code') or ''.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wait. code and message are guaranteed to exist: https://learn.microsoft.com/en-us/graph/errors#error-resource-type

image

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

Labels

act-codegen-extensibility-squad act-identity-squad ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group Auto-Assign Auto assign by bot Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad Storage az storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants