Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 60 additions & 2 deletions sdk/storage/azure_storage_blob/src/models/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

use crate::models::{
AppendBlobClientCreateOptions, BlobTag, BlobTags, BlockBlobClientUploadBlobFromUrlOptions,
BlockBlobClientUploadOptions, PageBlobClientCreateOptions,
BlockBlobClientUploadOptions, PageBlobClientCreateOptions, StorageError, StorageErrorCode,
};
use azure_core::error::ErrorKind;
use azure_core::{error::ErrorKind, http::headers::Headers};
use std::collections::HashMap;

/// Augments the current options bag to only create if the Page blob does not already exist.
Expand Down Expand Up @@ -109,3 +109,61 @@ impl From<HashMap<String, String>> for BlobTags {
}
}
}

impl TryFrom<azure_core::Error> for StorageError {
type Error = azure_core::Error;

fn try_from(error: azure_core::Error) -> Result<Self, Self::Error> {
let message = error.to_string();

match error.kind() {
ErrorKind::HttpResponse {
status,
error_code,
raw_response,
} => {
let error_code = error_code.as_ref().ok_or_else(|| {
azure_core::Error::with_message(
azure_core::error::ErrorKind::DataConversion,
"error_code field missing from HttpResponse.",
)
})?;

let headers = raw_response
.as_ref()
.map(|raw_resp| raw_resp.headers().clone())
.unwrap_or_default();

let error_code_enum = error_code
.parse()
.unwrap_or(StorageErrorCode::UnknownValue(error_code.clone()));

Ok(StorageError {
status_code: *status,
error_code: error_code_enum,
message,
headers,
})
}
_ => Err(azure_core::Error::with_message(
azure_core::error::ErrorKind::DataConversion,
"ErrorKind was not HttpResponse and could not be parsed.",
)),
}
}
}

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(())
}
Comment on lines +156 to +168
Copy link
Member Author

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: BlobNotFoundThe specified blob does not exist.
RequestId:f65c5ab0-601e-008d-677c-2ec0a1000000
Time:2025-09-26T00:29:56.3906662Z

Http 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..

Copy link
Member

Choose a reason for hiding this comment

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

  1. probably remove the quotations on headers as that's not generally how they're rendered.
  2. would this end up in logs? if so we probably want to hook in whatever we're using to allow/redact specific headers

Copy link
Member

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.

Copy link
Member

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.

}
36 changes: 36 additions & 0 deletions sdk/storage/azure_storage_blob/src/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,39 @@ pub use crate::generated::models::{
VecSignedIdentifierHeaders,
};
pub use extensions::*;

use azure_core::error::ErrorKind;
use azure_core::http::{headers::Headers, StatusCode};
use azure_core::Error;

/// A Storage-specific error that provides access to HTTP response details.
///
#[derive(Debug, Clone)]
pub struct StorageError {
/// The HTTP status code.
pub status_code: StatusCode,
/// The Storage error code.
pub error_code: StorageErrorCode,
/// The error message.
pub message: String,
/// The headers from the response.
pub headers: Headers,
}

impl StorageError {
pub fn status_code(&self) -> StatusCode {
self.status_code
}

pub fn error_code(&self) -> StorageErrorCode {
self.error_code.clone()
}

pub fn message(&self) -> &str {
&self.message
}

pub fn headers(&self) -> &Headers {
&self.headers
}
}
22 changes: 21 additions & 1 deletion sdk/storage/azure_storage_blob/tests/blob_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

use azure_core::{
error::ErrorKind,
http::{RequestContent, StatusCode},
Bytes,
};
Expand All @@ -12,7 +13,7 @@ use azure_storage_blob::models::{
BlobClientGetAccountInfoResultHeaders, BlobClientGetPropertiesOptions,
BlobClientGetPropertiesResultHeaders, BlobClientSetMetadataOptions,
BlobClientSetPropertiesOptions, BlobClientSetTierOptions, BlockBlobClientUploadOptions,
LeaseState,
LeaseState, StorageError,
};

use azure_storage_blob_test::{create_test_blob, get_blob_name, get_container_client};
Expand Down Expand Up @@ -480,3 +481,22 @@ async fn test_get_account_info(ctx: TestContext) -> Result<(), Box<dyn Error>> {

Ok(())
}

#[recorded::test]
async fn test_storage_error_model(ctx: TestContext) -> Result<(), Box<dyn Error>> {
// Recording Setup
let recording = ctx.recording();
let container_client = get_container_client(recording, true).await?;
let blob_client = container_client.blob_client(get_blob_name(recording));

// Act
let response = blob_client.download(None).await;
let error_response = response.unwrap_err();
let error_kind = error_response.kind();
assert!(matches!(error_kind, ErrorKind::HttpResponse { .. }));

let storage_error: StorageError = error_response.try_into()?;
println!("{}", storage_error);

Ok(())
}
Loading