fix: improve log output again

X9VoiD · X9VoiD · commit 9287eb6a4279 · 2023-03-11T22:33:01.000+08:00
Also cleanup some error handling
diff --git a/src/cleanup_modules/device_cleanup.rs b/src/cleanup_modules/device_cleanup.rs
@@ -1,24 +1,21 @@
 use async_trait::async_trait;
-use error_stack::{report, IntoReport, Result, ResultExt};
+use error_stack::{IntoReport, Result, ResultExt};
 use serde::Deserialize;
 use uuid::Uuid;
 use windows::{
     core::HSTRING,
-    Win32::{
-        Devices::DeviceAndDriverInstallation::*,
-        Foundation::{GetLastError, BOOL},
-    },
+    Win32::{Devices::DeviceAndDriverInstallation::*, Foundation::BOOL},
 };
 
 use super::{
-    get_path_to_dump, Dumper, IntoModuleReport, ModuleError, ModuleMetadata, ModuleRunInfo,
-    ModuleStrategy, ToUninstall, UninstallError,
+    get_path_to_dump, Dumper, IntoModuleReport, IntoUninstallReport, ModuleError, ModuleMetadata,
+    ModuleRunInfo, ModuleStrategy, ToUninstall, UninstallError,
 };
 use crate::{
     cleanup_modules::create_dump_file,
     services::{
         self, identifiers, regex_cache,
-        windows::{enumerate_devices, inf_regex, Device, HResultExt},
+        windows::{enumerate_devices, inf_regex, Device},
     },
     State,
 };
@@ -85,15 +82,15 @@ impl ModuleStrategy for DeviceCleanupModule {
     async fn uninstall_object(
         &self,
         object: Self::Object,
-        _to_uninstall: &Self::ToUninstall,
+        to_uninstall: &Self::ToUninstall,
         _state: &State,
         run_info: &mut ModuleRunInfo,
     ) -> Result<(), UninstallError> {
         unsafe {
             let device_info_set = SetupDiCreateDeviceInfoList(None, None)
                 .into_report()
-                .change_context(UninstallError::UninstallFailed)
-                .attach_printable_lazy(|| "failed to create a device list")?;
+                .attach_printable_lazy(|| "failed to create a device list")
+                .into_uninstall_report(to_uninstall)?;
             let mut device_info_data = SP_DEVINFO_DATA {
                 cbSize: std::mem::size_of::<SP_DEVINFO_DATA>() as u32,
                 ..SP_DEVINFO_DATA::default()
@@ -108,12 +105,13 @@ impl ModuleStrategy for DeviceCleanupModule {
             )
             .as_bool()
             {
-                let error = GetLastError();
-                return Err(report!(UninstallError::UninstallFailed))
+                let error = windows::core::Error::from_win32();
+                return Err(error)
+                    .into_report()
                     .attach_printable_lazy(|| {
                         format!("failed to open device info of {}", object.instance_id())
                     })
-                    .attach_win32_error(error);
+                    .into_uninstall_report(to_uninstall);
             }
 
             let mut reboot: BOOL = false.into();
@@ -126,12 +124,13 @@ impl ModuleStrategy for DeviceCleanupModule {
             )
             .as_bool()
             {
-                let error = GetLastError();
-                return Err(report!(UninstallError::UninstallFailed))
+                let error = windows::core::Error::from_win32();
+                return Err(error)
+                    .into_report()
                     .attach_printable_lazy(|| {
                         format!("failed to uninstall device {}", object.instance_id())
                     })
-                    .attach_win32_error(error);
+                    .into_uninstall_report(to_uninstall);
             }
 
             if reboot.as_bool() {
diff --git a/src/cleanup_modules/driver_cleanup.rs b/src/cleanup_modules/driver_cleanup.rs
@@ -1,26 +1,23 @@
 use std::path::Path;
 
 use async_trait::async_trait;
-use error_stack::{report, IntoReport, Result, ResultExt};
+use error_stack::{IntoReport, Result, ResultExt};
 use serde::Deserialize;
 use uuid::Uuid;
 use windows::{
     core::HSTRING,
-    Win32::{
-        Devices::DeviceAndDriverInstallation::DiUninstallDriverW,
-        Foundation::{GetLastError, BOOL},
-    },
+    Win32::{Devices::DeviceAndDriverInstallation::DiUninstallDriverW, Foundation::BOOL},
 };
 
 use super::{
-    Dumper, IntoModuleReport, ModuleError, ModuleMetadata, ModuleRunInfo, ModuleStrategy,
-    ToUninstall, UninstallError,
+    Dumper, IntoModuleReport, IntoUninstallReport, ModuleError, ModuleMetadata, ModuleRunInfo,
+    ModuleStrategy, ToUninstall, UninstallError,
 };
 use crate::{
     cleanup_modules::{create_dump_file, get_path_to_dump},
     services::{
         self, identifiers, regex_cache,
-        windows::{enumerate_drivers, Driver, HResultExt},
+        windows::{enumerate_drivers, Driver},
     },
     State,
 };
@@ -87,7 +84,7 @@ impl ModuleStrategy for DriverCleanupModule {
     async fn uninstall_object(
         &self,
         object: Self::Object,
-        _to_uninstall: &Self::ToUninstall,
+        to_uninstall: &Self::ToUninstall,
         _state: &State,
         run_info: &mut ModuleRunInfo,
     ) -> Result<(), UninstallError> {
@@ -104,12 +101,13 @@ impl ModuleStrategy for DriverCleanupModule {
             )
             .as_bool()
             {
-                let err = GetLastError();
-                return Err(report!(UninstallError::UninstallFailed))
+                let err = windows::core::Error::from_win32();
+                return Err(err)
+                    .into_report()
                     .attach_printable_lazy(|| {
                         format!("failed to uninstall inf: {}", inf_path.display())
                     })
-                    .attach_win32_error(err);
+                    .into_uninstall_report(to_uninstall);
             }
 
             if reboot.as_bool() {
diff --git a/src/cleanup_modules/driver_package_cleanup.rs b/src/cleanup_modules/driver_package_cleanup.rs
@@ -16,8 +16,8 @@ use winreg::{enums::HKEY_LOCAL_MACHINE, RegKey};
 use wmi::{COMLibrary, WMIConnection, WMIError};
 
 use super::{
-    create_dump_file, Dumper, IntoModuleReport, ModuleError, ModuleMetadata, ModuleRunInfo,
-    ModuleStrategy, ToUninstall, UninstallError,
+    create_dump_file, Dumper, IntoModuleReport, IntoUninstallReport, ModuleError, ModuleMetadata,
+    ModuleRunInfo, ModuleStrategy, ToUninstall, UninstallError,
 };
 use crate::{
     cleanup_modules::get_path_to_dump,
@@ -99,21 +99,18 @@ impl ModuleStrategy for DriverPackageCleanupModule {
         use UninstallMethod::*;
 
         match &to_uninstall.uninstall_method {
-            Normal => {
-                run_uninstall_method(uninstall_normal, state, &object, to_uninstall).await
-            },
+            Normal => run_uninstall_method(uninstall_normal, state, &object, to_uninstall).await,
             Deferred => {
                 run_uninstall_method(uninstall_deferred, state, &object, to_uninstall).await
             }
             RegistryOnly => {
-                uninstall_registry_only(object, to_uninstall)
-                    .attach_printable_lazy(|| {
-                        format!(
-                            "failed to open uninstall key for driver package '{}'",
-                            to_uninstall.friendly_name
-                        )
-                    })
-            },
+                uninstall_registry_only(object, to_uninstall).attach_printable_lazy(|| {
+                    format!(
+                        "failed to open uninstall key for driver package '{}'",
+                        to_uninstall.friendly_name
+                    )
+                })
+            }
         }
     }
 
@@ -124,7 +121,7 @@ impl ModuleStrategy for DriverPackageCleanupModule {
 
 fn uninstall_registry_only(
     object: DriverPackage,
-    _to_uninstall: &DriverPackageToUninstall,
+    to_uninstall: &DriverPackageToUninstall,
 ) -> Result<(), UninstallError> {
     let key_path = Path::new(object.key_name());
     let key_parent = key_path.parent().unwrap();
@@ -135,13 +132,13 @@ fn uninstall_registry_only(
         .open_subkey_with_flags(key_parent, flags)
         .into_report()
         .attach_printable_lazy(|| key_parent.to_string_lossy().to_string())
-        .change_context(UninstallError::UninstallFailed)?;
+        .into_uninstall_report(to_uninstall)?;
 
     uninstall_key
         .delete_subkey_all(key_name)
         .into_report()
         .attach_printable_lazy(|| key_path.to_string_lossy().to_string())
-        .change_context(UninstallError::UninstallFailed)
+        .into_uninstall_report(to_uninstall)
 }
 
 #[derive(Default)]
@@ -226,40 +223,40 @@ where
 async fn uninstall_normal(
     _state: &State,
     object: &DriverPackage,
-    _to_uninstall: &DriverPackageToUninstall,
+    to_uninstall: &DriverPackageToUninstall,
     _ct: CancellationToken,
 ) -> Result<(), UninstallError> {
     let uninstall_string = object.uninstall_string().unwrap();
     let child_process = match to_command(uninstall_string).spawn() {
         Ok(child) => child,
         Err(err) => match err.kind() {
-            ErrorKind::NotFound => bail!(UninstallError::AlreadyUninstalled),
+            ErrorKind::NotFound => bail!(UninstallError::uninstalled(to_uninstall)),
             _ => {
                 return Err(err)
                     .into_report()
-                    .change_context(UninstallError::UninstallFailed)
                     .attach_printable_lazy(|| {
                         format!("failed to launch uninstaller: {}", uninstall_string)
                     })
+                    .into_uninstall_report(to_uninstall)
             }
         },
     };
 
     wait_for_process_async(child_process)
         .await
         .into_report()
-        .change_context(UninstallError::UninstallFailed)
         .attach_printable_lazy(|| {
             format!("failed to wait on child process, exe: {}", uninstall_string)
-        })?;
+        })
+        .into_uninstall_report(to_uninstall)?;
 
     Ok(())
 }
 
 async fn uninstall_deferred(
     _state: &State,
     object: &DriverPackage,
-    _to_uninstall: &DriverPackageToUninstall,
+    to_uninstall: &DriverPackageToUninstall,
     _ct: CancellationToken,
 ) -> Result<(), UninstallError> {
     let uninstall_string = object.uninstall_string().unwrap();
@@ -274,14 +271,14 @@ async fn uninstall_deferred(
     let child = match command.spawn() {
         Ok(child) => child,
         Err(err) => match err.kind() {
-            ErrorKind::NotFound => bail!(UninstallError::AlreadyUninstalled),
+            ErrorKind::NotFound => bail!(UninstallError::uninstalled(to_uninstall)),
             _ => {
                 return Err(err)
                     .into_report()
-                    .change_context(UninstallError::UninstallFailed)
                     .attach_printable_lazy(|| {
                         format!("failed to launch uninstaller: {}", uninstall_string)
                     })
+                    .into_uninstall_report(to_uninstall)
             }
         },
     };
@@ -314,22 +311,22 @@ async fn uninstall_deferred(
             (Err(err), _) => {
                 return Err(err)
                     .into_report()
-                    .change_context(UninstallError::UninstallFailed)
                     .attach_printable("failed to wait for main uninstaller process")
+                    .into_uninstall_report(to_uninstall)
             }
             (_, Err(err)) => {
                 return Err(err)
-                    .change_context(UninstallError::UninstallFailed)
                     .attach_printable("failed to wait for uninstaller's delegated process")
+                    .into_uninstall_report(to_uninstall)
             }
         }
         ct.cancel();
     } else {
         wait_for_process_async(child)
             .await
             .into_report()
-            .change_context(UninstallError::UninstallFailed)
-            .attach_printable("failed to wait for main uninstaller process")?;
+            .attach_printable("failed to wait for main uninstaller process")
+            .into_uninstall_report(to_uninstall)?;
     }
 
     Ok(())
diff --git a/src/cleanup_modules/mod.rs b/src/cleanup_modules/mod.rs
@@ -35,11 +35,29 @@ pub struct ModuleError {
 }
 
 #[derive(Debug, Error)]
-enum UninstallError {
-    #[error("failed to uninstall")]
-    UninstallFailed,
-    #[error("the component is already uninstalled")]
-    AlreadyUninstalled,
+pub(crate) enum UninstallError {
+    #[error("Failed to uninstall {0}")]
+    UninstallFailed(&'static str),
+    #[error("{0} is already uninstalled")]
+    AlreadyUninstalled(&'static str),
+}
+
+impl UninstallError {
+    fn failed<T>(uninstall_object: &T) -> Self
+    where
+        T: Display,
+    {
+        let str: &'static str = Box::leak(uninstall_object.to_string().into_boxed_str());
+        Self::UninstallFailed(str)
+    }
+
+    fn uninstalled<T>(uninstall_object: &T) -> Self
+    where
+        T: Display,
+    {
+        let str: &'static str = Box::leak(uninstall_object.to_string().into_boxed_str());
+        Self::AlreadyUninstalled(str)
+    }
 }
 
 trait ToUninstall<T> {
@@ -125,17 +143,8 @@ where
                     .uninstall_object(object, object_to_uninstall, state, &mut module_run_info)
                     .await;
 
-                match ret {
-                    Ok(_) => {}
-                    Err(err) => match err.current_context() {
-                        UninstallError::AlreadyUninstalled => {
-                            println!("'{}' is already uninstalled!", object_to_uninstall)
-                        }
-                        UninstallError::UninstallFailed => {
-                            eprintln!("Failed to uninstall '{}'!", object_to_uninstall);
-                            eprintln!("{:?}", err);
-                        }
-                    },
+                if let Err(err) = ret {
+                    eprintln!("{:?}", err);
                 }
             }
         }
@@ -204,3 +213,17 @@ where
         self.change_context_lazy(|| ModuleError { name: module_name })
     }
 }
+
+pub(crate) trait IntoUninstallReport<T, T2> {
+    fn into_uninstall_report(self, uninstall_object: &T2) -> Result<T, UninstallError>;
+}
+
+impl<T, T2, E> IntoUninstallReport<T, T2> for CResult<T, Report<E>>
+where
+    T2: Display,
+    E: Error + Context + Display,
+{
+    fn into_uninstall_report(self, uninstall_object: &T2) -> Result<T, UninstallError> {
+        self.change_context_lazy(|| UninstallError::failed(uninstall_object))
+    }
+}
diff --git a/src/services/identifiers.rs b/src/services/identifiers.rs
@@ -89,11 +89,11 @@ pub async fn get_resource(identifier: &'static str, state: &State) -> Result<Sou
     match resource {
         Ok(resource) => {
             no_color(|| info!("Got resource '{}' embedded", identifier));
-            return Ok(resource);
+            Ok(resource)
         }
         Err(err) => {
             no_color(|| warn!("{:?}", err));
-            return Err(err);
+            Err(err)
         }
     }
 }
diff --git a/src/services/windows.rs b/src/services/windows.rs

Original file line number	Diff line number	Diff line change
`@@ -89,11 +89,11 @@ pub async fn get_resource(identifier: &'static str, state: &State) -> Result<Sou`
`89`	`89`	`match resource {`
`90`	`90`	`Ok(resource) => {`
`91`	`91`	`no_color(\|\| info!("Got resource '{}' embedded", identifier));`
`92`		`- return Ok(resource);`
	`92`	`+ Ok(resource)`
`93`	`93`	`}`
`94`	`94`	`Err(err) => {`
`95`	`95`	`no_color(\|\| warn!("{:?}", err));`
`96`		`- return Err(err);`
	`96`	`+ Err(err)`
`97`	`97`	`}`
`98`	`98`	`}`
`99`	`99`	`}`