Skip to content

switch Method and StatusCode to http-types #860

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

Merged
merged 12 commits into from
Jun 28, 2022
Merged
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion sdk/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ bytes = "1.0"
chrono = "0.4"
dyn-clone = "1.0"
futures = "0.3"
http = "0.2"
http-types = "2.12"
log = "0.4"
rand = "0.8"
reqwest = { version = "0.11", features = [
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/src/error/http_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl HttpError {
let body = response.into_body().await;
error_code = error_code.or_else(|| get_error_code_from_body(&body));
HttpError {
status: status.as_u16(),
status: status as u16,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are re-exporting StatusCode now, can we make the error include the enum value instead of a u16? This will make dealing with contextualized errors significantly more straightforward.

Copy link
Member

Choose a reason for hiding this comment

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

I think that is a good idea in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added #870

headers,
error_code,
body,
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/src/http_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub trait HttpClient: Send + Sync + std::fmt::Debug {
let rsp = self.execute_request(request).await?;
let (status, headers, body) = rsp.deconstruct();
let body = crate::collect_pinned_stream(body).await?;
let status_u16 = status.as_u16();
let status_u16 = status as u16;
if !(200..400).contains(&status_u16) {
return Err(ErrorKind::http_response_from_body(status_u16, &body).into_error());
}
Expand Down
34 changes: 31 additions & 3 deletions sdk/core/src/http_client/reqwest.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::*;
use std::collections::HashMap;
use std::{collections::HashMap, str::FromStr};

/// Construct a new `HttpClient` with the `reqwest` backend.
pub fn new_http_client() -> std::sync::Arc<dyn HttpClient> {
Expand All @@ -10,7 +10,7 @@ pub fn new_http_client() -> std::sync::Arc<dyn HttpClient> {
impl HttpClient for ::reqwest::Client {
async fn execute_request(&self, request: &crate::Request) -> crate::Result<crate::Response> {
let url = request.url().clone();
let mut req = self.request(request.method().clone(), url);
let mut req = self.request(try_from_method(request.method())?, url);
for (name, value) in request.headers().iter() {
req = req.header(name.as_str(), value.as_str());
}
Expand Down Expand Up @@ -42,7 +42,11 @@ impl HttpClient for ::reqwest::Client {
)
}));

Ok(crate::Response::new(status, headers, body))
Ok(crate::Response::new(
try_from_status(status)?,
headers,
body,
))
}
}

Expand All @@ -65,3 +69,27 @@ fn to_headers(map: &::reqwest::header::HeaderMap) -> crate::Result<crate::header
.collect::<HashMap<_, _>>();
Ok(crate::headers::Headers::from(map))
}

fn try_from_method(method: &crate::Method) -> crate::Result<::reqwest::Method> {
match method {
crate::Method::Connect => Ok(::reqwest::Method::CONNECT),
crate::Method::Delete => Ok(::reqwest::Method::DELETE),
crate::Method::Get => Ok(::reqwest::Method::GET),
crate::Method::Head => Ok(::reqwest::Method::HEAD),
crate::Method::Options => Ok(::reqwest::Method::OPTIONS),
crate::Method::Patch => Ok(::reqwest::Method::PATCH),
crate::Method::Post => Ok(::reqwest::Method::POST),
crate::Method::Put => Ok(::reqwest::Method::PUT),
crate::Method::Trace => Ok(::reqwest::Method::TRACE),
_ => ::reqwest::Method::from_str(method.as_ref()).map_kind(ErrorKind::DataConversion),
}
}

fn try_from_status(status: ::reqwest::StatusCode) -> crate::Result<crate::StatusCode> {
let status = u16::from(status);
crate::StatusCode::try_from(status).map_err(|_| {
Error::with_message(ErrorKind::DataConversion, || {
format!("invalid status code {status}")
})
})
}
4 changes: 2 additions & 2 deletions sdk/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ pub use seekable_stream::*;
pub use sleep::sleep;

// re-export important types at crate level
pub use http::Method;
pub use http::StatusCode;
pub use http_types::Method;
pub use http_types::StatusCode;
pub use url::Url;

/// A unique identifier for a request.
Expand Down
5 changes: 3 additions & 2 deletions sdk/core/src/mock/mock_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ impl<'de> Deserialize<'de> for MockResponse {
headers.insert(name, value);
}
let body = Bytes::from(base64::decode(r.body).map_err(Error::custom)?);
let status = StatusCode::from_u16(r.status).map_err(Error::custom)?;
let status = StatusCode::try_from(r.status)
.map_err(|_| Error::custom(format!("invalid status code {}", r.status)))?;

Ok(Self::new(status, headers, body))
}
Expand All @@ -85,7 +86,7 @@ impl Serialize for MockResponse {
for (h, v) in self.headers.iter() {
headers.insert(h.as_str().into(), v.as_str().into());
}
let status = self.status.as_u16();
let status = self.status as u16;
let body = base64::encode(&self.body as &[u8]);
let s = SerializedMockResponse {
status,
Expand Down
22 changes: 13 additions & 9 deletions sdk/core/src/policies/retry_policies/retry_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ pub trait RetryPolicy {
///
/// On all other 4xx and 5xx status codes no retry is attempted.
const RETRY_STATUSES: &[StatusCode] = &[
StatusCode::REQUEST_TIMEOUT,
StatusCode::TOO_MANY_REQUESTS,
StatusCode::INTERNAL_SERVER_ERROR,
StatusCode::BAD_GATEWAY,
StatusCode::SERVICE_UNAVAILABLE,
StatusCode::GATEWAY_TIMEOUT,
StatusCode::RequestTimeout,
StatusCode::TooManyRequests,
StatusCode::InternalServerError,
StatusCode::BadGateway,
StatusCode::ServiceUnavailable,
StatusCode::GatewayTimeout,
];

#[async_trait::async_trait]
Expand All @@ -48,7 +48,7 @@ where

loop {
let error = match next[0].send(ctx, request, &next[1..]).await {
Ok(response) if (200..400).contains(&response.status().as_u16()) => {
Ok(response) if (200..400).contains(&u16::from(response.status())) => {
log::trace!(
"Succesful response. Request={:?} response={:?}",
request,
Expand All @@ -59,12 +59,16 @@ where
}
Ok(response) => {
// Error status code
let code = response.status().as_u16();
let code = response.status() as u16;

let http_error = HttpError::new(response).await;
// status code should already be parsed as valid from the underlying HTTP
// implementations.
let status = StatusCode::from_u16(code).expect("invalid status code");
let status = StatusCode::try_from(code).map_err(|_| {
Error::with_message(ErrorKind::DataConversion, || {
format!("invalid status code '{code}'")
})
})?;
let error = Error::full(
ErrorKind::http_response(
code,
Expand Down
32 changes: 16 additions & 16 deletions sdk/data_cosmos/src/authorization_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,15 @@ fn string_to_sign(
format!(
"{}\n{}\n{}\n{}\n\n",
match *http_method {
azure_core::Method::GET => "get",
azure_core::Method::PUT => "put",
azure_core::Method::POST => "post",
azure_core::Method::DELETE => "delete",
azure_core::Method::HEAD => "head",
azure_core::Method::TRACE => "trace",
azure_core::Method::OPTIONS => "options",
azure_core::Method::CONNECT => "connect",
azure_core::Method::PATCH => "patch",
azure_core::Method::Get => "get",
azure_core::Method::Put => "put",
azure_core::Method::Post => "post",
azure_core::Method::Delete => "delete",
azure_core::Method::Head => "head",
azure_core::Method::Trace => "trace",
azure_core::Method::Options => "options",
azure_core::Method::Connect => "connect",
azure_core::Method::Patch => "patch",
_ => "extension",
},
match rt {
Expand Down Expand Up @@ -242,7 +242,7 @@ mod tests {
let time = time.with_timezone(&chrono::Utc).into();

let ret = string_to_sign(
&azure_core::Method::GET,
&azure_core::Method::Get,
&ResourceType::Databases,
"dbs/MyDatabase/colls/MyCollection",
time,
Expand Down Expand Up @@ -271,7 +271,7 @@ mon, 01 jan 1900 01:00:00 gmt

let ret = generate_authorization(
&auth_token,
&azure_core::Method::GET,
&azure_core::Method::Get,
&ResourceType::Databases,
"dbs/MyDatabase/colls/MyCollection",
time,
Expand All @@ -295,7 +295,7 @@ mon, 01 jan 1900 01:00:00 gmt

let ret = generate_authorization(
&auth_token,
&azure_core::Method::GET,
&azure_core::Method::Get,
&ResourceType::Databases,
"dbs/ToDoList",
time,
Expand All @@ -316,7 +316,7 @@ mon, 01 jan 1900 01:00:00 gmt
fn generate_resource_link_00() {
let request = Request::new(
reqwest::Url::parse("https://.documents.azure.com/dbs/second").unwrap(),
azure_core::Method::GET,
azure_core::Method::Get,
);
assert_eq!(&generate_resource_link(&request), "dbs/second");
}
Expand All @@ -325,7 +325,7 @@ mon, 01 jan 1900 01:00:00 gmt
fn generate_resource_link_01() {
let request = Request::new(
reqwest::Url::parse("https://.documents.azure.com/dbs").unwrap(),
azure_core::Method::GET,
azure_core::Method::Get,
);
assert_eq!(&generate_resource_link(&request), "");
}
Expand All @@ -334,7 +334,7 @@ mon, 01 jan 1900 01:00:00 gmt
fn generate_resource_link_02() {
let request = Request::new(
reqwest::Url::parse("https://.documents.azure.com/colls/second/third").unwrap(),
azure_core::Method::GET,
azure_core::Method::Get,
);
assert_eq!(&generate_resource_link(&request), "colls/second/third");
}
Expand All @@ -343,7 +343,7 @@ mon, 01 jan 1900 01:00:00 gmt
fn generate_resource_link_03() {
let request = Request::new(
reqwest::Url::parse("https://.documents.azure.com/dbs/test_db/colls").unwrap(),
azure_core::Method::GET,
azure_core::Method::Get,
);
assert_eq!(&generate_resource_link(&request), "dbs/test_db");
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/clients/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl CosmosOptions {
/// Create new options with a given transaction name
pub fn new_with_transaction_name(name: String) -> Self {
Self {
options: ClientOptions::new_with_transaction_name(name.into()),
options: ClientOptions::new_with_transaction_name(name),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/create_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl CreateCollectionBuilder {

pub fn into_future(self) -> CreateCollection {
Box::pin(async move {
let mut request = self.client.collections_request(azure_core::Method::POST);
let mut request = self.client.collections_request(azure_core::Method::Post);
request.insert_headers(&self.offer);
if let Some(cl) = &self.consistency_level {
request.insert_headers(cl);
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/create_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl CreateDatabaseBuilder {

pub fn into_future(self) -> CreateDatabase {
Box::pin(async move {
let mut request = self.client.request("dbs", azure_core::Method::POST);
let mut request = self.client.request("dbs", azure_core::Method::Post);

let body = CreateDatabaseBody {
id: self.database_name.as_str(),
Expand Down
4 changes: 2 additions & 2 deletions sdk/data_cosmos/src/operations/create_document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<D: Serialize + CosmosEntity + Send + 'static> CreateDocumentBuilder<D> {
Some(s) => s,
None => serialize_partition_key(&document.partition_key())?,
};
let mut request = self.client.docs_request(azure_core::Method::POST);
let mut request = self.client.docs_request(azure_core::Method::Post);

add_as_partition_key_header_serialized(&partition_key, &mut request);
request.insert_headers(&self.if_match_condition);
Expand Down Expand Up @@ -141,7 +141,7 @@ impl CreateDocumentResponse {
let body = collect_pinned_stream(pinned_stream).await?;

Ok(CreateDocumentResponse {
is_update: status_code == StatusCode::OK,
is_update: status_code == StatusCode::Ok,

last_state_change: last_state_change_from_headers(&headers)?,
etag: etag_from_headers(&headers)?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ impl CreateOrReplaceAttachmentBuilder {
pub fn into_future(self) -> CreateOrReplaceAttachment {
Box::pin(async move {
let mut req = if self.is_create {
self.client.attachments_request(azure_core::Method::POST)
self.client.attachments_request(azure_core::Method::Post)
} else {
self.client.attachment_request(azure_core::Method::PUT)
self.client.attachment_request(azure_core::Method::Put)
};

if let Some(cl) = &self.consistency_level {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ impl CreateOrReplaceSlugAttachmentBuilder {
pub fn into_future(self) -> CreateOrReplaceSlugAttachment {
Box::pin(async move {
let mut request = if self.is_create {
self.client.attachments_request(Method::POST)
self.client.attachments_request(Method::Post)
} else {
self.client.attachment_request(Method::PUT)
self.client.attachment_request(Method::Put)
};

request.insert_headers(&self.if_match_condition);
Expand Down
4 changes: 2 additions & 2 deletions sdk/data_cosmos/src/operations/create_or_replace_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ impl CreateOrReplaceTriggerBuilder {
pub fn into_future(self) -> CreateOrReplaceTrigger {
Box::pin(async move {
let mut request = if self.is_create {
self.client.triggers_request(azure_core::Method::POST)
self.client.triggers_request(azure_core::Method::Post)
} else {
self.client.trigger_request(azure_core::Method::PUT)
self.client.trigger_request(azure_core::Method::Put)
};

if let Some(cl) = &self.consistency_level {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ impl CreateOrReplaceUserDefinedFunctionBuilder {
pub fn into_future(self) -> CreateOrReplaceUserDefinedFunction {
Box::pin(async move {
let mut request = match self.is_create {
true => self.client.udfs_request(azure_core::Method::POST),
false => self.client.udf_request(azure_core::Method::PUT),
true => self.client.udfs_request(azure_core::Method::Post),
false => self.client.udf_request(azure_core::Method::Put),
};

if let Some(cl) = &self.consistency_level {
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/create_permission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl CreatePermissionBuilder {
self.client.database_client().database_name(),
self.client.user_client().user_name()
),
azure_core::Method::POST,
azure_core::Method::Post,
);

if let Some(cl) = &self.consistency_level {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl CreateStoredProcedureBuilder {
Box::pin(async move {
let mut req = self
.client
.stored_procedures_request(azure_core::Method::POST);
.stored_procedures_request(azure_core::Method::Post);

if let Some(cl) = &self.consistency_level {
req.insert_headers(cl);
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/create_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl CreateUserBuilder {
"dbs/{}/users",
self.client.database_client().database_name()
),
azure_core::Method::POST,
azure_core::Method::Post,
);

if let Some(cl) = &self.consistency_level {
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/delete_attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl DeleteAttachmentBuilder {

pub fn into_future(self) -> DeleteAttachment {
Box::pin(async move {
let mut request = self.client.attachment_request(azure_core::Method::DELETE);
let mut request = self.client.attachment_request(azure_core::Method::Delete);

request.insert_headers(&self.if_match_condition);
if let Some(cl) = &self.consistency_level {
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/delete_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl DeleteCollectionBuilder {

pub fn into_future(self) -> DeleteCollection {
Box::pin(async move {
let mut request = self.client.collection_request(azure_core::Method::DELETE);
let mut request = self.client.collection_request(azure_core::Method::Delete);

if let Some(cl) = &self.consistency_level {
request.insert_headers(cl);
Expand Down
2 changes: 1 addition & 1 deletion sdk/data_cosmos/src/operations/delete_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl DeleteDatabaseBuilder {

pub fn into_future(self) -> DeleteDatabase {
Box::pin(async move {
let mut request = self.client.database_request(azure_core::Method::DELETE);
let mut request = self.client.database_request(azure_core::Method::Delete);
if let Some(cl) = &self.consistency_level {
request.insert_headers(cl);
}
Expand Down
Loading