Skip to content

Commit c2199dc

Browse files
authored
Make Error constructions consistent (#3024)
We had some inconsistency in construction functions, and many didn't align to our guidelines. This essentially migrates a pattern I evolved for my own Rust crates' template based more or less on what we had.
1 parent e2c101f commit c2199dc

File tree

82 files changed

+429
-349
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+429
-349
lines changed

sdk/core/azure_core/CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,23 @@
44

55
### Features Added
66

7+
- Added `Error::with_error_fn()`.
8+
79
### Breaking Changes
810

911
- Changed `ClientOptions::retry` from `Option<RetryOptions>` to `RetryOptions`.
1012
- Removed several unreferenced HTTP headers and accessor structures for those headers.
1113
- Renamed `TransportOptions` to `Transport`.
1214
- Renamed `TransportOptions::new_custom_policy()` to `Transport::with_policy()`.
15+
- Renamed a number of construction functions for `Error` to align with [guidelines](https://azure.github.io/azure-sdk/rust_introduction.html)"
16+
- Renamed `Error::full()` to `Error::with_error()`.
17+
- Renamed `Error::with_message()` to `Error::with_message_fn()`.
18+
- Renamed `Error::message()` to `Error::with_message()`.
19+
- Renamed `Error::with_context()` to `Error::with_context_fn()`.
20+
- Renamed `Error::context()` to `Error::with_context()`.
21+
- Renamed `ResultExt::map_kind()` to `ResultExt::with_kind()`.
22+
- Renamed `ResultExt::with_context()` to `ResultExt::with_context_fn()`.
23+
- Renamed `ResultExt::context()` to `ResultExt::with_context()`.
1324

1425
### Bugs Fixed
1526

sdk/core/azure_core/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ impl HttpClient for Agent {
512512
let response = self
513513
.0
514514
.run(request)
515-
.with_context(ErrorKind::Io, || "failed to send request")?;
515+
.with_context_fn(ErrorKind::Io, || "failed to send request")?;
516516

517517
Ok(todo!("convert their response into our response"))
518518
}

sdk/core/azure_core/benches/http_transport_benchmarks.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl TestServiceClient {
5050
let options = options.unwrap_or_default();
5151
let mut endpoint = Url::parse(endpoint)?;
5252
if !endpoint.scheme().starts_with("http") {
53-
return Err(azure_core::Error::message(
53+
return Err(azure_core::Error::with_message(
5454
azure_core::error::ErrorKind::Other,
5555
format!("{endpoint} must use http(s)"),
5656
));
@@ -99,7 +99,7 @@ impl TestServiceClient {
9999
.send(&options.method_options.context, &mut request)
100100
.await?;
101101
if !response.status().is_success() {
102-
return Err(azure_core::Error::message(
102+
return Err(azure_core::Error::with_message(
103103
azure_core::error::ErrorKind::HttpResponse {
104104
status: response.status(),
105105
error_code: None,

sdk/core/azure_core/examples/core_ureq_client.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl HttpClient for Agent {
4040
let response = self
4141
.0
4242
.run(request)
43-
.with_context(ErrorKind::Io, || "failed to send request")?;
43+
.with_context_fn(ErrorKind::Io, || "failed to send request")?;
4444

4545
into_response(response)
4646
}
@@ -81,19 +81,19 @@ fn into_request(request: &Request) -> azure_core::Result<::http::Request<Vec<u8>
8181
.url()
8282
.as_str()
8383
.parse()
84-
.with_context(ErrorKind::DataConversion, || "failed to parse url")?;
84+
.with_context_fn(ErrorKind::DataConversion, || "failed to parse url")?;
8585
*req.method_mut() = request
8686
.method()
8787
.as_str()
8888
.parse()
89-
.with_context(ErrorKind::DataConversion, || "failed to parse method")?;
89+
.with_context_fn(ErrorKind::DataConversion, || "failed to parse method")?;
9090
let headers = req.headers_mut();
9191
for (name, value) in request.headers().iter() {
9292
headers.insert(
9393
HeaderName::from_bytes(name.as_str().as_bytes())
94-
.with_context(ErrorKind::DataConversion, || "failed to parse header name")?,
94+
.with_context_fn(ErrorKind::DataConversion, || "failed to parse header name")?,
9595
HeaderValue::from_bytes(value.as_str().as_bytes())
96-
.with_context(ErrorKind::DataConversion, || "failed to parse header value")?,
96+
.with_context_fn(ErrorKind::DataConversion, || "failed to parse header value")?,
9797
);
9898
}
9999
let body: Bytes = request.body().into();
@@ -120,13 +120,13 @@ fn into_response(response: ::http::Response<ureq::Body>) -> azure_core::Result<B
120120
name.as_str().to_ascii_lowercase(),
121121
value
122122
.to_str()
123-
.with_context(ErrorKind::DataConversion, || "failed to parse header value")?
123+
.with_context_fn(ErrorKind::DataConversion, || "failed to parse header value")?
124124
.to_string(),
125125
);
126126
}
127127
let body: Vec<u8> = body
128128
.read_to_vec()
129-
.with_context(ErrorKind::Io, || "failed to read response body")?;
129+
.with_context_fn(ErrorKind::Io, || "failed to read response body")?;
130130

131131
Ok(BufResponse::from_bytes(status, response_headers, body))
132132
}

sdk/core/azure_core/src/error/error_response.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ pub async fn check_success(response: BufResponse) -> crate::Result<BufResponse>
130130
if response.body().is_empty() {
131131
let code = response.headers().get_optional_str(&ERROR_CODE);
132132
let error_kind = ErrorKind::http_response(status, code.map(str::to_owned));
133-
return Err(Error::message(error_kind, status.to_string()));
133+
return Err(Error::with_message(error_kind, status.to_string()));
134134
}
135135
let internal_response =
136136
serde_json::de::from_slice::<ErrorResponseInternal>(response.body()).map_err(Error::from);
@@ -144,7 +144,7 @@ pub async fn check_success(response: BufResponse) -> crate::Result<BufResponse>
144144
status,
145145
Some(code.map_or_else(|| response.status().to_string(), str::to_owned)),
146146
);
147-
return Err(Error::message(
147+
return Err(Error::with_message(
148148
error_kind,
149149
format!(
150150
"{}: {}",
@@ -161,7 +161,7 @@ pub async fn check_success(response: BufResponse) -> crate::Result<BufResponse>
161161

162162
let error_kind = ErrorKind::http_response(status, code.map(str::to_owned));
163163

164-
Err(Error::message(
164+
Err(Error::with_message(
165165
error_kind,
166166
internal_response
167167
.error

sdk/core/azure_core/src/hmac.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub fn hmac_sha256(data: &str, key: &Secret) -> crate::Result<String> {
2727
use sha2::Sha256;
2828
let key = base64::decode(key.secret())?;
2929
let mut hmac = Hmac::<Sha256>::new_from_slice(&key)
30-
.with_context(ErrorKind::DataConversion, || {
30+
.with_context_fn(ErrorKind::DataConversion, || {
3131
"failed to create hmac from key"
3232
})?;
3333
hmac.update(data.as_bytes());
@@ -58,7 +58,7 @@ pub fn hmac_sha256(data: &str, key: &Secret) -> crate::Result<String> {
5858
signer.update(data.as_bytes())?;
5959
signer.sign_to_vec()
6060
}()
61-
.with_context(ErrorKind::DataConversion, || {
61+
.with_context_fn(ErrorKind::DataConversion, || {
6262
"failed to create hmac from key"
6363
})?;
6464
Ok(base64::encode(signature))

sdk/core/azure_core/src/http/pager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ mod tests {
749749
.into(),
750750
continuation: "1",
751751
}),
752-
PagerState::More("1") => Err(typespec::Error::message(
752+
PagerState::More("1") => Err(typespec::Error::with_message(
753753
typespec::error::ErrorKind::Other,
754754
"yon request didst fail",
755755
)),

sdk/core/azure_core/src/http/policies/bearer_token_policy.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ impl Policy for BearerTokenCredentialPolicy {
120120
}
121121

122122
let access_token = self.access_token().await.ok_or_else(|| {
123-
Error::message(
123+
Error::with_message(
124124
ErrorKind::Credential,
125125
"The request failed due to an error while fetching the access token.",
126126
)
@@ -200,7 +200,7 @@ mod tests {
200200
let i = self.calls.fetch_add(1, Ordering::SeqCst);
201201
self.tokens
202202
.get(i)
203-
.ok_or_else(|| Error::message(ErrorKind::Credential, "no more mock tokens"))
203+
.ok_or_else(|| Error::with_message(ErrorKind::Credential, "no more mock tokens"))
204204
.cloned()
205205
}
206206
}

sdk/core/azure_core/src/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl FromStr for TestMode {
6363
"playback" => Ok(Self::Playback),
6464
"record" => Ok(Self::Record),
6565
"live" => Ok(Self::Live),
66-
_ => Err(Error::message(
66+
_ => Err(Error::with_message(
6767
ErrorKind::DataConversion,
6868
"expected 'playback', 'record', or 'live'",
6969
)),
@@ -106,7 +106,7 @@ impl FromStr for RecordingMode {
106106
match s.to_ascii_lowercase().as_str() {
107107
"playback" => Ok(Self::Playback),
108108
"record" => Ok(Self::Record),
109-
_ => Err(Error::message(
109+
_ => Err(Error::with_message(
110110
ErrorKind::DataConversion,
111111
"expected 'playback' or 'record'",
112112
)),

sdk/core/azure_core_amqp/src/fe2o3/cbs.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ impl Fe2o3ClaimsBasedSecurity {
2525
}
2626

2727
fn cbs_already_attached() -> azure_core::Error {
28-
azure_core::Error::message(
28+
azure_core::Error::with_message(
2929
azure_core::error::ErrorKind::Amqp,
3030
"Claims Based Security is already attached",
3131
)
3232
}
3333
fn cbs_not_set() -> azure_core::Error {
34-
azure_core::Error::message(
34+
azure_core::Error::with_message(
3535
azure_core::error::ErrorKind::Amqp,
3636
"Claims Based Security is not set",
3737
)
3838
}
3939
fn cbs_not_attached() -> azure_core::Error {
40-
azure_core::Error::message(
40+
azure_core::Error::with_message(
4141
azure_core::error::ErrorKind::Amqp,
4242
"Claims Based Security is not attached",
4343
)
@@ -97,7 +97,7 @@ impl AmqpClaimsBasedSecurityApis for Fe2o3ClaimsBasedSecurity {
9797
.unix_timestamp()
9898
.checked_mul(1_000)
9999
.ok_or_else(|| {
100-
azure_core::Error::message(
100+
azure_core::Error::with_message(
101101
azure_core::error::ErrorKind::Amqp,
102102
"Unable to convert time to unix timestamp.",
103103
)
@@ -116,7 +116,7 @@ impl AmqpClaimsBasedSecurityApis for Fe2o3ClaimsBasedSecurity {
116116
Ok(amqp_error) => amqp_error.into(),
117117
Err(e) => {
118118
debug!("Failed to convert management error to azure error: {:?}", e);
119-
azure_core::Error::full(
119+
azure_core::Error::with_error(
120120
azure_core::error::ErrorKind::Amqp,
121121
e,
122122
"Failed to convert management error to azure error.",

0 commit comments

Comments
 (0)