-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
{AD} Show error code for Microsoft Graph errors #29941
base: dev
Are you sure you want to change the base?
Conversation
️✔️AzureCLI-FullTest
|
Hi @jiasli, |
️✔️AzureCLI-BreakingChangeTest
|
AD |
I am hesitating on
ARM clients' implementation is from 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 Even though Graph error also contains similar properties, such as # 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 |
err = ex.response.json()['error'] | ||
code = err['code'] | ||
message = err['message'] | ||
raise GraphError(f"({code}) {message}", ex.response) from ex |
There was a problem hiding this comment.
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', '')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will
code
andmessage
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 ''
.
There was a problem hiding this comment.
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
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:ARM clients show error code in parentheses and then the error message. They are also repeated in
Code:
andMessage:
lines:This PR aligns Graph client with ARM client but without
Code:
andMessage:
lines: