From f3babaff51eb2383d9f35c18daf993aa740f2f2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= <igonzalezsosa@suse.com>
Date: Fri, 15 Mar 2024 11:01:02 +0000
Subject: [PATCH 1/5] Unify some error types that brought no value

---
 rust/agama-server/src/error.rs        | 24 ++++++++++++++
 rust/agama-server/src/manager/web.rs  | 27 +++-------------
 rust/agama-server/src/software/web.rs | 35 ++++----------------
 rust/agama-server/src/web/common.rs   | 46 ++-------------------------
 4 files changed, 39 insertions(+), 93 deletions(-)

diff --git a/rust/agama-server/src/error.rs b/rust/agama-server/src/error.rs
index 926feab063..c8b08c850c 100644
--- a/rust/agama-server/src/error.rs
+++ b/rust/agama-server/src/error.rs
@@ -1,3 +1,10 @@
+use agama_lib::error::ServiceError;
+use axum::{
+    http::StatusCode,
+    response::{IntoResponse, Response},
+    Json,
+};
+use serde_json::json;
 use zbus_macros::DBusError;
 
 #[derive(DBusError, Debug)]
@@ -25,3 +32,20 @@ impl From<Error> for zbus::fdo::Error {
         zbus::fdo::Error::Failed(format!("Localization error: {value}"))
     }
 }
+
+#[derive(Debug, thiserror::Error)]
+pub enum ApiError {
+    #[error("Agama service error: {0}")]
+    Service(#[from] ServiceError),
+    #[error("D-Bus error: {0}")]
+    DBus(#[from] zbus::Error),
+}
+
+impl IntoResponse for ApiError {
+    fn into_response(self) -> Response {
+        let body = json!({
+            "error": self.to_string()
+        });
+        (StatusCode::BAD_REQUEST, Json(body)).into_response()
+    }
+}
diff --git a/rust/agama-server/src/manager/web.rs b/rust/agama-server/src/manager/web.rs
index 8f6bc6b08c..02c60df493 100644
--- a/rust/agama-server/src/manager/web.rs
+++ b/rust/agama-server/src/manager/web.rs
@@ -14,18 +14,14 @@ use agama_lib::{
 };
 use axum::{
     extract::State,
-    http::StatusCode,
-    response::{IntoResponse, Response},
     routing::{get, post},
     Json, Router,
 };
 use serde::Serialize;
-use serde_json::json;
-use thiserror::Error;
 use tokio_stream::{Stream, StreamExt};
 
 use crate::{
-    error::Error,
+    error::{ApiError, Error},
     web::{
         common::{progress_router, service_status_router},
         Event,
@@ -37,19 +33,6 @@ pub struct ManagerState<'a> {
     manager: ManagerClient<'a>,
 }
 
-#[derive(Error, Debug)]
-pub enum ManagerError {
-    #[error("Manager service error: {0}")]
-    Error(#[from] ServiceError),
-}
-
-impl IntoResponse for ManagerError {
-    fn into_response(self) -> Response {
-        let body = json!({});
-        (StatusCode::BAD_REQUEST, Json(body)).into_response()
-    }
-}
-
 /// Holds information about the manager's status.
 #[derive(Clone, Serialize, utoipa::ToSchema)]
 pub struct InstallerStatus {
@@ -115,7 +98,7 @@ pub async fn manager_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
 #[utoipa::path(get, path = "/api/manager/probe", responses(
   (status = 200, description = "The probing process was started.")
 ))]
-async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), ManagerError> {
+async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
     state.manager.probe().await?;
     Ok(())
 }
@@ -124,7 +107,7 @@ async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), Manag
 #[utoipa::path(get, path = "/api/manager/install", responses(
   (status = 200, description = "The installation process was started.")
 ))]
-async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), ManagerError> {
+async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
     state.manager.install().await?;
     Ok(())
 }
@@ -133,7 +116,7 @@ async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), Man
 #[utoipa::path(get, path = "/api/manager/install", responses(
   (status = 200, description = "The installation tasks are executed.")
 ))]
-async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), ManagerError> {
+async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
     state.manager.finish().await?;
     Ok(())
 }
@@ -144,7 +127,7 @@ async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), Mana
 ))]
 async fn installer_status(
     State(state): State<ManagerState<'_>>,
-) -> Result<Json<InstallerStatus>, ManagerError> {
+) -> Result<Json<InstallerStatus>, ApiError> {
     let status = InstallerStatus {
         phase: state.manager.current_installation_phase().await?,
         busy: state.manager.busy_services().await?,
diff --git a/rust/agama-server/src/software/web.rs b/rust/agama-server/src/software/web.rs
index de8c599352..59d748beea 100644
--- a/rust/agama-server/src/software/web.rs
+++ b/rust/agama-server/src/software/web.rs
@@ -6,7 +6,7 @@
 //! * `software_stream` which offers an stream that emits the software events coming from D-Bus.
 
 use crate::{
-    error::Error,
+    error::{ApiError, Error},
     web::{
         common::{progress_router, service_status_router},
         Event,
@@ -22,15 +22,11 @@ use agama_lib::{
 };
 use axum::{
     extract::State,
-    http::StatusCode,
-    response::{IntoResponse, Response},
     routing::{get, post, put},
     Json, Router,
 };
 use serde::{Deserialize, Serialize};
-use serde_json::json;
 use std::{collections::HashMap, pin::Pin};
-use thiserror::Error;
 use tokio_stream::{Stream, StreamExt};
 
 #[derive(Clone)]
@@ -45,21 +41,6 @@ pub struct SoftwareConfig {
     product: Option<String>,
 }
 
-#[derive(Error, Debug)]
-pub enum SoftwareError {
-    #[error("Software service error: {0}")]
-    Error(#[from] ServiceError),
-}
-
-impl IntoResponse for SoftwareError {
-    fn into_response(self) -> Response {
-        let body = json!({
-            "error": self.to_string()
-        });
-        (StatusCode::BAD_REQUEST, Json(body)).into_response()
-    }
-}
-
 /// Returns an stream that emits software related events coming from D-Bus.
 ///
 /// It emits the Event::ProductChanged and Event::PatternsChanged events.
@@ -159,9 +140,7 @@ pub async fn software_service(dbus: zbus::Connection) -> Result<Router, ServiceE
     (status = 200, description = "List of known products", body = Vec<Product>),
     (status = 400, description = "The D-Bus service could not perform the action")
 ))]
-async fn products(
-    State(state): State<SoftwareState<'_>>,
-) -> Result<Json<Vec<Product>>, SoftwareError> {
+async fn products(State(state): State<SoftwareState<'_>>) -> Result<Json<Vec<Product>>, ApiError> {
     let products = state.product.products().await?;
     Ok(Json(products))
 }
@@ -185,7 +164,7 @@ pub struct PatternEntry {
 ))]
 async fn patterns(
     State(state): State<SoftwareState<'_>>,
-) -> Result<Json<Vec<PatternEntry>>, SoftwareError> {
+) -> Result<Json<Vec<PatternEntry>>, ApiError> {
     let patterns = state.software.patterns(true).await?;
     let selected = state.software.selected_patterns().await?;
     let items = patterns
@@ -216,7 +195,7 @@ async fn patterns(
 async fn set_config(
     State(state): State<SoftwareState<'_>>,
     Json(config): Json<SoftwareConfig>,
-) -> Result<(), SoftwareError> {
+) -> Result<(), ApiError> {
     if let Some(product) = config.product {
         state.product.select_product(&product).await?;
     }
@@ -237,7 +216,7 @@ async fn set_config(
 ))]
 async fn get_config(
     State(state): State<SoftwareState<'_>>,
-) -> Result<Json<SoftwareConfig>, SoftwareError> {
+) -> Result<Json<SoftwareConfig>, ApiError> {
     let product = state.product.product().await?;
     let product = if product.is_empty() {
         None
@@ -269,7 +248,7 @@ pub struct SoftwareProposal {
 ))]
 async fn proposal(
     State(state): State<SoftwareState<'_>>,
-) -> Result<Json<SoftwareProposal>, SoftwareError> {
+) -> Result<Json<SoftwareProposal>, ApiError> {
     let size = state.software.used_disk_space().await?;
     let proposal = SoftwareProposal { size };
     Ok(Json(proposal))
@@ -284,7 +263,7 @@ async fn proposal(
         (status = 400, description = "The D-Bus service could not perform the action
 ")
 ))]
-async fn probe(State(state): State<SoftwareState<'_>>) -> Result<Json<()>, SoftwareError> {
+async fn probe(State(state): State<SoftwareState<'_>>) -> Result<Json<()>, ApiError> {
     state.software.probe().await?;
     Ok(Json(()))
 }
diff --git a/rust/agama-server/src/web/common.rs b/rust/agama-server/src/web/common.rs
index 5c927bbe06..037e956dc4 100644
--- a/rust/agama-server/src/web/common.rs
+++ b/rust/agama-server/src/web/common.rs
@@ -7,21 +7,13 @@ use agama_lib::{
     progress::Progress,
     proxies::{ProgressProxy, ServiceStatusProxy},
 };
-use axum::{
-    extract::State,
-    http::StatusCode,
-    response::{IntoResponse, Response},
-    routing::get,
-    Json, Router,
-};
+use axum::{extract::State, routing::get, Json, Router};
 use pin_project::pin_project;
 use serde::Serialize;
-use serde_json::json;
-use thiserror::Error;
 use tokio_stream::{Stream, StreamExt};
 use zbus::PropertyStream;
 
-use crate::error::Error;
+use crate::error::{ApiError, Error};
 
 use super::Event;
 
@@ -83,21 +75,6 @@ struct ServiceStatus {
     current: u32,
 }
 
-#[derive(Error, Debug)]
-pub enum ServiceStatusError {
-    #[error("Service status error: {0}")]
-    Error(#[from] ServiceError),
-}
-
-impl IntoResponse for ServiceStatusError {
-    fn into_response(self) -> Response {
-        let body = json!({
-            "error": self.to_string()
-        });
-        (StatusCode::BAD_REQUEST, Json(body)).into_response()
-    }
-}
-
 /// Builds a stream of the changes in the the `org.opensuse.Agama1.ServiceStatus`
 /// interface of the given D-Bus object.
 ///
@@ -186,24 +163,7 @@ struct ProgressState<'a> {
     proxy: ProgressProxy<'a>,
 }
 
-#[derive(Error, Debug)]
-pub enum ProgressError {
-    #[error("Progress error: {0}")]
-    Error(#[from] ServiceError),
-    #[error("D-Bus error: {0}")]
-    DBusError(#[from] zbus::Error),
-}
-
-impl IntoResponse for ProgressError {
-    fn into_response(self) -> Response {
-        let body = json!({
-            "error": self.to_string()
-        });
-        (StatusCode::BAD_REQUEST, Json(body)).into_response()
-    }
-}
-
-async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<Progress>, ProgressError> {
+async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<Progress>, ApiError> {
     let proxy = state.proxy;
     let progress = Progress::from_proxy(&proxy).await?;
     Ok(Json(progress))

From f72393e2548aed6cb5761e1ed05b9f949b91b7aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= <igonzalezsosa@suse.com>
Date: Fri, 15 Mar 2024 12:05:29 +0000
Subject: [PATCH 2/5] More errors simplification

---
 rust/agama-server/src/error.rs             | 15 ++++++-----
 rust/agama-server/src/questions.rs         | 31 ++++++++++++----------
 rust/agama-server/src/questions/answers.rs | 10 +++----
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/rust/agama-server/src/error.rs b/rust/agama-server/src/error.rs
index c8b08c850c..d3037cd4e2 100644
--- a/rust/agama-server/src/error.rs
+++ b/rust/agama-server/src/error.rs
@@ -5,14 +5,17 @@ use axum::{
     Json,
 };
 use serde_json::json;
-use zbus_macros::DBusError;
 
-#[derive(DBusError, Debug)]
-#[dbus_error(prefix = "org.opensuse.Agama1.Locale")]
+use crate::questions::QuestionsError;
+
+#[derive(thiserror::Error, Debug)]
 pub enum Error {
-    #[dbus_error(zbus_error)]
-    ZBus(zbus::Error),
+    #[error("D-Bus error: {0}")]
+    DBus(#[from] zbus::Error),
+    #[error("Generic error: {0}")]
     Anyhow(String),
+    #[error("Answers handling error: {0}")]
+    Answers(QuestionsError),
 }
 
 // This would be nice, but using it for a return type
@@ -29,7 +32,7 @@ impl From<anyhow::Error> for Error {
 
 impl From<Error> for zbus::fdo::Error {
     fn from(value: Error) -> zbus::fdo::Error {
-        zbus::fdo::Error::Failed(format!("Localization error: {value}"))
+        zbus::fdo::Error::Failed(format!("D-Bus error: {value}"))
     }
 }
 
diff --git a/rust/agama-server/src/questions.rs b/rust/agama-server/src/questions.rs
index fda860bf6d..795029674c 100644
--- a/rust/agama-server/src/questions.rs
+++ b/rust/agama-server/src/questions.rs
@@ -1,12 +1,19 @@
 use std::collections::HashMap;
 
-use crate::error::Error;
 use agama_lib::questions::{self, GenericQuestion, WithPassword};
 use log;
 use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection};
 
 mod answers;
 
+#[derive(thiserror::Error, Debug)]
+pub enum QuestionsError {
+    #[error("Could not read the answers file: {0}")]
+    IO(std::io::Error),
+    #[error("Could not deserialize the answers file: {0}")]
+    Deserialize(serde_yaml::Error),
+}
+
 #[derive(Clone, Debug)]
 struct GenericQuestionObject(questions::GenericQuestion);
 
@@ -48,7 +55,7 @@ impl GenericQuestionObject {
     }
 
     #[dbus_interface(property)]
-    pub fn set_answer(&mut self, value: &str) -> Result<(), zbus::fdo::Error> {
+    pub fn set_answer(&mut self, value: &str) -> zbus::fdo::Result<()> {
         // TODO verify if answer exists in options or if it is valid in other way
         self.0.answer = value.to_string();
 
@@ -143,7 +150,7 @@ impl Questions {
         options: Vec<&str>,
         default_option: &str,
         data: HashMap<String, String>,
-    ) -> Result<ObjectPath, zbus::fdo::Error> {
+    ) -> zbus::fdo::Result<ObjectPath> {
         log::info!("Creating new question with text: {}.", text);
         let id = self.last_id;
         self.last_id += 1; // TODO use some thread safety
@@ -176,7 +183,7 @@ impl Questions {
         options: Vec<&str>,
         default_option: &str,
         data: HashMap<String, String>,
-    ) -> Result<ObjectPath, zbus::fdo::Error> {
+    ) -> zbus::fdo::Result<ObjectPath> {
         log::info!("Creating new question with password with text: {}.", text);
         let id = self.last_id;
         self.last_id += 1; // TODO use some thread safety
@@ -213,7 +220,7 @@ impl Questions {
     }
 
     /// Removes question at given object path
-    async fn delete(&mut self, question: ObjectPath<'_>) -> Result<(), Error> {
+    async fn delete(&mut self, question: ObjectPath<'_>) -> zbus::fdo::Result<()> {
         // TODO: error checking
         let id: u32 = question.rsplit('/').next().unwrap().parse().unwrap();
         let qtype = self.questions.get(&id).unwrap();
@@ -266,16 +273,12 @@ impl Questions {
         }
     }
 
-    fn add_answer_file(&mut self, path: String) -> Result<(), Error> {
+    fn add_answer_file(&mut self, path: String) -> zbus::fdo::Result<()> {
         log::info!("Adding answer file {}", path);
-        let answers = answers::Answers::new_from_file(path.as_str());
-        match answers {
-            Ok(answers) => {
-                self.answer_strategies.push(Box::new(answers));
-                Ok(())
-            }
-            Err(e) => Err(e.into()),
-        }
+        let answers = answers::Answers::new_from_file(path.as_str())
+            .map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
+        self.answer_strategies.push(Box::new(answers));
+        Ok(())
     }
 }
 
diff --git a/rust/agama-server/src/questions/answers.rs b/rust/agama-server/src/questions/answers.rs
index 60ade14fa0..c774950f94 100644
--- a/rust/agama-server/src/questions/answers.rs
+++ b/rust/agama-server/src/questions/answers.rs
@@ -1,9 +1,10 @@
 use std::collections::HashMap;
 
 use agama_lib::questions::GenericQuestion;
-use anyhow::Context;
 use serde::{Deserialize, Serialize};
 
+use super::QuestionsError;
+
 /// Data structure for single yaml answer. For variables specification see
 /// corresponding [agama_lib::questions::GenericQuestion] fields.
 /// The *matcher* part is: `class`, `text`, `data`.
@@ -60,10 +61,9 @@ pub struct Answers {
 }
 
 impl Answers {
-    pub fn new_from_file(path: &str) -> anyhow::Result<Self> {
-        let f = std::fs::File::open(path).context(format!("Failed to open {}", path))?;
-        let result: Self =
-            serde_yaml::from_reader(f).context(format!("Failed to parse values at {}", path))?;
+    pub fn new_from_file(path: &str) -> Result<Self, QuestionsError> {
+        let f = std::fs::File::open(path).map_err(QuestionsError::IO)?;
+        let result: Self = serde_yaml::from_reader(f).map_err(QuestionsError::Deserialize)?;
 
         Ok(result)
     }

From c4878b88be2046a13addd296afd6f8911fdda27c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= <igonzalezsosa@suse.com>
Date: Fri, 15 Mar 2024 12:18:30 +0000
Subject: [PATCH 3/5] Remove unneeded code from the localization D-Bus API

---
 rust/agama-server/src/l10n.rs | 83 +++++------------------------------
 1 file changed, 12 insertions(+), 71 deletions(-)

diff --git a/rust/agama-server/src/l10n.rs b/rust/agama-server/src/l10n.rs
index 10eff942fe..e130c01a46 100644
--- a/rust/agama-server/src/l10n.rs
+++ b/rust/agama-server/src/l10n.rs
@@ -6,7 +6,7 @@ pub mod web;
 
 use crate::error::Error;
 use agama_locale_data::{KeymapId, LocaleId};
-use anyhow::Context;
+
 use keyboard::KeymapsDatabase;
 use locale::LocalesDatabase;
 use regex::Regex;
@@ -32,31 +32,6 @@ pub struct Locale {
 
 #[dbus_interface(name = "org.opensuse.Agama1.Locale")]
 impl Locale {
-    /// Gets the supported locales information.
-    ///
-    /// Each element of the list has these parts:
-    ///
-    /// * The locale code (e.g., "es_ES.UTF-8").
-    /// * The name of the language according to the language defined by the
-    ///   UILocale property.
-    /// * The name of the territory according to the language defined by the
-    ///   UILocale property.
-    fn list_locales(&self) -> Result<Vec<(String, String, String)>, Error> {
-        let locales = self
-            .locales_db
-            .entries()
-            .iter()
-            .map(|l| {
-                (
-                    l.id.to_string(),
-                    l.language.to_string(),
-                    l.territory.to_string(),
-                )
-            })
-            .collect::<Vec<_>>();
-        Ok(locales)
-    }
-
     #[dbus_interface(property)]
     fn locales(&self) -> Vec<String> {
         self.locales.to_owned()
@@ -64,6 +39,11 @@ impl Locale {
 
     #[dbus_interface(property)]
     fn set_locales(&mut self, locales: Vec<String>) -> zbus::fdo::Result<()> {
+        if locales.is_empty() {
+            return Err(zbus::fdo::Error::Failed(format!(
+                "The locales list cannot be empty"
+            )));
+        }
         for loc in &locales {
             if !self.locales_db.exists(loc.as_str()) {
                 return Err(zbus::fdo::Error::Failed(format!(
@@ -89,22 +69,6 @@ impl Locale {
         Ok(self.translate(&locale)?)
     }
 
-    /// Returns a list of the supported keymaps.
-    ///
-    /// Each element of the list contains:
-    ///
-    /// * The keymap identifier (e.g., "es" or "es(ast)").
-    /// * The name of the keyboard in language set by the UILocale property.
-    fn list_keymaps(&self) -> Result<Vec<(String, String)>, Error> {
-        let keymaps = self
-            .keymaps_db
-            .entries()
-            .iter()
-            .map(|k| (k.id.to_string(), k.localized_description()))
-            .collect();
-        Ok(keymaps)
-    }
-
     #[dbus_interface(property)]
     fn keymap(&self) -> String {
         self.keymap.to_string()
@@ -125,32 +89,6 @@ impl Locale {
         Ok(())
     }
 
-    /// Returns a list of the supported timezones.
-    ///
-    /// Each element of the list contains:
-    ///
-    /// * The timezone identifier (e.g., "Europe/Berlin").
-    /// * A list containing each part of the name in the language set by the
-    ///   UILocale property.
-    /// * The name, in the language set by UILocale, of the main country
-    ///   associated to the timezone (typically, the name of the city that is
-    ///   part of the identifier) or empty string if there is no country.
-    fn list_timezones(&self) -> Result<Vec<(String, Vec<String>, String)>, Error> {
-        let timezones: Vec<_> = self
-            .timezones_db
-            .entries()
-            .iter()
-            .map(|tz| {
-                (
-                    tz.code.to_string(),
-                    tz.parts.clone(),
-                    tz.country.clone().unwrap_or_default(),
-                )
-            })
-            .collect();
-        Ok(timezones)
-    }
-
     #[dbus_interface(property)]
     fn timezone(&self) -> &str {
         self.timezone.as_str()
@@ -169,22 +107,25 @@ impl Locale {
     }
 
     // TODO: what should be returned value for commit?
-    fn commit(&mut self) -> Result<(), Error> {
+    fn commit(&mut self) -> zbus::fdo::Result<()> {
         const ROOT: &str = "/mnt";
+
         Command::new("/usr/bin/systemd-firstboot")
             .args([
                 "--root",
                 ROOT,
                 "--force",
                 "--locale",
-                self.locales.first().context("missing locale")?.as_str(),
+                self.locales.first().unwrap_or(&"en_US.UTF-8".to_string()),
                 "--keymap",
                 &self.keymap.to_string(),
                 "--timezone",
                 &self.timezone,
             ])
             .status()
-            .context("Failed to execute systemd-firstboot")?;
+            .map_err(|e| {
+                zbus::fdo::Error::Failed(format!("Could not apply the l10n configuration: {e}"))
+            })?;
 
         Ok(())
     }

From f2848d084f6a56722e964002efd051bee613686e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= <igonzalezsosa@suse.com>
Date: Fri, 15 Mar 2024 12:22:50 +0000
Subject: [PATCH 4/5] Unify ApiError and Error

---
 rust/agama-server/src/error.rs        | 12 +++---------
 rust/agama-server/src/manager/web.rs  | 10 +++++-----
 rust/agama-server/src/software/web.rs | 18 +++++++-----------
 rust/agama-server/src/web/common.rs   |  4 ++--
 4 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/rust/agama-server/src/error.rs b/rust/agama-server/src/error.rs
index d3037cd4e2..7f3665b24b 100644
--- a/rust/agama-server/src/error.rs
+++ b/rust/agama-server/src/error.rs
@@ -14,6 +14,8 @@ pub enum Error {
     DBus(#[from] zbus::Error),
     #[error("Generic error: {0}")]
     Anyhow(String),
+    #[error("Agama service error: {0}")]
+    Service(#[from] ServiceError),
     #[error("Answers handling error: {0}")]
     Answers(QuestionsError),
 }
@@ -36,15 +38,7 @@ impl From<Error> for zbus::fdo::Error {
     }
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum ApiError {
-    #[error("Agama service error: {0}")]
-    Service(#[from] ServiceError),
-    #[error("D-Bus error: {0}")]
-    DBus(#[from] zbus::Error),
-}
-
-impl IntoResponse for ApiError {
+impl IntoResponse for Error {
     fn into_response(self) -> Response {
         let body = json!({
             "error": self.to_string()
diff --git a/rust/agama-server/src/manager/web.rs b/rust/agama-server/src/manager/web.rs
index 02c60df493..ce3ebe64d5 100644
--- a/rust/agama-server/src/manager/web.rs
+++ b/rust/agama-server/src/manager/web.rs
@@ -21,7 +21,7 @@ use serde::Serialize;
 use tokio_stream::{Stream, StreamExt};
 
 use crate::{
-    error::{ApiError, Error},
+    error::Error,
     web::{
         common::{progress_router, service_status_router},
         Event,
@@ -98,7 +98,7 @@ pub async fn manager_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
 #[utoipa::path(get, path = "/api/manager/probe", responses(
   (status = 200, description = "The probing process was started.")
 ))]
-async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
+async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), Error> {
     state.manager.probe().await?;
     Ok(())
 }
@@ -107,7 +107,7 @@ async fn probe_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiEr
 #[utoipa::path(get, path = "/api/manager/install", responses(
   (status = 200, description = "The installation process was started.")
 ))]
-async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
+async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), Error> {
     state.manager.install().await?;
     Ok(())
 }
@@ -116,7 +116,7 @@ async fn install_action(State(state): State<ManagerState<'_>>) -> Result<(), Api
 #[utoipa::path(get, path = "/api/manager/install", responses(
   (status = 200, description = "The installation tasks are executed.")
 ))]
-async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiError> {
+async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), Error> {
     state.manager.finish().await?;
     Ok(())
 }
@@ -127,7 +127,7 @@ async fn finish_action(State(state): State<ManagerState<'_>>) -> Result<(), ApiE
 ))]
 async fn installer_status(
     State(state): State<ManagerState<'_>>,
-) -> Result<Json<InstallerStatus>, ApiError> {
+) -> Result<Json<InstallerStatus>, Error> {
     let status = InstallerStatus {
         phase: state.manager.current_installation_phase().await?,
         busy: state.manager.busy_services().await?,
diff --git a/rust/agama-server/src/software/web.rs b/rust/agama-server/src/software/web.rs
index 59d748beea..de7e907501 100644
--- a/rust/agama-server/src/software/web.rs
+++ b/rust/agama-server/src/software/web.rs
@@ -6,7 +6,7 @@
 //! * `software_stream` which offers an stream that emits the software events coming from D-Bus.
 
 use crate::{
-    error::{ApiError, Error},
+    error::Error,
     web::{
         common::{progress_router, service_status_router},
         Event,
@@ -140,7 +140,7 @@ pub async fn software_service(dbus: zbus::Connection) -> Result<Router, ServiceE
     (status = 200, description = "List of known products", body = Vec<Product>),
     (status = 400, description = "The D-Bus service could not perform the action")
 ))]
-async fn products(State(state): State<SoftwareState<'_>>) -> Result<Json<Vec<Product>>, ApiError> {
+async fn products(State(state): State<SoftwareState<'_>>) -> Result<Json<Vec<Product>>, Error> {
     let products = state.product.products().await?;
     Ok(Json(products))
 }
@@ -164,7 +164,7 @@ pub struct PatternEntry {
 ))]
 async fn patterns(
     State(state): State<SoftwareState<'_>>,
-) -> Result<Json<Vec<PatternEntry>>, ApiError> {
+) -> Result<Json<Vec<PatternEntry>>, Error> {
     let patterns = state.software.patterns(true).await?;
     let selected = state.software.selected_patterns().await?;
     let items = patterns
@@ -195,7 +195,7 @@ async fn patterns(
 async fn set_config(
     State(state): State<SoftwareState<'_>>,
     Json(config): Json<SoftwareConfig>,
-) -> Result<(), ApiError> {
+) -> Result<(), Error> {
     if let Some(product) = config.product {
         state.product.select_product(&product).await?;
     }
@@ -214,9 +214,7 @@ async fn set_config(
     (status = 200, description = "Software configuration", body = SoftwareConfig),
     (status = 400, description = "The D-Bus service could not perform the action")
 ))]
-async fn get_config(
-    State(state): State<SoftwareState<'_>>,
-) -> Result<Json<SoftwareConfig>, ApiError> {
+async fn get_config(State(state): State<SoftwareState<'_>>) -> Result<Json<SoftwareConfig>, Error> {
     let product = state.product.product().await?;
     let product = if product.is_empty() {
         None
@@ -246,9 +244,7 @@ pub struct SoftwareProposal {
     get, path = "/software/proposal", responses(
         (status = 200, description = "Software proposal", body = SoftwareProposal)
 ))]
-async fn proposal(
-    State(state): State<SoftwareState<'_>>,
-) -> Result<Json<SoftwareProposal>, ApiError> {
+async fn proposal(State(state): State<SoftwareState<'_>>) -> Result<Json<SoftwareProposal>, Error> {
     let size = state.software.used_disk_space().await?;
     let proposal = SoftwareProposal { size };
     Ok(Json(proposal))
@@ -263,7 +259,7 @@ async fn proposal(
         (status = 400, description = "The D-Bus service could not perform the action
 ")
 ))]
-async fn probe(State(state): State<SoftwareState<'_>>) -> Result<Json<()>, ApiError> {
+async fn probe(State(state): State<SoftwareState<'_>>) -> Result<Json<()>, Error> {
     state.software.probe().await?;
     Ok(Json(()))
 }
diff --git a/rust/agama-server/src/web/common.rs b/rust/agama-server/src/web/common.rs
index 037e956dc4..8739d6795b 100644
--- a/rust/agama-server/src/web/common.rs
+++ b/rust/agama-server/src/web/common.rs
@@ -13,7 +13,7 @@ use serde::Serialize;
 use tokio_stream::{Stream, StreamExt};
 use zbus::PropertyStream;
 
-use crate::error::{ApiError, Error};
+use crate::error::Error;
 
 use super::Event;
 
@@ -163,7 +163,7 @@ struct ProgressState<'a> {
     proxy: ProgressProxy<'a>,
 }
 
-async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<Progress>, ApiError> {
+async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<Progress>, Error> {
     let proxy = state.proxy;
     let progress = Progress::from_proxy(&proxy).await?;
     Ok(Json(progress))

From 80ac668d5835b1e0aff2974c31958061a9e8ee43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= <igonzalezsosa@suse.com>
Date: Fri, 15 Mar 2024 12:44:52 +0000
Subject: [PATCH 5/5] Convert LocaleError into Error

---
 rust/agama-server/src/error.rs    |  8 ++++---
 rust/agama-server/src/l10n/web.rs | 38 +++++++++++--------------------
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/rust/agama-server/src/error.rs b/rust/agama-server/src/error.rs
index 7f3665b24b..1c9b7a7847 100644
--- a/rust/agama-server/src/error.rs
+++ b/rust/agama-server/src/error.rs
@@ -6,7 +6,7 @@ use axum::{
 };
 use serde_json::json;
 
-use crate::questions::QuestionsError;
+use crate::{l10n::web::LocaleError, questions::QuestionsError};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -16,8 +16,10 @@ pub enum Error {
     Anyhow(String),
     #[error("Agama service error: {0}")]
     Service(#[from] ServiceError),
-    #[error("Answers handling error: {0}")]
-    Answers(QuestionsError),
+    #[error("Questions service error: {0}")]
+    Questions(QuestionsError),
+    #[error("Software service error: {0}")]
+    Locale(#[from] LocaleError),
 }
 
 // This would be nice, but using it for a return type
diff --git a/rust/agama-server/src/l10n/web.rs b/rust/agama-server/src/l10n/web.rs
index 09bd9d9b10..5cb2a7f777 100644
--- a/rust/agama-server/src/l10n/web.rs
+++ b/rust/agama-server/src/l10n/web.rs
@@ -9,20 +9,16 @@ use crate::{
 use agama_locale_data::{InvalidKeymap, LocaleId};
 use axum::{
     extract::State,
-    http::StatusCode,
-    response::{IntoResponse, Response},
     routing::{get, put},
     Json, Router,
 };
 use serde::{Deserialize, Serialize};
-use serde_json::json;
 use std::{
     process::Command,
     sync::{Arc, RwLock},
 };
-use thiserror::Error;
 
-#[derive(Error, Debug)]
+#[derive(thiserror::Error, Debug)]
 pub enum LocaleError {
     #[error("Unknown locale code: {0}")]
     UnknownLocale(String),
@@ -30,19 +26,8 @@ pub enum LocaleError {
     UnknownTimezone(String),
     #[error("Invalid keymap: {0}")]
     InvalidKeymap(#[from] InvalidKeymap),
-    #[error("Cannot translate: {0}")]
-    OtherError(#[from] Error),
-    #[error("Cannot change the local keymap: {0}")]
-    CouldNotSetKeymap(#[from] std::io::Error),
-}
-
-impl IntoResponse for LocaleError {
-    fn into_response(self) -> Response {
-        let body = json!({
-            "error": self.to_string()
-        });
-        (StatusCode::BAD_REQUEST, Json(body)).into_response()
-    }
+    #[error("Could not apply the changes")]
+    Commit(#[from] std::io::Error),
 }
 
 #[derive(Clone)]
@@ -119,14 +104,14 @@ async fn keymaps(State(state): State<LocaleState>) -> Json<Vec<Keymap>> {
 async fn set_config(
     State(state): State<LocaleState>,
     Json(value): Json<LocaleConfig>,
-) -> Result<Json<()>, LocaleError> {
+) -> Result<Json<()>, Error> {
     let mut data = state.locale.write().unwrap();
     let mut changes = LocaleConfig::default();
 
     if let Some(locales) = &value.locales {
         for loc in locales {
             if !data.locales_db.exists(loc.as_str()) {
-                return Err(LocaleError::UnknownLocale(loc.to_string()));
+                return Err(LocaleError::UnknownLocale(loc.to_string()))?;
             }
         }
         data.locales = locales.clone();
@@ -135,14 +120,14 @@ async fn set_config(
 
     if let Some(timezone) = &value.timezone {
         if !data.timezones_db.exists(timezone) {
-            return Err(LocaleError::UnknownTimezone(timezone.to_string()));
+            return Err(LocaleError::UnknownTimezone(timezone.to_string()))?;
         }
         data.timezone = timezone.to_owned();
         changes.timezone = Some(data.timezone.clone());
     }
 
     if let Some(keymap_id) = &value.keymap {
-        data.keymap = keymap_id.parse()?;
+        data.keymap = keymap_id.parse().map_err(LocaleError::InvalidKeymap)?;
         changes.keymap = Some(keymap_id.clone());
     }
 
@@ -161,14 +146,17 @@ async fn set_config(
     }
 
     if let Some(ui_keymap) = &value.ui_keymap {
-        data.ui_keymap = ui_keymap.parse()?;
+        // data.ui_keymap = ui_keymap.parse().into::<Result<KeymapId, LocaleError>>()?;
+        data.ui_keymap = ui_keymap.parse().map_err(LocaleError::InvalidKeymap)?;
         Command::new("/usr/bin/localectl")
             .args(["set-x11-keymap", &ui_keymap])
-            .output()?;
+            .output()
+            .map_err(LocaleError::Commit)?;
         Command::new("/usr/bin/setxkbmap")
             .arg(&ui_keymap)
             .env("DISPLAY", ":0")
-            .output()?;
+            .output()
+            .map_err(LocaleError::Commit)?;
     }
 
     _ = state.events.send(Event::L10nConfigChanged(changes));