replace unwrap calls in azure_core (#661)

Author: bmc-msft

sdk/core/src/context.rs

@@ -32,7 +32,7 @@ impl Context {
         // the `unwrap` below is safe.
         self.type_map
             .insert(TypeId::of::<E>(), Arc::new(entity))
-            .map(|displaced| displaced.downcast().unwrap())
+            .map(|displaced| displaced.downcast().expect("failed to unwrap downcast"))
     }
 
     /// Inserts an entity in the type map. If the an entity with the same type signature is
@@ -54,7 +54,7 @@ impl Context {
     {
         self.type_map
             .remove(&TypeId::of::<E>())
-            .map(|removed| removed.downcast().unwrap())
+            .map(|removed| removed.downcast().expect("failed to unwrap downcast"))
     }
 
     /// Returns a reference of the entity of the specified type signature, if it exists.

sdk/core/src/error/azure_core_errors.rs

@@ -36,6 +36,7 @@ impl From<crate::errors::HttpError> for Error {
             crate::HttpError::Utf8(e) => Error::new(ErrorKind::DataConversion, e),
             crate::HttpError::BuildResponse(e) => Error::new(ErrorKind::DataConversion, e),
             crate::HttpError::BuildClientRequest(e) => Error::new(ErrorKind::Other, e),
+            crate::HttpError::Url(e) => Error::new(ErrorKind::Other, e),
         }
     }
 }

sdk/core/src/error/http_error.rs

@@ -1,4 +1,5 @@
 use crate::Response;
+use std::collections::HashMap;
 
 /// An unsuccessful HTTP response
 #[derive(Debug)]
@@ -16,13 +17,13 @@ impl HttpError {
     pub async fn new(response: Response) -> Self {
         let status = response.status();
         let mut error_code = get_error_code_from_header(&response);
-        let headers = response
-            .headers()
-            .into_iter()
-            // TODO: the following will panic if a non-UTF8 header value is sent back
-            // We should not panic but instead handle this gracefully
-            .map(|(n, v)| (n.as_str().to_owned(), v.to_str().unwrap().to_owned()))
-            .collect();
+        let mut headers = HashMap::new();
+
+        for (name, value) in response.headers() {
+            let value = String::from_utf8_lossy(value.as_bytes()).to_string();
+            headers.insert(name.to_string(), value);
+        }
+
         let body = response.into_body_string().await;
         error_code = error_code.or_else(|| get_error_code_from_body(&body));
         HttpError {

sdk/core/src/errors.rs

@@ -133,6 +133,8 @@ pub enum HttpError {
     BuildResponse(#[source] http::Error),
     #[error("failed to reset stream")]
     StreamReset(#[source] StreamError),
+    #[error("failed to parse URL")]
+    Url(#[from] url::ParseError),
 }
 
 /// An error caused by invalid permissions.

sdk/core/src/http_client.rs

@@ -108,10 +108,8 @@ pub trait HttpClient: Send + Sync + std::fmt::Debug {
 #[cfg_attr(not(target_arch = "wasm32"), async_trait)]
 impl HttpClient for reqwest::Client {
     async fn execute_request(&self, request: Request<Bytes>) -> Result<Response<Bytes>, HttpError> {
-        let mut reqwest_request = self.request(
-            request.method().clone(),
-            url::Url::parse(&request.uri().to_string()).unwrap(),
-        );
+        let url = url::Url::parse(&request.uri().to_string())?;
+        let mut reqwest_request = self.request(request.method().clone(), url);
         for (header, value) in request.headers() {
             reqwest_request = reqwest_request.header(header, value);
         }
@@ -149,10 +147,8 @@ impl HttpClient for reqwest::Client {
         &self,
         request: &crate::Request,
     ) -> Result<crate::Response, HttpError> {
-        let mut reqwest_request = self.request(
-            request.method(),
-            url::Url::parse(&request.uri().to_string()).unwrap(),
-        );
+        let url = url::Url::parse(&request.uri().to_string())?;
+        let mut reqwest_request = self.request(request.method(), url);
         for header in request.headers() {
             reqwest_request = reqwest_request.header(header.0, header.1);
         }

sdk/core/src/policies/retry_policies/retry_policy.rs

@@ -61,14 +61,18 @@ where
                 }
                 Ok(response) => {
                     // Error status code
-                    let error = HttpError::new(response).await;
-                    let status = StatusCode::from_u16(error.status()).unwrap();
+                    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 error = Error::full(
                         ErrorKind::http_response(
-                            status.as_u16(),
-                            error.error_code().map(|s| s.to_owned()),
+                            code,
+                            http_error.error_code().map(|s| s.to_owned()),
                         ),
-                        error,
+                        http_error,
                         "server returned error status which will not be retried",
                     );
 

sdk/core/src/request_options/content_range.rs

@@ -49,7 +49,14 @@ impl FromStr for ContentRange {
             })?;
 
         let mut split_at_dash = remaining.split('-');
-        let start = split_at_dash.next().unwrap().parse()?;
+        let start = split_at_dash
+            .next()
+            .ok_or_else(|| ParseError::TokenNotFound {
+                item: "ContentRange",
+                token: "-".to_owned(),
+                full: s.into(),
+            })?
+            .parse()?;
 
         let mut split_at_slash = split_at_dash
             .next()
@@ -60,7 +67,15 @@ impl FromStr for ContentRange {
             })?
             .split('/');
 
-        let end = split_at_slash.next().unwrap().parse()?;
+        let end = split_at_slash
+            .next()
+            .ok_or_else(|| ParseError::TokenNotFound {
+                item: "ContentRange",
+                token: "/".to_owned(),
+                full: s.into(),
+            })?
+            .parse()?;
+
         let total_length = split_at_slash
             .next()
             .ok_or_else(|| ParseError::TokenNotFound {

sdk/core/src/request_options/metadata.rs

@@ -78,12 +78,10 @@ impl From<&HeaderMap> for Metadata {
         header_map
             .iter()
             .map(|header| (header.0.as_str(), header.1.as_bytes()))
-            .filter(|(key, _)| key.starts_with("x-ms-meta-"))
             .for_each(|(key, value)| {
-                metadata.insert(
-                    key.strip_prefix("x-ms-meta-").unwrap().to_owned(),
-                    value.to_owned(),
-                );
+                if let Some(key) = key.strip_prefix("x-ms-meta-") {
+                    metadata.insert(key.to_owned(), value.to_owned());
+                }
             });
 
         metadata