refactor(rust): reduce ServiceError (round 1)

rust/agama-cli/src/lib.rs

@@ -204,7 +204,7 @@ async fn build_http_client(
     api_url: Url,
     insecure: bool,
     authenticated: bool,
-) -> Result<BaseHTTPClient, ServiceError> {
+) -> anyhow::Result<BaseHTTPClient> {
     let mut client = BaseHTTPClient::new(api_url)?;
 
     if insecure {
@@ -216,11 +216,11 @@ async fn build_http_client(
     if authenticated {
         // this deals with authentication need inside
         if let Some(token) = find_client_token(&client.base_url) {
-            return client.authenticated(&token);
+            return Ok(client.authenticated(&token)?);
         }
-        return Err(ServiceError::NotAuthenticated);
+        return Err(ServiceError::NotAuthenticated.into());
     } else {
-        client.unauthenticated()
+        Ok(client.unauthenticated()?)
     }
 }
 

rust/agama-cli/src/logs.rs

@@ -19,7 +19,7 @@
 // find current contact information at www.suse.com.
 
 use agama_lib::base_http_client::BaseHTTPClient;
-use agama_lib::manager::http_client::ManagerHTTPClient as HTTPClient;
+use agama_lib::manager::http_client::ManagerHTTPClient;
 use clap::Subcommand;
 use std::io;
 use std::path::PathBuf;
@@ -40,7 +40,7 @@ pub enum LogsCommands {
 
 /// Main entry point called from agama CLI main loop
 pub async fn run(client: BaseHTTPClient, subcommand: LogsCommands) -> anyhow::Result<()> {
-    let client = HTTPClient::new(client);
+    let client = ManagerHTTPClient::new(client);
 
     match subcommand {
         LogsCommands::Store { destination } => {

rust/agama-cli/src/questions.rs

@@ -18,9 +18,11 @@
 // To contact SUSE LLC about this file by physical or electronic mail, you may
 // find current contact information at www.suse.com.
 
-use agama_lib::proxies::questions::QuestionsProxy;
-use agama_lib::questions::http_client::HTTPClient;
-use agama_lib::{base_http_client::BaseHTTPClient, connection, error::ServiceError};
+use agama_lib::{
+    base_http_client::BaseHTTPClient, connection, proxies::questions::QuestionsProxy,
+    questions::http_client::HTTPClient,
+};
+use anyhow::anyhow;
 use clap::{Args, Subcommand, ValueEnum};
 
 // TODO: use for answers also JSON to be consistent
@@ -60,54 +62,47 @@ pub enum Modes {
     NonInteractive,
 }
 
-async fn set_mode(proxy: QuestionsProxy<'_>, value: Modes) -> Result<(), ServiceError> {
+async fn set_mode(proxy: QuestionsProxy<'_>, value: Modes) -> anyhow::Result<()> {
     proxy
         .set_interactive(value == Modes::Interactive)
         .await
         .map_err(|e| e.into())
 }
 
-async fn set_answers(proxy: QuestionsProxy<'_>, path: String) -> Result<(), ServiceError> {
+async fn set_answers(proxy: QuestionsProxy<'_>, path: String) -> anyhow::Result<()> {
     proxy
         .add_answer_file(path.as_str())
         .await
         .map_err(|e| e.into())
 }
 
-async fn list_questions(client: BaseHTTPClient) -> Result<(), ServiceError> {
+async fn list_questions(client: BaseHTTPClient) -> anyhow::Result<()> {
     let client = HTTPClient::new(client)?;
     let questions = client.list_questions().await?;
     // FIXME: if performance is bad, we can skip converting json from http to struct and then
     // serialize it, but it won't be pretty string
-    let questions_json = serde_json::to_string_pretty(&questions)
-        .map_err(|e| ServiceError::InternalError(e.to_string()))?;
+    let questions_json = serde_json::to_string_pretty(&questions)?;
     println!("{}", questions_json);
     Ok(())
 }
 
-async fn ask_question(client: BaseHTTPClient) -> Result<(), ServiceError> {
+async fn ask_question(client: BaseHTTPClient) -> anyhow::Result<()> {
     let client = HTTPClient::new(client)?;
     let question = serde_json::from_reader(std::io::stdin())?;
 
     let created_question = client.create_question(&question).await?;
     let Some(id) = created_question.generic.id else {
-        return Err(ServiceError::InternalError(
-            "Created question does not get id".to_string(),
-        ));
+        return Err(anyhow!("The created question does not have an ID"));
     };
     let answer = client.get_answer(id).await?;
-    let answer_json = serde_json::to_string_pretty(&answer)
-        .map_err(|e| ServiceError::InternalError(e.to_string()))?;
+    let answer_json = serde_json::to_string_pretty(&answer).map_err(|e| anyhow!(e.to_string()))?;
     println!("{}", answer_json);
 
     client.delete_question(id).await?;
     Ok(())
 }
 
-pub async fn run(
-    client: BaseHTTPClient,
-    subcommand: QuestionsCommands,
-) -> Result<(), ServiceError> {
+pub async fn run(client: BaseHTTPClient, subcommand: QuestionsCommands) -> anyhow::Result<()> {
     let connection = connection().await?;
     let proxy = QuestionsProxy::new(&connection).await?;
 

rust/agama-lib/src/base_http_client.rs

@@ -22,7 +22,21 @@ use reqwest::{header, IntoUrl, Response};
 use serde::{de::DeserializeOwned, Serialize};
 use url::Url;
 
-use crate::{auth::AuthToken, error::ServiceError};
+use crate::auth::AuthToken;
+
+#[derive(Debug, thiserror::Error)]
+pub enum BaseHTTPClientError {
+    #[error(transparent)]
+    HTTP(#[from] reqwest::Error),
+    #[error(transparent)]
+    InvalidHeaderValue(#[from] reqwest::header::InvalidHeaderValue),
+    #[error(transparent)]
+    InvalidURL(#[from] url::ParseError),
+    #[error(transparent)]
+    InvalidJSON(#[from] serde_json::Error),
+    #[error("Backend call failed with status {0} and text '{1}'")]
+    BackendError(u16, String),
+}
 
 /// Base that all HTTP clients should use.
 ///
@@ -33,10 +47,9 @@ use crate::{auth::AuthToken, error::ServiceError};
 ///
 /// ```no_run
 ///   use agama_lib::questions::model::Question;
-///   use agama_lib::base_http_client::BaseHTTPClient;
-///   use agama_lib::error::ServiceError;
+///   use agama_lib::base_http_client::{BaseHTTPClient, BaseHTTPClientError};
 ///
-///   async fn get_questions() -> Result<Vec<Question>, ServiceError> {
+///   async fn get_questions() -> Result<Vec<Question>, BaseHTTPClientError> {
 ///     let client = BaseHTTPClient::new("http://localhost/api/").unwrap();
 ///     client.get("questions").await
 ///   }
@@ -54,7 +67,7 @@ impl BaseHTTPClient {
     ///
     /// * `base_url`: base URL of the API to connect to. A trailing "/" is relevant if the URL
     ///   has a path.
-    pub fn new<T: IntoUrl>(base_url: T) -> Result<Self, ServiceError> {
+    pub fn new<T: IntoUrl>(base_url: T) -> Result<Self, BaseHTTPClientError> {
         let mut url = base_url.into_url()?;
 
         // A trailing slash is significant. Let's make sure that it is there.
@@ -81,32 +94,30 @@ impl BaseHTTPClient {
     /// Turns the client into an authenticated one using the given token.
     ///
     /// * `token`: authentication token.
-    pub fn authenticated(self, token: &AuthToken) -> Result<Self, ServiceError> {
+    pub fn authenticated(self, token: &AuthToken) -> Result<Self, BaseHTTPClientError> {
         Ok(Self {
             client: Self::authenticated_client(self.insecure, token)?,
             ..self
         })
     }
 
     /// Configures itself for connection(s) without authentication token
-    pub fn unauthenticated(self) -> Result<Self, ServiceError> {
+    pub fn unauthenticated(self) -> Result<Self, BaseHTTPClientError> {
         Ok(Self {
             client: reqwest::Client::builder()
                 .danger_accept_invalid_certs(self.insecure)
-                .build()
-                .map_err(anyhow::Error::new)?,
+                .build()?,
             ..self
         })
     }
 
     fn authenticated_client(
         insecure: bool,
         token: &AuthToken,
-    ) -> Result<reqwest::Client, ServiceError> {
+    ) -> Result<reqwest::Client, BaseHTTPClientError> {
         let mut headers = header::HeaderMap::new();
         // just use generic anyhow error here as Bearer format is constructed by us, so failures can come only from token
-        let value = header::HeaderValue::from_str(format!("Bearer {}", token).as_str())
-            .map_err(anyhow::Error::new)?;
+        let value = header::HeaderValue::from_str(format!("Bearer {}", token).as_str())?;
 
         headers.insert(header::AUTHORIZATION, value);
 
@@ -127,11 +138,11 @@ impl BaseHTTPClient {
     /// Arguments:
     ///
     /// * `path`: path relative to HTTP API like `/questions`
-    pub async fn get<T>(&self, path: &str) -> Result<T, ServiceError>
+    pub async fn get<T>(&self, path: &str) -> Result<T, BaseHTTPClientError>
     where
         T: DeserializeOwned,
     {
-        let response: Result<_, ServiceError> = self
+        let response: Result<_, BaseHTTPClientError> = self
             .client
             .get(self.url(path)?)
             .send()
@@ -140,7 +151,11 @@ impl BaseHTTPClient {
         self.deserialize_or_error(response?).await
     }
 
-    pub async fn post<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
+    pub async fn post<T>(
+        &self,
+        path: &str,
+        object: &impl Serialize,
+    ) -> Result<T, BaseHTTPClientError>
     where
         T: DeserializeOwned,
     {
@@ -156,7 +171,11 @@ impl BaseHTTPClient {
     ///
     /// * `path`: path relative to HTTP API like `/questions`
     /// * `object`: Object that can be serialiazed to JSON as body of request.
-    pub async fn post_void(&self, path: &str, object: &impl Serialize) -> Result<(), ServiceError> {
+    pub async fn post_void(
+        &self,
+        path: &str,
+        object: &impl Serialize,
+    ) -> Result<(), BaseHTTPClientError> {
         let response = self
             .request_response(reqwest::Method::POST, path, object)
             .await?;
@@ -169,7 +188,11 @@ impl BaseHTTPClient {
     ///
     /// * `path`: path relative to HTTP API like `/users/first`
     /// * `object`: Object that can be serialiazed to JSON as body of request.
-    pub async fn put<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
+    pub async fn put<T>(
+        &self,
+        path: &str,
+        object: &impl Serialize,
+    ) -> Result<T, BaseHTTPClientError>
     where
         T: DeserializeOwned,
     {
@@ -185,7 +208,11 @@ impl BaseHTTPClient {
     ///
     /// * `path`: path relative to HTTP API like `/users/first`
     /// * `object`: Object that can be serialiazed to JSON as body of request.
-    pub async fn put_void(&self, path: &str, object: &impl Serialize) -> Result<(), ServiceError> {
+    pub async fn put_void(
+        &self,
+        path: &str,
+        object: &impl Serialize,
+    ) -> Result<(), BaseHTTPClientError> {
         let response = self
             .request_response(reqwest::Method::PUT, path, object)
             .await?;
@@ -198,7 +225,11 @@ impl BaseHTTPClient {
     ///
     /// * `path`: path relative to HTTP API like `/users/first`
     /// * `object`: Object that can be serialiazed to JSON as body of request.
-    pub async fn patch<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
+    pub async fn patch<T>(
+        &self,
+        path: &str,
+        object: &impl Serialize,
+    ) -> Result<T, BaseHTTPClientError>
     where
         T: DeserializeOwned,
     {
@@ -212,7 +243,7 @@ impl BaseHTTPClient {
         &self,
         path: &str,
         object: &impl Serialize,
-    ) -> Result<(), ServiceError> {
+    ) -> Result<(), BaseHTTPClientError> {
         let response = self
             .request_response(reqwest::Method::PATCH, path, object)
             .await?;
@@ -224,8 +255,8 @@ impl BaseHTTPClient {
     /// Arguments:
     ///
     /// * `path`: path relative to HTTP API like `/questions/1`
-    pub async fn delete_void(&self, path: &str) -> Result<(), ServiceError> {
-        let response: Result<_, ServiceError> = self
+    pub async fn delete_void(&self, path: &str) -> Result<(), BaseHTTPClientError> {
+        let response: Result<_, BaseHTTPClientError> = self
             .client
             .delete(self.url(path)?)
             .send()
@@ -236,8 +267,8 @@ impl BaseHTTPClient {
 
     /// Returns raw reqwest::Response. Use e.g. in case when response content is not
     /// JSON body but e.g. binary data
-    pub async fn get_raw(&self, path: &str) -> Result<Response, ServiceError> {
-        let raw: Result<_, ServiceError> = self
+    pub async fn get_raw(&self, path: &str) -> Result<Response, BaseHTTPClientError> {
+        let raw: Result<_, BaseHTTPClientError> = self
             .client
             .get(self.url(path)?)
             .send()
@@ -268,7 +299,7 @@ impl BaseHTTPClient {
         method: reqwest::Method,
         path: &str,
         object: &impl Serialize,
-    ) -> Result<Response, ServiceError> {
+    ) -> Result<Response, BaseHTTPClientError> {
         self.client
             .request(method, self.url(path)?)
             .json(object)
@@ -277,8 +308,11 @@ impl BaseHTTPClient {
             .map_err(|e| e.into())
     }
 
-    /// Return deserialized JSON body as `Ok(T)` or an `Err` with [`ServiceError::BackendError`]
-    pub async fn deserialize_or_error<T>(&self, response: Response) -> Result<T, ServiceError>
+    /// Return deserialized JSON body as `Ok(T)` or an `Err` with [`BaseHTTPClientError::BackendError`]
+    pub async fn deserialize_or_error<T>(
+        &self,
+        response: Response,
+    ) -> Result<T, BaseHTTPClientError>
     where
         T: DeserializeOwned,
     {
@@ -291,7 +325,8 @@ impl BaseHTTPClient {
             // BUT also peek into the response text, in case something is wrong
             // so this copies the implementation from the above and adds a debug part
 
-            let bytes_r: Result<_, ServiceError> = response.bytes().await.map_err(|e| e.into());
+            let bytes_r: Result<_, BaseHTTPClientError> =
+                response.bytes().await.map_err(|e| e.into());
             let bytes = bytes_r?;
 
             // DEBUG: (we expect JSON so dbg! would escape too much, eprintln! is better)
@@ -304,8 +339,8 @@ impl BaseHTTPClient {
         }
     }
 
-    /// Return `Ok(())` or an `Err` with [`ServiceError::BackendError`]
-    async fn unit_or_error(&self, response: Response) -> Result<(), ServiceError> {
+    /// Return `Ok(())` or an `Err` with [`BaseHTTPClientError::BackendError`]
+    async fn unit_or_error(&self, response: Response) -> Result<(), BaseHTTPClientError> {
         if response.status().is_success() {
             Ok(())
         } else {
@@ -314,19 +349,19 @@ impl BaseHTTPClient {
     }
 
     const NO_TEXT: &'static str = "(Failed to extract error text from HTTP response)";
-    /// Builds [`ServiceError::BackendError`] from response.
+    /// Builds [`BaseHTTPClientError::BackendError`] from response.
     ///
     /// It contains also processing of response body, that is why it has to be async.
     ///
     /// Arguments:
     ///
     /// * `response`: response from which generate error
-    async fn build_backend_error(&self, response: Response) -> ServiceError {
+    async fn build_backend_error(&self, response: Response) -> BaseHTTPClientError {
         let code = response.status().as_u16();
         let text = response
             .text()
             .await
             .unwrap_or_else(|_| Self::NO_TEXT.to_string());
-        ServiceError::BackendError(code, text)
+        BaseHTTPClientError::BackendError(code, text)
     }
 }