[refact] Introduce NetworkResult and NetworkError types (#64)

Extracted from #60
Working on #63

In this PR we are just introducing new types `NetworkResult` and
`NetworkError` that will be used inside the `network` module. Our
purpose is to isolate inside this `network` module everything related to
outside-world communications and provide a clear (and small) interface
that can be reused from the other modules in the crate.

Signed-off-by: Javier G. Sogo <[email protected]>

Author: jgsogo

src/errors.rs

@@ -2,6 +2,7 @@ use std::sync::PoisonError;
 
 use thiserror::Error;
 
+use crate::network::errors::NetworkError;
 use crate::segment_evaluation::errors::SegmentEvaluationError;
 
 pub type Result<T> = std::result::Result<T, Error>;
@@ -28,9 +29,6 @@ pub enum Error {
     #[error("Inner type cannot be converted to requested type")]
     MismatchType,
 
-    #[error(transparent)]
-    ReqwestError(#[from] reqwest::Error),
-
     #[error(transparent)]
     TungsteniteError(#[from] tungstenite::Error),
 
@@ -49,6 +47,9 @@ pub enum Error {
     #[error("Failed to evaluate entity: {0}")]
     EntityEvaluationError(EntityEvaluationError),
 
+    #[error(transparent)]
+    NetworkError(#[from] NetworkError),
+
     #[error("{0}")]
     Other(String),
 }

src/lib.rs

@@ -105,6 +105,6 @@ pub use value::Value;
 #[cfg(feature = "http_client")]
 pub use client::AppConfigurationClientHttp;
 #[cfg(feature = "http_client")]
-pub use network::{ServiceAddress, TokenProvider};
+pub use network::{NetworkError, NetworkResult, ServiceAddress, TokenProvider};
 #[cfg(test)]
 mod tests;

src/network/errors.rs

@@ -0,0 +1,33 @@
+// (C) Copyright IBM Corp. 2024.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use thiserror::Error;
+
+#[derive(Debug, Error)]
+pub enum NetworkError {
+    #[error(transparent)]
+    ReqwestError(#[from] reqwest::Error),
+
+    #[error(transparent)]
+    TungsteniteError(#[from] tungstenite::Error),
+
+    #[error("Protocol error. Unexpected data received from server")]
+    ProtocolError,
+
+    #[error("Cannot parse '{0}' as URL")]
+    UrlParseError(String),
+
+    #[error("Invalid header value for '{0}'")]
+    InvalidHeaderValue(String),
+}

src/network/http_client.rs

@@ -12,9 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use super::TokenProvider;
+use super::{NetworkError, NetworkResult, TokenProvider};
 use crate::models::ConfigurationJson;
-use crate::{ConfigurationId, Error, Result};
+use crate::ConfigurationId;
 use reqwest::blocking::Client;
 use std::cell::RefCell;
 use tungstenite::stream::MaybeTlsStream;
@@ -101,7 +101,7 @@ impl ServerClientImpl {
     pub fn new(
         service_address: ServiceAddress,
         token_provider: Box<dyn TokenProvider>,
-    ) -> Result<Self> {
+    ) -> NetworkResult<Self> {
         let access_token = RefCell::new(token_provider.get_access_token()?);
         Ok(Self {
             service_address,
@@ -110,29 +110,31 @@ impl ServerClientImpl {
         })
     }
 
-    pub fn get_configuration(&self, collection: &ConfigurationId) -> Result<ConfigurationJson> {
+    pub fn get_configuration(
+        &self,
+        configuration_id: &ConfigurationId,
+    ) -> NetworkResult<ConfigurationJson> {
         let url = format!(
             "{}/feature/v1/instances/{}/config",
             self.service_address.base_url(ServiceAddressProtocol::Http),
-            collection.guid
+            configuration_id.guid
         );
+        let url = Url::parse(&url).map_err(|_| NetworkError::UrlParseError(url))?;
         let client = Client::new();
         let r = client
             .get(url)
             .query(&[
                 ("action", "sdkConfig"),
-                ("environment_id", &collection.environment_id),
-                ("collection_id", &collection.collection_id),
+                ("environment_id", &configuration_id.environment_id),
+                ("collection_id", &configuration_id.collection_id),
             ])
             .header("Accept", "application/json")
             .header("User-Agent", "appconfiguration-rust-sdk/0.0.1")
             .bearer_auth(self.access_token.borrow())
             .send();
 
         match r {
-            Ok(response) => response.json().map_err(|_| {
-                Error::ProtocolError("Failed to deserialize JSON from server response".to_string())
-            }),
+            Ok(response) => response.json().map_err(|_| NetworkError::ProtocolError),
             Err(e) => {
                 // TODO: Identify if token expired, get new one and retry
                 if false {
@@ -147,13 +149,12 @@ impl ServerClientImpl {
     pub fn get_configuration_monitoring_websocket(
         &self,
         collection: &ConfigurationId,
-    ) -> Result<(WebSocket<MaybeTlsStream<TcpStream>>, Response)> {
+    ) -> NetworkResult<(WebSocket<MaybeTlsStream<TcpStream>>, Response)> {
         let ws_url = format!(
             "{}/wsfeature",
             self.service_address.base_url(ServiceAddressProtocol::Ws)
         );
-        let mut ws_url = Url::parse(&ws_url)
-            .map_err(|e| Error::Other(format!("Cannot parse '{}' as URL: {}", ws_url, e)))?;
+        let mut ws_url = Url::parse(&ws_url).map_err(|_| NetworkError::UrlParseError(ws_url))?;
 
         ws_url
             .query_pairs_mut()
@@ -164,21 +165,19 @@ impl ServerClientImpl {
         let mut request = ws_url
             .as_str()
             .into_client_request()
-            .map_err(Error::TungsteniteError)?;
+            .map_err(NetworkError::TungsteniteError)?;
         let headers = request.headers_mut();
         headers.insert(
             "User-Agent",
             "appconfiguration-rust-sdk/0.0.1"
                 .parse()
-                .map_err(|_| Error::Other("Invalid header value for 'User-Agent'".to_string()))?,
+                .map_err(|_| NetworkError::InvalidHeaderValue("User-Agent".to_string()))?,
         );
         headers.insert(
             "Authorization",
             format!("Bearer {}", self.access_token.borrow())
                 .parse()
-                .map_err(|_| {
-                    Error::Other("Invalid header value for 'Authorization'".to_string())
-                })?,
+                .map_err(|_| NetworkError::InvalidHeaderValue("Authorization".to_string()))?,
         );
 
         Ok(connect(request)?)

src/network/mod.rs

@@ -12,10 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+pub mod errors;
 pub(crate) mod http_client;
 mod token_provider;
 
 pub(crate) use http_client::ServerClientImpl;
 pub use http_client::ServiceAddress;
 pub(crate) use token_provider::IBMCloudTokenProvider;
 pub use token_provider::TokenProvider;
+
+pub use errors::NetworkError;
+pub type NetworkResult<T> = std::result::Result<T, NetworkError>;

src/network/token_provider.rs

@@ -14,12 +14,12 @@
 
 use std::collections::HashMap;
 
-use crate::{Error, Result};
+use super::{NetworkError, NetworkResult};
 use reqwest::blocking::Client;
 use serde::Deserialize;
 
 pub trait TokenProvider: std::fmt::Debug + Send + Sync {
-    fn get_access_token(&self) -> Result<String>;
+    fn get_access_token(&self) -> NetworkResult<String>;
 }
 
 #[derive(Debug)]
@@ -41,7 +41,7 @@ struct AccessTokenResponse {
 }
 
 impl TokenProvider for IBMCloudTokenProvider {
-    fn get_access_token(&self) -> Result<String> {
+    fn get_access_token(&self) -> NetworkResult<String> {
         let mut form_data = HashMap::new();
         form_data.insert("reponse_type".to_string(), "cloud_iam".to_string());
         form_data.insert(
@@ -56,9 +56,9 @@ impl TokenProvider for IBMCloudTokenProvider {
             .header("Accept", "application/json")
             .form(&form_data)
             .send()
-            .map_err(Error::ReqwestError)?
+            .map_err(NetworkError::ReqwestError)?
             .json::<AccessTokenResponse>()
-            .map_err(Error::ReqwestError)? // FIXME: This is a deserialization error (extract it from Reqwest)
+            .map_err(NetworkError::ReqwestError)? // FIXME: This is a deserialization error (extract it from Reqwest)
             .access_token)
     }
 }

tests/test_initial_configuration_fails.rs

@@ -4,7 +4,7 @@ use std::sync::mpsc::channel;
 use std::thread::spawn;
 
 use appconfiguration::{
-    AppConfigurationClientHttp, ConfigurationId, Error, ServiceAddress, TokenProvider,
+    AppConfigurationClientHttp, ConfigurationId, NetworkError, ServiceAddress, TokenProvider,
 };
 
 fn handle_config_request_error(server: &TcpListener) {
@@ -83,7 +83,7 @@ fn server_thread() -> ServerHandle {
 struct MockTokenProvider {}
 
 impl TokenProvider for MockTokenProvider {
-    fn get_access_token(&self) -> appconfiguration::Result<String> {
+    fn get_access_token(&self) -> appconfiguration::NetworkResult<String> {
         Ok("mock_token".into())
     }
 }
@@ -111,12 +111,18 @@ fn main() {
     );
 
     assert!(client.is_err());
-    assert!(matches!(client.unwrap_err(), Error::ReqwestError(_)));
+    assert!(matches!(
+        client.unwrap_err(),
+        appconfiguration::Error::NetworkError(NetworkError::ReqwestError(_))
+    ));
 
     // Test response is successful (200) but configuration JSON is invalid
     let client =
         AppConfigurationClientHttp::new(address, Box::new(MockTokenProvider {}), config_id);
 
     assert!(client.is_err());
-    assert!(matches!(client.unwrap_err(), Error::ProtocolError(_)));
+    assert!(matches!(
+        client.unwrap_err(),
+        appconfiguration::Error::NetworkError(NetworkError::ProtocolError)
+    ));
 }

tests/test_initial_configuration_from_server.rs

@@ -101,7 +101,7 @@ fn server_thread() -> ServerHandle {
 struct MockTokenProvider {}
 
 impl TokenProvider for MockTokenProvider {
-    fn get_access_token(&self) -> appconfiguration::Result<String> {
+    fn get_access_token(&self) -> appconfiguration::NetworkResult<String> {
         Ok("mock_token".into())
     }
 }

tests/test_ws_connection_fails.rs

@@ -1,5 +1,5 @@
 use appconfiguration::{
-    AppConfigurationClientHttp, ConfigurationId, Error, ServiceAddress, TokenProvider,
+    AppConfigurationClientHttp, ConfigurationId, NetworkError, ServiceAddress, TokenProvider,
 };
 
 use std::io::{BufRead, BufReader, Write};
@@ -67,7 +67,7 @@ fn server_thread() -> ServerHandle {
 struct MockTokenProvider {}
 
 impl TokenProvider for MockTokenProvider {
-    fn get_access_token(&self) -> appconfiguration::Result<String> {
+    fn get_access_token(&self) -> appconfiguration::NetworkResult<String> {
         Ok("mock_token".into())
     }
 }
@@ -90,5 +90,8 @@ fn main() {
         AppConfigurationClientHttp::new(address, Box::new(MockTokenProvider {}), config_id);
 
     assert!(client.is_err());
-    assert!(matches!(client.unwrap_err(), Error::TungsteniteError(_)));
+    assert!(matches!(
+        client.unwrap_err(),
+        appconfiguration::Error::NetworkError(NetworkError::TungsteniteError(_))
+    ));
 }