Skip to content

make reqwest optional for azure_core #777

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 4 commits into from
May 31, 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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ jobs:
cargo fmt --manifest-path services/Cargo.toml --all -- --check
if: matrix.rust == 'stable'

- name: check core with --no-default-features
run: cargo check -p azure_core --no-default-features

- name: check core for wasm
run: cargo check -p azure_core --target=wasm32-unknown-unknown

# - name: check core for hyper
# run: cargo check -p azure_core --no-default-features --features enable_hyper

- name: sdk tests
run: cargo test --all --features mock_transport_framework

Expand Down
5 changes: 1 addition & 4 deletions sdk/core/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ impl From<super::error::Error> for Error {
}
}

#[cfg(feature = "enable_hyper")]
type HttpClientError = hyper::Error;
#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
type HttpClientError = reqwest::Error;
type HttpClientError = Box<dyn std::error::Error + Send + Sync + 'static>;

/// An error caused by a failure to parse data.
#[non_exhaustive]
Expand Down
28 changes: 9 additions & 19 deletions sdk/core/src/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,12 @@ use bytes::Bytes;
#[allow(unused_imports)]
use futures::TryStreamExt;
use http::{Request, Response, StatusCode};
#[cfg(feature = "enable_hyper")]
#[allow(unused_imports)]
use hyper_rustls::HttpsConnector;
use serde::Serialize;
use std::sync::Arc;

/// Construct a new HTTP client with the `reqwest` backend.
/// Construct a new `HttpClient` with the `reqwest` backend.
#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
pub fn new_http_client() -> Arc<dyn HttpClient> {
Arc::new(reqwest::Client::new())
}

/// Construct a new HTTP client with the `hyper` backend.
#[cfg(feature = "enable_hyper")]
pub fn new_http_client() -> Arc<dyn HttpClient> {
Arc::new(hyper::Client::builder().build(hyper_rustls::HttpsConnector::with_native_roots()))
pub fn new_http_client() -> std::sync::Arc<dyn HttpClient> {
std::sync::Arc::new(reqwest::Client::new())
}

/// An HTTP client which can send requests.
Expand Down Expand Up @@ -82,12 +72,12 @@ impl HttpClient for reqwest::Client {
let reqwest_request = reqwest_request
.body(request.into_body())
.build()
.map_err(HttpError::BuildClientRequest)?;
.map_err(|error| HttpError::BuildClientRequest(error.into()))?;

let reqwest_response = self
.execute(reqwest_request)
.await
.map_err(HttpError::ExecuteRequest)?;
.map_err(|error| HttpError::ExecuteRequest(error.into()))?;

let mut response = Response::builder().status(reqwest_response.status());

Expand All @@ -100,7 +90,7 @@ impl HttpClient for reqwest::Client {
reqwest_response
.bytes()
.await
.map_err(HttpError::ReadBytes)?,
.map_err(|error| HttpError::ReadBytes(error.into()))?,
)
.map_err(HttpError::BuildResponse)?;

Expand All @@ -125,21 +115,21 @@ impl HttpClient for reqwest::Client {
Body::Bytes(bytes) => reqwest_request
.body(bytes)
.build()
.map_err(HttpError::BuildClientRequest)?,
.map_err(|error| HttpError::BuildClientRequest(error.into()))?,
Body::SeekableStream(mut seekable_stream) => {
seekable_stream.reset().await.unwrap(); // TODO: remove unwrap when `HttpError` has been removed

reqwest_request
.body(reqwest::Body::wrap_stream(seekable_stream))
.build()
.map_err(HttpError::BuildClientRequest)?
.map_err(|error| HttpError::BuildClientRequest(error.into()))?
}
};

let reqwest_response = self
.execute(reqwest_request)
.await
.map_err(HttpError::ExecuteRequest)?;
.map_err(|error| HttpError::ExecuteRequest(error.into()))?;
let mut response = crate::ResponseBuilder::new(reqwest_response.status());

for (key, value) in reqwest_response.headers() {
Expand Down
24 changes: 18 additions & 6 deletions sdk/core/src/options.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::policies::{ExponentialRetryPolicy, FixedRetryPolicy, NoRetryPolicy, Policy};
use crate::{new_http_client, HttpClient};
use crate::HttpClient;
use std::sync::Arc;
use std::time::Duration;

Expand All @@ -15,7 +15,11 @@ use std::time::Duration;
/// .retry(RetryOptions::default().max_retries(10u32))
/// .telemetry(TelemetryOptions::default().application_id("my-application"));
/// ```
#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug)]
#[cfg_attr(
any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"),
derive(Default)
)]
pub struct ClientOptions {
/// Policies called per call.
pub(crate) per_call_policies: Vec<Arc<dyn Policy>>,
Expand All @@ -30,8 +34,14 @@ pub struct ClientOptions {
}

impl ClientOptions {
pub fn new() -> Self {
Self::default()
pub fn new(transport: TransportOptions) -> Self {
Self {
per_call_policies: Vec::new(),
per_retry_policies: Vec::new(),
retry: RetryOptions::default(),
telemetry: TelemetryOptions::default(),
transport,
}
}

#[cfg(feature = "mock_transport_framework")]
Expand Down Expand Up @@ -185,17 +195,19 @@ impl TransportOptions {
}

#[cfg(feature = "mock_transport_framework")]
#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
pub fn new_with_transaction_name(transaction_name: String) -> Self {
Self {
http_client: new_http_client(),
http_client: crate::http_client::new_http_client(),
transaction_name,
}
}
}

#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
impl Default for TransportOptions {
/// Creates an instance of the `TransportOptions` using the default `HttpClient`.
fn default() -> Self {
Self::new(new_http_client())
Self::new(crate::http_client::new_http_client())
}
}
14 changes: 10 additions & 4 deletions sdk/core/src/response.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use bytes::Bytes;
use futures::Stream;
use futures::StreamExt;
use http::{header::HeaderName, HeaderMap, HeaderValue, StatusCode};
use http::{HeaderMap, StatusCode};
use std::pin::Pin;

type PinnedStream = Pin<Box<dyn Stream<Item = crate::error::Result<Bytes>> + Send + Sync>>;

#[allow(dead_code)]
#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
pub(crate) struct ResponseBuilder {
status: StatusCode,
headers: HeaderMap,
}

#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
impl ResponseBuilder {
pub fn new(status: StatusCode) -> Self {
Self {
Expand All @@ -20,8 +21,12 @@ impl ResponseBuilder {
}
}

#[allow(dead_code)]
pub fn with_header(&mut self, key: &HeaderName, value: HeaderValue) -> &mut Self {
#[cfg(not(target_arch = "wasm32"))]
pub fn with_header(
&mut self,
key: &http::header::HeaderName,
value: http::HeaderValue,
) -> &mut Self {
self.headers.append(key, value);
self
}
Expand All @@ -39,6 +44,7 @@ pub struct Response {
}

impl Response {
#[cfg(any(feature = "enable_reqwest", feature = "enable_reqwest_rustls"))]
pub(crate) fn new(status: StatusCode, headers: HeaderMap, body: PinnedStream) -> Self {
Self {
status,
Expand Down
10 changes: 5 additions & 5 deletions sdk/device_update/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ impl DeviceUpdateClient {
.bearer_auth(self.get_token().await?.token.secret())
.send()
.await
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e))))?;
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e.into()))))?;

let body = resp
.bytes()
.await
.map_err(|e| Error::Core(CoreError::Http(HttpError::ReadBytes(e))))?;
.map_err(|e| Error::Core(CoreError::Http(HttpError::ReadBytes(e.into()))))?;
serde_json::from_slice(&body).map_err(|e| Error::Core(CoreError::Json(e)))
}

Expand All @@ -97,7 +97,7 @@ impl DeviceUpdateClient {
let resp = req
.send()
.await
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e))))?;
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e.into()))))?;

if resp.status() == 202u16 {
let headers = resp.headers();
Expand All @@ -120,11 +120,11 @@ impl DeviceUpdateClient {
.header("Content-Type", "application/json")
.send()
.await
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e))))?;
.map_err(|e| Error::Core(CoreError::Http(HttpError::ExecuteRequest(e.into()))))?;
let body = resp
.text()
.await
.map_err(|e| Error::Core(CoreError::Http(HttpError::ReadBytes(e))))?;
.map_err(|e| Error::Core(CoreError::Http(HttpError::ReadBytes(e.into()))))?;
Ok(body)
}
}
Expand Down