refactor: improve error handling (#453)

Removed many unwrap calls by propagating the error to the caller This is the first external code contribution! Big up to @nozwock who is learning Rust ! :) Co-authored-by: w1nst0n <[email protected]>
Backup-Gits · Dec 7, 2022 · fbb374d · fbb374d
1 parent 10924c7
commit fbb374d
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 34 deletions.
diff --git a/src/core/save.rs b/src/core/save.rs
@@ -113,7 +113,15 @@ pub fn restore_backup(
     packages: &[Vec<PackageRow>],
     settings: &DeviceSettings,
 ) -> Result<Vec<BackupPackage>, String> {
-    match fs::read_to_string(settings.backup.selected.as_ref().unwrap().path.clone()) {
+    match fs::read_to_string(
+        settings
+            .backup
+            .selected
+            .as_ref()
+            .ok_or("field should be Some type")?
+            .path
+            .clone(),
+    ) {
         Ok(data) => {
             let phone_backup: PhoneBackup =
                 serde_json::from_str(&data).expect("Unable to parse backup file");
@@ -143,7 +151,10 @@ pub fn restore_backup(
                     let p_commands = apply_pkg_state_commands(
                         &package,
                         &backup_package.state,
-                        &settings.backup.selected_user.unwrap(),
+                        &settings
+                            .backup
+                            .selected_user
+                            .ok_or("field should be Some type")?,
                         selected_device,
                     );
                     if !p_commands.is_empty() {

diff --git a/src/core/sync.rs b/src/core/sync.rs
@@ -78,14 +78,23 @@ pub fn adb_shell_command(shell: bool, args: &str) -> Result<String, String> {
         }
         Ok(o) => {
             if !o.status.success() {
-                let stdout = String::from_utf8(o.stdout).unwrap().trim_end().to_string();
-                let stderr = String::from_utf8(o.stderr).unwrap().trim_end().to_string();
+                let stdout = String::from_utf8(o.stdout)
+                    .map_err(|e| e.to_string())?
+                    .trim_end()
+                    .to_string();
+                let stderr = String::from_utf8(o.stderr)
+                    .map_err(|e| e.to_string())?
+                    .trim_end()
+                    .to_string();
 
                 // ADB does really weird things. Some errors are not redirected to stderr
                 let err = if stdout.is_empty() { stderr } else { stdout };
                 Err(err)
             } else {
-                Ok(String::from_utf8(o.stdout).unwrap().trim_end().to_string())
+                Ok(String::from_utf8(o.stdout)
+                    .map_err(|e| e.to_string())?
+                    .trim_end()
+                    .to_string())
             }
         }
     }

diff --git a/src/core/uad_lists.rs b/src/core/uad_lists.rs
@@ -210,7 +210,7 @@ pub fn load_debloat_lists(remote: bool) -> (Result<PackageHashMap, PackageHashMa
             .call()
             {
                 Ok(data) => {
-                    let text = data.into_string().unwrap();
+                    let text = data.into_string().expect("response should be Ok type");
                     fs::write(cached_uad_lists.clone(), &text).expect("Unable to write file");
                     let list = serde_json::from_str(&text).expect("Unable to parse");
                     OperationResult::Ok(list)

diff --git a/src/core/update.rs b/src/core/update.rs
@@ -63,7 +63,7 @@ pub async fn download_file<T: ToString>(url: T, dest_file: PathBuf) -> Result<()
 
     match ureq::get(&url).call() {
         Ok(res) => {
-            let mut file = fs::File::create(dest_file).unwrap();
+            let mut file = fs::File::create(dest_file).map_err(|e| e.to_string())?;
 
             if let Err(e) = copy(&mut res.into_reader(), &mut file) {
                 return Err(e.to_string());
@@ -82,20 +82,20 @@ pub async fn download_update_to_temp_file(
     bin_name: String,
     release: Release,
 ) -> Result<(PathBuf, PathBuf), ()> {
-    let current_bin_path = std::env::current_exe().unwrap();
+    let current_bin_path = std::env::current_exe().map_err(|_| ())?;
 
     // Path to download the new version to
     let download_path = current_bin_path
         .parent()
-        .unwrap()
+        .ok_or(())?
         .join(format!("tmp_{bin_name}"));
 
     // Path to temporarily force rename current process to, se we can then
     // rename `download_path` to `current_bin_path` and then launch new version
     // cleanly as `current_bin_path`
     let tmp_path = current_bin_path
         .parent()
-        .unwrap()
+        .ok_or(())?
         .join(format!("tmp2_{bin_name}"));
 
     // MacOS and Linux release are gziped tarball
@@ -108,20 +108,21 @@ pub async fn download_update_to_temp_file(
             .iter()
             .find(|a| a.name == asset_name)
             .cloned()
-            .unwrap();
+            .ok_or(())?;
 
-        let archive_path = current_bin_path.parent().unwrap().join(&asset_name);
+        let archive_path = current_bin_path.parent().ok_or(())?.join(&asset_name);
 
         if let Err(e) = download_file(asset.download_url, archive_path.clone()).await {
             error!("Couldn't download UAD update: {}", e);
             return Err(());
         }
+
         if extract_binary_from_tar(&archive_path, &download_path).is_err() {
             error!("Couldn't extract UAD release tarball");
             return Err(());
         }
 
-        std::fs::remove_file(&archive_path).unwrap();
+        std::fs::remove_file(&archive_path).map_err(|_| ())?;
     }
 
     // For Windows we download the new binary directly
@@ -132,7 +133,7 @@ pub async fn download_update_to_temp_file(
             .iter()
             .find(|a| a.name == bin_name)
             .cloned()
-            .unwrap();
+            .ok_or(())?;
 
         if let Err(e) = download_file(asset.download_url, download_path.clone()).await {
             error!("Couldn't download UAD update: {}", e);
@@ -145,7 +146,7 @@ pub async fn download_update_to_temp_file(
     {
         use std::os::unix::fs::PermissionsExt;
 
-        let mut permissions = fs::metadata(&download_path).unwrap().permissions();
+        let mut permissions = fs::metadata(&download_path).map_err(|_| ())?.permissions();
         permissions.set_mode(0o755);
         if let Err(e) = fs::set_permissions(&download_path, permissions) {
             error!("[SelfUpdate] Couldn't set permission to temp file: {}", e);
@@ -181,9 +182,14 @@ pub fn get_latest_release() -> Result<Option<Release>, ()> {
         .call()
     {
         Ok(res) => {
-            let release: Release =
-                serde_json::from_value(res.into_json::<serde_json::Value>().unwrap()[0].clone())
-                    .unwrap();
+            let release: Release = serde_json::from_value(
+                res.into_json::<serde_json::Value>()
+                    .map_err(|_| ())?
+                    .get(0)
+                    .ok_or(())?
+                    .clone(),
+            )
+            .map_err(|_| ())?;
             if release.tag_name.as_str() != "dev-build"
                 && release.tag_name.as_str() > env!("CARGO_PKG_VERSION")
             {
@@ -202,25 +208,24 @@ pub fn get_latest_release() -> Result<Option<Release>, ()> {
 /// Extracts the binary from a `tar.gz` archive to temp_file path
 #[cfg(feature = "self-update")]
 #[cfg(not(target_os = "windows"))]
-pub fn extract_binary_from_tar(archive_path: &Path, temp_file: &Path) -> Result<(), ()> {
+pub fn extract_binary_from_tar(archive_path: &Path, temp_file: &Path) -> io::Result<()> {
     use flate2::read::GzDecoder;
     use std::fs::File;
     use tar::Archive;
-    let mut archive = Archive::new(GzDecoder::new(File::open(archive_path).unwrap()));
+    let mut archive = Archive::new(GzDecoder::new(File::open(archive_path)?));
 
-    let mut temp_file = File::create(temp_file).unwrap();
+    let mut temp_file = File::create(temp_file)?;
 
-    for file in archive.entries().unwrap() {
-        let mut file = file.unwrap();
+    for file in archive.entries()? {
+        let mut file = file?;
 
-        let path = file.path().unwrap();
+        let path = file.path()?;
         if path.to_str().is_some() {
-            io::copy(&mut file, &mut temp_file).unwrap();
+            io::copy(&mut file, &mut temp_file)?;
             return Ok(());
         }
     }
-
-    Err(())
+    Err(io::ErrorKind::NotFound.into())
 }
 
 /// Hardcoded binary names for each compilation target

diff --git a/src/gui/mod.rs b/src/gui/mod.rs
@@ -314,17 +314,16 @@ impl Application for UadGui {
 }
 
 impl UadGui {
-    pub fn start() {
-        let settings: Settings<()> = Settings {
+    pub fn start() -> iced::Result {
+        Self::run(Settings {
             window: Window {
                 size: (1050, 800),
                 resizable: true,
                 decorations: true,
                 ..iced::window::Settings::default()
             },
             default_text_size: 17,
-            ..iced::Settings::default()
-        };
-        Self::run(settings).unwrap_err();
+            ..Settings::default()
+        })
     }
 }
diff --git a/src/main.rs b/src/main.rs
@@ -22,9 +22,9 @@ static CONFIG_DIR: PathBuf = setup_uad_dir(dirs::config_dir());
 #[dynamic]
 static CACHE_DIR: PathBuf = setup_uad_dir(dirs::cache_dir());
 
-fn main() {
+fn main() -> iced::Result {
     setup_logger().expect("setup logging");
-    gui::UadGui::start();
+    gui::UadGui::start()
 }
 
 pub fn setup_logger() -> Result<(), fern::InitError> {