-
Notifications
You must be signed in to change notification settings - Fork 313
[Storage] Add StorageError
model type
#3011
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
base: main
Are you sure you want to change the base?
[Storage] Add StorageError
model type
#3011
Conversation
cc: @heaths Heath, any pointers here on how to proceed with accessing the raw response on an Error response. Thanks! |
StorageError
model type
impl std::fmt::Display for StorageError { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
writeln!(f, "Standard Error Message: {}\n", self.message)?; | ||
writeln!(f, "Http Status Code: {}", self.status_code)?; | ||
writeln!(f, "Storage Error Code: {}", self.error_code)?; | ||
writeln!(f, "Response Headers:")?; | ||
|
||
for (name, value) in self.headers.iter() { | ||
writeln!(f, " \"{}\": \"{}\"", name.as_str(), value.as_str())?; | ||
} | ||
|
||
Ok(()) | ||
} |
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.
Looking for some feedback here on how we want to implement the display. This is sort of what I came up with, and to re-iterate this is what currently comes out:
Standard Error Message: 404:
BlobNotFound
The specified blob does not exist.
RequestId:f65c5ab0-601e-008d-677c-2ec0a1000000
Time:2025-09-26T00:29:56.3906662ZHttp Status Code: 404
Storage Error Code: BlobNotFound
Response Headers:
"x-ms-client-request-id": "3b2dec8d-758b-45fd-adad-ba81c0d761b9"
"content-length": "215"
"server": "Microsoft-HTTPAPI/2.0"
"x-ms-version": "2025-11-05"
"content-type": "application/xml"
"date": "Fri, 26 Sep 2025 00:29:56 GMT"
"x-ms-request-id": "f65c5ab0-601e-008d-677c-2ec0a1000000"
"x-ms-error-code": "BlobNotFound"
"x-recording-mode": "record"
Ideally I want to eliminate writeln!(f, "Standard Error Message: {}\n", self.message)?;
and I currently just have it there to show the current functionality of azure_core::Error.to_string()
.
I think we want to be able to extract just the message and surface that since we have all the other displayed elsewhere.
i.e. The specified blob does not exist.
.
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.
- probably remove the quotations on headers as that's not generally how they're rendered.
- would this end up in logs? if so we probably want to hook in whatever we're using to allow/redact specific headers
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.
Headers should be sanitized. (Error bodies aren't supposed to contain PII.) @LarryOsterman added a sanitize
function you should use.
I also recommend proper grammar e.g., "HTTP" instead of "Http", etc. Order of fields should make sense. In .NET, it was:
{HTTP status code} {HTTP error code}
{Headers}
{Error response body}
Basically, formatted close to an actual HTTP response like many devs are used to.
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.
And before you go off and do this, we should discuss whether to change the Display
implementation for azure_core::Error
. Is this the only reason you're making StorageError
? We already support formatting for display.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
stdout for
test_storage_error_model
: