Add logger to installer-downloader

7 files changed, 30 insertions, 26 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 06256333f4..af13f48aa6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2038,7 +2038,9 @@ dependencies = [
  "anyhow",
  "async-trait",
  "cacao",
+ "env_logger 0.10.2",
  "insta",
+ "log",
  "mullvad-update",
  "native-windows-gui",
  "objc_id",
diff --git a/installer-downloader/Cargo.toml b/installer-downloader/Cargo.toml
index d4895e3b9f..e84e2091be 100644
--- a/installer-downloader/Cargo.toml
+++ b/installer-downloader/Cargo.toml
@@ -22,6 +22,10 @@ async-trait = "0.1"
 rand = { version = "0.8.5" }
 serde = { workspace = true, features = ["derive"] }
 
+# Note: Not using workspace since we want fewer features
+env_logger = { version = "0.10.0", default-features = false }
+log = { workspace = true }
+
 mullvad-update = { path = "../mullvad-update", features = ["client", "native-tls"] }
 
 [target.'cfg(target_os = "windows")'.dependencies]
diff --git a/installer-downloader/src/controller.rs b/installer-downloader/src/controller.rs
index 5bd42709e2..1485f7d2b5 100644
--- a/installer-downloader/src/controller.rs
+++ b/installer-downloader/src/controller.rs
@@ -11,7 +11,6 @@ use mullvad_update::{
 };
 use rand::seq::SliceRandom;
 
-use std::error::Error;
 use std::future::Future;
 use std::path::PathBuf;
 use tokio::sync::{mpsc, oneshot};
@@ -149,7 +148,7 @@ async fn fetch_app_version_info<Delegate, VersionProvider>(
             Err(err) => err,
         };
 
-        eprintln!("Failed to get version info: {err}");
+        log::error!("Failed to get version info: {err:?}");
 
         enum Action {
             Retry,
@@ -235,9 +234,11 @@ async fn handle_action_messages<D, A, DirProvider>(
             }
             TaskMessage::TryBeta => {
                 let Some(version_info) = version_info.as_ref() else {
+                    log::error!("Attempted 'try beta' before having version info");
                     continue;
                 };
                 let Some(beta_info) = version_info.beta.as_ref() else {
+                    log::error!("Attempted 'try beta' without beta version");
                     continue;
                 };
 
@@ -252,6 +253,7 @@ async fn handle_action_messages<D, A, DirProvider>(
             }
             TaskMessage::TryStable => {
                 let Some(version_info) = version_info.as_ref() else {
+                    log::error!("Attempted 'try stable' before having version info");
                     continue;
                 };
                 let stable_info = &version_info.stable;
@@ -267,9 +269,10 @@ async fn handle_action_messages<D, A, DirProvider>(
             }
             TaskMessage::BeginDownload => {
                 if active_download.take().is_some() {
-                    println!("Interrupting ongoing download");
+                    log::debug!("Interrupting ongoing download");
                 }
                 let Some(version_info) = version_info.clone() else {
+                    log::error!("Attempted 'begin download' before having version info");
                     continue;
                 };
 
@@ -341,12 +344,7 @@ async fn handle_action_messages<D, A, DirProvider>(
                     let ui_downloader = UiAppDownloader::new(self_, downloader);
                     let _ = tx.send(tokio::spawn(async move {
                         if let Err(err) = app::install_and_upgrade(ui_downloader).await {
-                            eprintln!("install_and_upgrade failed: {err}");
-                            let mut source = err.source();
-                            while let Some(error) = source {
-                                eprintln!("caused by: {error}");
-                                source = error.source();
-                            }
+                            log::error!("install_and_upgrade failed: {err:?}");
                         }
                     }));
                 });
@@ -359,6 +357,7 @@ async fn handle_action_messages<D, A, DirProvider>(
                 }
 
                 let Some(version_info) = version_info.as_ref() else {
+                    log::error!("Attempted 'cancel' before having version info");
                     continue;
                 };
 
diff --git a/installer-downloader/src/main.rs b/installer-downloader/src/main.rs
index 51f9e303e5..f190377d28 100644
--- a/installer-downloader/src/main.rs
+++ b/installer-downloader/src/main.rs
@@ -13,6 +13,8 @@ mod inner {
     pub use installer_downloader::resource;
 
     pub fn run() {
+        env_logger::init();
+
         let rt = tokio::runtime::Builder::new_multi_thread()
             .enable_all()
             .build()
diff --git a/installer-downloader/src/winapi_impl/ui.rs b/installer-downloader/src/winapi_impl/ui.rs
index a24874a55a..a83d2007bc 100644
--- a/installer-downloader/src/winapi_impl/ui.rs
+++ b/installer-downloader/src/winapi_impl/ui.rs
@@ -314,16 +314,16 @@ impl AppWindow {
     /// This function is called when the top-level window has been created
     fn on_init(&self) {
         if let Err(err) = self.load_banner_image() {
-            eprintln!("load_banner_image failed: {err}");
+            log::error!("load_banner_image failed: {err}");
             // not fatal, so continue
         }
         if let Err(err) = self.load_banner_text_image() {
-            eprintln!("load_banner_text_image failed: {err}");
+            log::error!("load_banner_text_image failed: {err}");
             // not fatal, so continue
         }
 
         if let Err(err) = handle_banner_label_colors(&self.banner.handle, SET_LABEL_HANDLER_ID) {
-            eprintln!("handle_banner_label_colors failed: {err}");
+            log::error!("handle_banner_label_colors failed: {err}");
             // not fatal, so continue
         }
 
diff --git a/installer-downloader/tests/controller.rs b/installer-downloader/tests/controller.rs
index 704835e5a7..0128746a42 100644
--- a/installer-downloader/tests/controller.rs
+++ b/installer-downloader/tests/controller.rs
@@ -14,6 +14,7 @@ use mullvad_update::api::VersionInfoProvider;
 use mullvad_update::app::{AppDownloader, DownloadError};
 use mullvad_update::fetch::ProgressUpdater;
 use mullvad_update::version::{Version, VersionInfo, VersionParameters};
+use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, LazyLock, Mutex};
 use std::time::Duration;
@@ -113,8 +114,8 @@ impl<const EXE_SUCCEED: bool, const VERIFY_SUCCEED: bool, const LAUNCH_SUCCEED:
         if LAUNCH_SUCCEED {
             Ok(())
         } else {
-            Err(DownloadError::InstallFailed(anyhow::anyhow!(
-                "install failed"
+            Err(DownloadError::InstallFailed(io::Error::other(
+                "install failed",
             )))
         }
     }
diff --git a/mullvad-update/src/client/app.rs b/mullvad-update/src/client/app.rs
index 8987cb73bd..decf8d932b 100644
--- a/mullvad-update/src/client/app.rs
+++ b/mullvad-update/src/client/app.rs
@@ -11,16 +11,16 @@ use crate::{
 
 #[derive(Debug, thiserror::Error)]
 pub enum DownloadError {
-    #[error("Failed to create download directory")]
-    CreateDir(#[source] anyhow::Error),
     #[error("Failed to download app")]
     FetchApp(#[source] anyhow::Error),
     #[error("Failed to verify app")]
     Verification(#[source] anyhow::Error),
     #[error("Failed to launch app")]
     Launch(#[source] std::io::Error),
-    #[error("App installer failed")]
-    InstallFailed(#[source] anyhow::Error),
+    #[error("Installer exited with error: {0}")]
+    InstallExited(std::process::ExitStatus),
+    #[error("Installer failed on child.wait(): {0}")]
+    InstallFailed(std::io::Error),
 }
 
 /// Parameters required to construct an [AppDownloader].
@@ -125,14 +125,10 @@ impl<AppProgress: ProgressUpdater> AppDownloader for HttpAppDownloader<AppProgre
             Err(_timeout) => Ok(()),
             // No timeout: Incredibly quick but successful (or wrong exit code, probably)
             Ok(Ok(status)) if status.success() => Ok(()),
-            // Installer failed
-            Ok(Ok(status)) => Err(DownloadError::InstallFailed(anyhow::anyhow!(
-                "Install failed with status: {status}"
-            ))),
-            // Installer failed
-            Ok(Err(err)) => Err(DownloadError::InstallFailed(anyhow::anyhow!(
-                "Install failed: {err}"
-            ))),
+            // Installer exited with error code
+            Ok(Ok(status)) => Err(DownloadError::InstallExited(status)),
+            // `child.wait()` returned an error
+            Ok(Err(err)) => Err(DownloadError::InstallFailed(err)),
         }
     }
 }