Skip to content

Commit

Permalink
Add logger to installer-downloader
Browse files Browse the repository at this point in the history
  • Loading branch information
dlon committed Feb 23, 2025
1 parent 2774e14 commit 4eef94d
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 26 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions installer-downloader/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
17 changes: 8 additions & 9 deletions installer-downloader/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
};

Expand All @@ -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;
Expand All @@ -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;
};

Expand Down Expand Up @@ -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:?}");
}
}));
});
Expand All @@ -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;
};

Expand Down
2 changes: 2 additions & 0 deletions installer-downloader/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions installer-downloader/src/winapi_impl/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,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
}

Expand Down
5 changes: 3 additions & 2 deletions installer-downloader/tests/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
)))
}
}
Expand Down
20 changes: 8 additions & 12 deletions mullvad-update/src/client/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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)),
}
}
}
Expand Down

0 comments on commit 4eef94d

Please sign in to comment.