From 8c5812d652ef72d4d6e96b8896dbac30dd7dffc2 Mon Sep 17 00:00:00 2001 From: LynithDev <61880709+LynithDev@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:03:42 +0200 Subject: [PATCH] fix: migration issues, cluster preparation and sentry logging --- Cargo.lock | 1 + packages/oneclient_app/src/assets.rs | 3 +- .../src/components/dynamic_art.rs | 11 +- packages/oneclient_app/src/events.rs | 15 +- packages/oneclient_app/src/hooks/actions.rs | 19 +- .../oneclient_app/src/hooks/queries/image.rs | 16 +- packages/oneclient_app/src/lib.rs | 1 + packages/oneclient_app/src/main.rs | 3 +- packages/oneclient_app/src/recovery.rs | 70 +++++ packages/oneclient_app/src/state.rs | 2 + .../src/view/app/clusters/page.rs | 193 +++++++------ packages/oneclient_app/src/view/startup.rs | 64 ++++- packages/oneclient_auth/src/diagnostics.rs | 53 +--- .../oneclient_core/src/clusters/prepare.rs | 11 +- packages/oneclient_core/src/error.rs | 49 +++- packages/oneclient_core/src/logger.rs | 32 ++- packages/oneclient_core/src/reporting.rs | 57 +--- packages/oneclient_db/Cargo.toml | 1 + packages/oneclient_db/src/backup.rs | 268 ++++++++++++++++++ packages/oneclient_db/src/lib.rs | 2 + packages/oneclient_db/src/migrate.rs | 133 +++++++++ packages/oneclient_db/src/pool.rs | 2 +- packages/oneclient_net/src/error.rs | 105 ++++++- packages/oneclient_net/src/lib.rs | 5 +- 24 files changed, 890 insertions(+), 226 deletions(-) create mode 100644 packages/oneclient_app/src/recovery.rs create mode 100644 packages/oneclient_db/src/backup.rs create mode 100644 packages/oneclient_db/src/migrate.rs diff --git a/Cargo.lock b/Cargo.lock index 6b3fcd03..7bb879b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4704,6 +4704,7 @@ dependencies = [ name = "oneclient_db" version = "2.2.1" dependencies = [ + "async-tempfile", "chrono", "dunce", "serde", diff --git a/packages/oneclient_app/src/assets.rs b/packages/oneclient_app/src/assets.rs index bb35c241..74287862 100644 --- a/packages/oneclient_app/src/assets.rs +++ b/packages/oneclient_app/src/assets.rs @@ -7,8 +7,7 @@ impl AppAssets { AppAssets::get(path).map(|file| match file.data { std::borrow::Cow::Borrowed(slice) => bytes::Bytes::copy_from_slice(slice), std::borrow::Cow::Owned(vec) => { - #[cfg(not(debug_assertions))] - tracing::warn!("Cloning asset data for '{}'", path); + tracing::debug!("Cloning asset data for '{}'", path); bytes::Bytes::from(vec) } }) diff --git a/packages/oneclient_app/src/components/dynamic_art.rs b/packages/oneclient_app/src/components/dynamic_art.rs index 8564e71b..f533c1e7 100644 --- a/packages/oneclient_app/src/components/dynamic_art.rs +++ b/packages/oneclient_app/src/components/dynamic_art.rs @@ -90,15 +90,18 @@ pub fn use_art_bytes( let image_query = use_cached_image(art_url.clone(), max_edge); - // Subscribed unconditionally to keep hook order stable same query when no preview size - let preview_edge = preview_edge.unwrap_or(max_edge); - let preview_query = use_cached_image(art_url.clone(), preview_edge); + let preview_edge = preview_edge.filter(|edge| *edge != max_edge); + let preview_query = match preview_edge { + Some(edge) => use_cached_image(art_url.clone(), edge), + None => use_cached_image(None, 0), + }; let full = loaded_image(art_url.as_deref(), &image_query) .map(|(url, bytes)| (format!("{max_edge}|{url}"), bytes)); let preview = || { + let edge = preview_edge?; loaded_image(art_url.as_deref(), &preview_query) - .map(|(url, bytes)| (format!("{preview_edge}|{url}"), bytes)) + .map(|(url, bytes)| (format!("{edge}|{url}"), bytes)) }; full.or_else(preview).unwrap_or_else(|| { diff --git a/packages/oneclient_app/src/events.rs b/packages/oneclient_app/src/events.rs index 15c979d2..1b7e09a9 100644 --- a/packages/oneclient_app/src/events.rs +++ b/packages/oneclient_app/src/events.rs @@ -323,6 +323,7 @@ pub async fn start_launcher( fetching: true, syncing_bundles: true, error: None, + snapshots: 0, data_dir, }; } @@ -345,11 +346,17 @@ pub fn report_startup_failure( let message = err.to_string(); tracing::error!("launcher init failed: {err:#}"); + let data_dir = oneclient_common::paths::launcher_dir() + .map(|p| p.display().to_string()) + .unwrap_or_default(); + let mut station = *station; - station - .write_channel(AppChannel::Launcher) - .launcher - .error = Some(message.clone()); + { + let mut guard = station.write_channel(AppChannel::Launcher); + guard.launcher.error = Some(message.clone()); + guard.launcher.data_dir = data_dir; + guard.launcher.snapshots = crate::recovery::snapshots().len(); + } let mut guard = station.write_channel(AppChannel::Notifications); let AppState { diff --git a/packages/oneclient_app/src/hooks/actions.rs b/packages/oneclient_app/src/hooks/actions.rs index 7afd5087..aee585fc 100644 --- a/packages/oneclient_app/src/hooks/actions.rs +++ b/packages/oneclient_app/src/hooks/actions.rs @@ -74,15 +74,32 @@ pub enum PumpSignal { pub struct Actions { station: RadioStation, pump: mpsc::UnboundedSender, + events: oneclient_events::EventBus, } impl Actions { + #[must_use] + pub fn station(&self) -> RadioStation { + self.station + } + + /// A channel handle, so a clone still feeds the pump set up at launch + #[must_use] + pub fn events(&self) -> oneclient_events::EventBus { + self.events.clone() + } + #[must_use] pub fn new( station: RadioStation, pump: mpsc::UnboundedSender, + events: oneclient_events::EventBus, ) -> Self { - Self { station, pump } + Self { + station, + pump, + events, + } } fn nudge(&self, signal: PumpSignal) { diff --git a/packages/oneclient_app/src/hooks/queries/image.rs b/packages/oneclient_app/src/hooks/queries/image.rs index d3dd8499..95b997bb 100644 --- a/packages/oneclient_app/src/hooks/queries/image.rs +++ b/packages/oneclient_app/src/hooks/queries/image.rs @@ -44,15 +44,13 @@ pub fn loaded_image(url: Option<&str>, query: &UseQuery) -> Op } pub fn use_cached_image(url: Option, max_edge: u32) -> UseQuery { + let url = url.unwrap_or_default(); + let enabled = !url.is_empty(); + use_query( - Query::new( - CachedImageKeys { - url: url.unwrap_or_default(), - max_edge, - }, - CachedImageQuery, - ) - .stale_time(IMAGE_STALE) - .clean_time(IMAGE_CLEAN), + Query::new(CachedImageKeys { url, max_edge }, CachedImageQuery) + .enable(enabled) + .stale_time(IMAGE_STALE) + .clean_time(IMAGE_CLEAN), ) } diff --git a/packages/oneclient_app/src/lib.rs b/packages/oneclient_app/src/lib.rs index 412de5f4..cb3c2d34 100644 --- a/packages/oneclient_app/src/lib.rs +++ b/packages/oneclient_app/src/lib.rs @@ -14,6 +14,7 @@ mod transfer; mod layout; mod notifications; pub mod platform; +pub mod recovery; mod routes; pub mod theme; mod ui; diff --git a/packages/oneclient_app/src/main.rs b/packages/oneclient_app/src/main.rs index 537b84b9..fffbaf56 100644 --- a/packages/oneclient_app/src/main.rs +++ b/packages/oneclient_app/src/main.rs @@ -18,13 +18,12 @@ struct OneClientApp; impl App for OneClientApp { fn render(&self) -> impl IntoElement { // Radio state is `!Send` so every writer runs on the UI thread `spawn_forever` - // rather than `spawn` because these must outlive any component scope let station = use_init_radio_station::(AppState::default); let actions = use_hook(move || { let (signals_tx, signals_rx) = tokio::sync::mpsc::unbounded_channel(); let (events_bus, events_rx) = oneclient_events::EventBus::channel(); - let actions = Actions::new(station, signals_tx); + let actions = Actions::new(station, signals_tx, events_bus.clone()); // Started first so nothing emitted during startup waits on a consumer spawn_forever( diff --git a/packages/oneclient_app/src/recovery.rs b/packages/oneclient_app/src/recovery.rs new file mode 100644 index 00000000..8d480573 --- /dev/null +++ b/packages/oneclient_app/src/recovery.rs @@ -0,0 +1,70 @@ +use std::path::PathBuf; + +use freya::prelude::spawn_forever; + +use crate::hooks::Actions; + +pub fn database_path() -> Option { + oneclient_common::paths::database_file().ok() +} + +pub fn snapshots() -> Vec { + database_path() + .map(|path| oneclient_db::backup::list(&path)) + .unwrap_or_default() +} + +pub fn open_data_folder() { + let Ok(dir) = oneclient_common::paths::launcher_dir() else { + return; + }; + + crate::platform::open_url(&dir.to_string_lossy()); +} + +pub fn restore_latest(actions: &Actions) { + let Some(path) = database_path() else { return }; + let Some(snapshot) = snapshots().into_iter().next() else { + set_error(actions, "There are no snapshots to restore.".to_owned()); + return; + }; + + match oneclient_db::backup::restore(&path, &snapshot) { + Ok(_) => retry(actions), + Err(err) => set_error(actions, format!("Couldn't restore the snapshot: {err}")), + } +} + +pub fn reset(actions: &Actions) { + let Some(path) = database_path() else { return }; + + match oneclient_db::backup::reset(&path) { + Ok(_) => retry(actions), + Err(err) => set_error(actions, format!("Couldn't reset the database: {err}")), + } +} + +fn retry(actions: &Actions) { + let station = actions.station(); + let events = actions.events(); + + { + let mut guard = station.clone().write_channel(crate::state::AppChannel::Launcher); + guard.launcher.error = None; + guard.launcher.snapshots = snapshots().len(); + } + + spawn_forever(async move { + if let Err(err) = crate::events::start_launcher(station, events).await { + crate::events::report_startup_failure(&station, &err); + } + }); +} + +fn set_error(actions: &Actions, message: String) { + let mut guard = actions + .station() + .write_channel(crate::state::AppChannel::Launcher); + guard.launcher.error = Some(message); + guard.launcher.snapshots = snapshots().len(); +} diff --git a/packages/oneclient_app/src/state.rs b/packages/oneclient_app/src/state.rs index e5479294..eae4ac29 100644 --- a/packages/oneclient_app/src/state.rs +++ b/packages/oneclient_app/src/state.rs @@ -73,6 +73,8 @@ pub struct LauncherInit { pub syncing_bundles: bool, pub error: Option, pub data_dir: String, + /// Restorable database snapshots, for the startup failure screen + pub snapshots: usize, } #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] diff --git a/packages/oneclient_app/src/view/app/clusters/page.rs b/packages/oneclient_app/src/view/app/clusters/page.rs index babc9767..14b74fa0 100644 --- a/packages/oneclient_app/src/view/app/clusters/page.rs +++ b/packages/oneclient_app/src/view/app/clusters/page.rs @@ -171,114 +171,121 @@ impl Component for Clusters { ), ) .child(match cluster { - Some(cluster) => detail_sidebar( + Some(cluster) => DetailSidebar { line, - cluster, - &clusters_for_line, + cluster_id: cluster.id, + clusters_for_line, selected_version, selected_loader, - ), + } + .into_element(), None => sidebar_error(), }), ) } } -fn detail_sidebar( +#[derive(PartialEq)] +struct DetailSidebar { line: ReleaseLine, - cluster: Cluster, - clusters_for_line: &[Cluster], + cluster_id: i64, + clusters_for_line: Vec, selected_version: State>, selected_loader: State>, -) -> Element { - let active_id = use_active_cluster_id(); - let dispatch = use_dispatch(); - let game = use_game_snapshot(); - let launcher = use_launcher(); - let syncing = launcher.fetching || launcher.syncing_bundles; - let keys = version_keys(clusters_for_line); - let loaders = loaders_for_line(clusters_for_line); - let cluster_id = cluster.id; - let version_title = line_title(line, clusters_for_line); - let version_value = *selected_version.read(); - let loader_value = *selected_loader.read(); +} - let metadata = use_version_metadata(Some(line.major), version_value, loader_value); +impl Component for DetailSidebar { + fn render(&self) -> impl IntoElement { + let active_id = use_active_cluster_id(); + let dispatch = use_dispatch(); + let game = use_game_snapshot(); + let launcher = use_launcher(); - let heading = metadata - .as_ref() - .map(|m| m.name.clone()) - .unwrap_or_else(|| format!("Version {version_title}")); - let description = metadata - .as_ref() - .and_then(|m| m.long_description.clone()) - .unwrap_or_else(|| PLACEHOLDER_VERSION_INFO.to_string()); - let tags = metadata - .as_ref() - .map(|m| m.tags.clone()) - .unwrap_or_default(); + let clusters_for_line = self.clusters_for_line.as_slice(); + let syncing = launcher.fetching || launcher.syncing_bundles; + let keys = version_keys(clusters_for_line); + let loaders = loaders_for_line(clusters_for_line); + let version_title = line_title(self.line, clusters_for_line); + let version_value = (self.selected_version)(); + let loader_value = (self.selected_loader)(); - rect() - .width(Size::px(SIDEBAR_WIDTH_PX)) - .min_width(Size::px(SIDEBAR_WIDTH_PX)) - .height(Size::fill()) - .vertical() - .spacing(8.) - .padding(8.) - .corner_radius(CornerRadius::new_all(12.)) - .background(colors::page_elevated()) - .border(border_all_color(1., colors::component_border())) - .overflow(Overflow::Clip) - .child(rect().width(Size::fill()).max_height(Size::px(140.)).child( - ClusterLandscapeArt::for_version(line.major, version_value, loader_value, false), - )) - .child( - rect() - .vertical() - .width(Size::fill()) - .height(Size::flex(1.0)) - .content(Content::Flex) - .padding(Gaps::new_all(8.)) - .spacing(8.) - .child( - rect() - .vertical() - .width(Size::fill()) - .height(Size::flex(1.0)) - .spacing(4.) - .child( - label() - .text(heading) - .font_size(24.) - .font_weight(FontWeight::SEMI_BOLD) - .color(colors::fg_primary()), - ) - .maybe_child(tags_row(&tags)) - .child( - label() - .text(description) - .font_size(12.) - .color(colors::fg_secondary()), - ) - .children(version_rows(line, &keys, version_value, selected_version)) - .children(loader_rows(&loaders, loader_value, selected_loader)), - ) - .child( - rect() - .horizontal() - .width(Size::fill()) - .content(Content::Flex) - .main_align(Alignment::Center) - .spacing(8.) - .child(play_button( - cluster_id, - dispatch, - launch_button_state(&game, cluster_id, syncing), - )) - .child(view_button(cluster_id, active_id)), - ), - ) - .into_element() + let metadata = use_version_metadata(Some(self.line.major), version_value, loader_value); + + let heading = metadata + .as_ref() + .map(|m| m.name.clone()) + .unwrap_or_else(|| format!("Version {version_title}")); + let description = metadata + .as_ref() + .and_then(|m| m.long_description.clone()) + .unwrap_or_else(|| PLACEHOLDER_VERSION_INFO.to_string()); + let tags = metadata + .as_ref() + .map(|m| m.tags.clone()) + .unwrap_or_default(); + + rect() + .width(Size::px(SIDEBAR_WIDTH_PX)) + .min_width(Size::px(SIDEBAR_WIDTH_PX)) + .height(Size::fill()) + .vertical() + .spacing(8.) + .padding(8.) + .corner_radius(CornerRadius::new_all(12.)) + .background(colors::page_elevated()) + .border(border_all_color(1., colors::component_border())) + .overflow(Overflow::Clip) + .child(rect().width(Size::fill()).max_height(Size::px(140.)).child( + ClusterLandscapeArt::for_version(self.line.major, version_value, loader_value, false), + )) + .child( + rect() + .vertical() + .width(Size::fill()) + .height(Size::flex(1.0)) + .content(Content::Flex) + .padding(Gaps::new_all(8.)) + .spacing(8.) + .child( + rect() + .vertical() + .width(Size::fill()) + .height(Size::flex(1.0)) + .spacing(4.) + .child( + label() + .text(heading) + .font_size(24.) + .font_weight(FontWeight::SEMI_BOLD) + .color(colors::fg_primary()), + ) + .maybe_child(tags_row(&tags)) + .child( + label() + .text(description) + .font_size(12.) + .color(colors::fg_secondary()), + ) + .children(version_rows(self.line, &keys, version_value, self.selected_version)) + .children(loader_rows(&loaders, loader_value, self.selected_loader)), + ) + .child( + rect() + .horizontal() + .width(Size::fill()) + .content(Content::Flex) + .main_align(Alignment::Center) + .spacing(8.) + .child(play_button( + self.cluster_id, + dispatch, + launch_button_state(&game, self.cluster_id, syncing), + )) + .child(view_button(self.cluster_id, active_id)), + ), + ) + .into_element() + } } fn tags_row(tags: &[String]) -> Option { diff --git a/packages/oneclient_app/src/view/startup.rs b/packages/oneclient_app/src/view/startup.rs index 28c11da8..a7e1edb8 100644 --- a/packages/oneclient_app/src/view/startup.rs +++ b/packages/oneclient_app/src/view/startup.rs @@ -7,8 +7,54 @@ use crate::hooks::{ terms_document, terms_is_loading, use_launcher, use_notifications_snapshot, use_settings_snapshot, use_splash, use_terms, }; +use crate::hooks::use_dispatch; +use crate::components::Button; use crate::theme::colors; +#[derive(PartialEq)] +struct RecoveryActions { + snapshots: usize, +} + +impl Component for RecoveryActions { + fn render(&self) -> impl IntoElement { + let actions = use_dispatch(); + let snapshots = self.snapshots; + + let restore = actions.clone(); + + let mut row = rect() + .horizontal() + .cross_align(Alignment::Center) + .spacing(8.) + .child( + Button::new() + .secondary() + .small() + .on_press(|_| crate::recovery::open_data_folder()) + .text("Open data folder"), + ); + + if snapshots > 0 { + row = row.child( + Button::new() + .primary() + .small() + .on_press(move |_| crate::recovery::restore_latest(&restore)) + .text("Restore last backup"), + ); + } + + row.child( + Button::new() + .danger() + .small() + .on_press(move |_| crate::recovery::reset(&actions)) + .text("Reset database"), + ) + } +} + #[derive(PartialEq)] pub struct Startup; @@ -153,11 +199,27 @@ impl Component for Startup { label() .text(detail) .font_size(12.) - .max_lines(1) + .max_lines(if is_error { 4 } else { 1 }) .color(colors::fg_secondary().with_a(180)), ); } + if is_error { + content = content + .child(rect().height(Size::px(6.))) + .child(RecoveryActions { snapshots: launcher.snapshots }) + .child( + label() + .text(format!( + "Your data stays in {}. Nothing here deletes it.", + launcher.data_dir + )) + .font_size(11.) + .max_lines(2) + .color(colors::fg_secondary().with_a(130)), + ); + } + let content = content .position(Position::new_absolute()) .width(Size::fill()) diff --git a/packages/oneclient_auth/src/diagnostics.rs b/packages/oneclient_auth/src/diagnostics.rs index c919656f..84ea1ef4 100644 --- a/packages/oneclient_auth/src/diagnostics.rs +++ b/packages/oneclient_auth/src/diagnostics.rs @@ -1,6 +1,7 @@ //! Adapted from Modrinth's `minecraft-auth-errors.ts` //! (GPL-3.0) +use oneclient_net::{NetworkFailure, classify_network_failure}; use reqwest::StatusCode; use super::error::{friendly_xbox_error, MinecraftAuthError, MinecraftAuthStep}; @@ -302,58 +303,6 @@ fn xerr(code: u64) -> MinecraftAuthError { } } -/// reqwest reports all of these as a connect error so the source chain text is -/// the only thing separating them anything unrecognised falls back to Generic -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum NetworkFailure { - Certificate, - Handshake, - Dns, - Generic, -} - -fn classify_network_failure(err: &reqwest::Error) -> NetworkFailure { - let chain = oneclient_net::error_chain(err).to_ascii_lowercase(); - - // Certificate before handshake a rejected certificate also aborts the - // handshake and the certificate wording is the more specific of the two - const CERTIFICATE: &[&str] = &[ - "invalid peer certificate", - "unknownissuer", - "certificate", - "certexpired", - "notvalidforname", - ]; - const HANDSHAKE: &[&str] = &[ - "handshake", - "received fatal alert", - "peer misbehaved", - "no cipher suites in common", - "protocol version", - "unsupported protocol", - ]; - const DNS: &[&str] = &[ - "dns error", - "failed to lookup address", - "no record found", - "nodename nor servname", - "name or service not known", - "no such host", - ]; - - let matches = |markers: &[&str]| markers.iter().any(|marker| chain.contains(marker)); - - if matches(CERTIFICATE) { - NetworkFailure::Certificate - } else if matches(HANDSHAKE) { - NetworkFailure::Handshake - } else if matches(DNS) { - NetworkFailure::Dns - } else { - NetworkFailure::Generic - } -} - fn network_guidance(failure: NetworkFailure) -> AuthErrorGuidance { match failure { NetworkFailure::Certificate => AuthErrorGuidance::new( diff --git a/packages/oneclient_core/src/clusters/prepare.rs b/packages/oneclient_core/src/clusters/prepare.rs index 7077b03f..e3f929ea 100644 --- a/packages/oneclient_core/src/clusters/prepare.rs +++ b/packages/oneclient_core/src/clusters/prepare.rs @@ -88,7 +88,16 @@ pub async fn prepare_cluster( } if let Err(err) = result { - tracing::error!(cluster_id, error = %err, "cluster preparation failed"); + if err.is_cancelled() { + tracing::info!( + cluster_id, + sentry = false, + error = %err, + "cluster preparation cancelled by the user" + ); + } else { + tracing::error!(cluster_id, error = %err, "cluster preparation failed"); + } if !continuing { let _ = state.clusters.set_stage(cluster_id, ClusterStage::NotReady).await; } diff --git a/packages/oneclient_core/src/error.rs b/packages/oneclient_core/src/error.rs index 213bf101..e36d89f3 100644 --- a/packages/oneclient_core/src/error.rs +++ b/packages/oneclient_core/src/error.rs @@ -92,12 +92,18 @@ impl LauncherError { ) } - /// Whether the install is missing pieces and so is worth repairing - /// - /// Deliberately narrow only a genuine not-found qualifies - /// Permission errors full disks and unreachable shares all survive a - /// repair unchanged + /// A dialog the user backed out of rather than anything that went wrong #[must_use] + pub fn is_cancelled(&self) -> bool { + self.is_login_cancelled() + || matches!( + self, + LauncherError::JavaError(oneclient_java::JavaError::Cancelled) + ) + } + + /// Whether the install is missing pieces and so is worth repairing + #[must_use] pub fn indicates_missing_files(&self) -> bool { let mut source: Option<&(dyn std::error::Error + 'static)> = Some(self); @@ -217,6 +223,39 @@ mod tests { assert!(err.indicates_missing_files(), "{err}"); } + #[test] + fn dismissing_a_dialog_is_a_cancellation_not_a_failure() { + assert!( + LauncherError::JavaError(oneclient_java::JavaError::Cancelled).is_cancelled() + ); + assert!( + LauncherError::AuthError(oneclient_auth::AuthError::LoginCancelled).is_cancelled() + ); + } + + /// The whole point of the split anything that is not a dismissal keeps + /// reporting at `error` + #[test] + fn real_java_failures_are_not_cancellations() { + for err in [ + oneclient_java::JavaError::MissingJava, + oneclient_java::JavaError::PackageNotFound { major: 21 }, + oneclient_java::JavaError::InvalidJavaPath { + path: "/opt/not-java".to_string(), + }, + oneclient_java::JavaError::VersionMismatch { + expected: 21, + found: 8, + }, + ] { + let err = LauncherError::JavaError(err); + assert!(!err.is_cancelled(), "{err}"); + } + + assert!(!LauncherError::NotInitialized.is_cancelled()); + assert!(!not_found("metadata/assets").is_cancelled()); + } + #[test] fn failures_a_repair_could_not_fix_are_left_alone() { for kind in [ diff --git a/packages/oneclient_core/src/logger.rs b/packages/oneclient_core/src/logger.rs index b704f0fd..6f2388fe 100644 --- a/packages/oneclient_core/src/logger.rs +++ b/packages/oneclient_core/src/logger.rs @@ -56,6 +56,36 @@ const NOISY_TARGETS: &[&str] = &[ "zbus", ]; +/// Whether `target` belongs to one of [`APP_TARGETS`] rather than a dependency +fn is_app_target(target: &str) -> bool { + APP_TARGETS.iter().any(|app| { + target + .strip_prefix(app) + .is_some_and(|rest| rest.is_empty() || rest.starts_with("::")) + }) +} + +fn sentry_event_filter(metadata: &tracing::Metadata<'_>) -> sentry_tracing::EventFilter { + dependency_filter(*metadata.level(), metadata.target()) + .unwrap_or_else(|| sentry_tracing::default_event_filter(metadata)) +} + +/// `None` when the target is one of ours, meaning the default filter decides +fn dependency_filter( + level: tracing::Level, + target: &str, +) -> Option { + if is_app_target(target) { + return None; + } + + Some(match level { + // Kept as context for a report of ours, but never a report of its own + tracing::Level::ERROR => sentry_tracing::EventFilter::Breadcrumb, + _ => sentry_tracing::EventFilter::Ignore, + }) +} + pub fn default_directives() -> String { directives("info", "warn") } @@ -146,7 +176,7 @@ pub fn init_filtered(filter: impl FnOnce() -> String) -> LauncherResult<()> { let stdout_layer = tracing_subscriber::fmt::layer().with_writer(io::stdout); - let sentry_layer = sentry_tracing::layer(); + let sentry_layer = sentry_tracing::layer().event_filter(sentry_event_filter); #[cfg(debug_assertions)] { diff --git a/packages/oneclient_core/src/reporting.rs b/packages/oneclient_core/src/reporting.rs index f60bb0ee..96d36982 100644 --- a/packages/oneclient_core/src/reporting.rs +++ b/packages/oneclient_core/src/reporting.rs @@ -2,7 +2,7 @@ use std::panic; use std::sync::Arc; use std::time::Duration; -use sentry::protocol::{Context, Event}; +use sentry::protocol::{Breadcrumb, Context, Event}; use sentry::{ClientInitGuard, ClientOptions}; use oneclient_common::constants::SENTRY_DSN; @@ -31,6 +31,16 @@ fn drop_opted_out_events(event: Event<'static>) -> Option> { if opted_out { None } else { Some(event) } } +fn drop_opted_out_breadcrumbs(breadcrumb: Breadcrumb) -> Option { + let opted_out = breadcrumb + .data + .get("sentry") + .and_then(serde_json::Value::as_bool) + == Some(false); + + if opted_out { None } else { Some(breadcrumb) } +} + pub fn init(enabled: bool) -> Option { if !enabled { tracing::debug!("crash reporting disabled by settings"); @@ -50,6 +60,7 @@ pub fn init(enabled: bool) -> Option { attach_stacktrace: true, send_default_pii: false, before_send: Some(Arc::new(drop_opted_out_events)), + before_breadcrumb: Some(Arc::new(drop_opted_out_breadcrumbs)), ..Default::default() }, )); @@ -75,46 +86,4 @@ fn install_panic_flush() { client.flush(Some(PANIC_FLUSH_TIMEOUT)); } })); -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn bundled_dsn_parses() { - SENTRY_DSN - .parse::() - .expect("bundled sentry DSN should be valid"); - } - - #[test] - fn opting_out_skips_the_client() { - assert!(init(false).is_none()); - } - - /// Builds an event the way `sentry-tracing` would - fn event_with_sentry_field(value: Option) -> Event<'static> { - let mut event = Event::default(); - if let Some(value) = value { - let mut fields = std::collections::BTreeMap::new(); - fields.insert("sentry".to_owned(), serde_json::Value::Bool(value)); - event - .contexts - .insert(TRACING_FIELDS_CONTEXT.to_owned(), Context::Other(fields)); - } - event - } - - #[test] - fn drops_events_flagged_sentry_false() { - assert!(drop_opted_out_events(event_with_sentry_field(Some(false))).is_none()); - } - - #[test] - fn keeps_events_without_opt_out() { - assert!(drop_opted_out_events(event_with_sentry_field(Some(true))).is_some()); - assert!(drop_opted_out_events(event_with_sentry_field(None)).is_some()); - assert!(drop_opted_out_events(Event::default()).is_some()); - } -} +} \ No newline at end of file diff --git a/packages/oneclient_db/Cargo.toml b/packages/oneclient_db/Cargo.toml index 21edaa66..73e03cb4 100644 --- a/packages/oneclient_db/Cargo.toml +++ b/packages/oneclient_db/Cargo.toml @@ -15,6 +15,7 @@ path = "src/lib.rs" [dev-dependencies] tokio.workspace = true +async-tempfile.workspace = true [dependencies] sqlx.workspace = true diff --git a/packages/oneclient_db/src/backup.rs b/packages/oneclient_db/src/backup.rs new file mode 100644 index 00000000..426deef6 --- /dev/null +++ b/packages/oneclient_db/src/backup.rs @@ -0,0 +1,268 @@ +//! Snapshots taken before a migration + +use std::path::{Path, PathBuf}; + +use crate::{DbError, DbPool}; + +pub const KEEP: usize = 5; + +const SIDECARS: &[&str] = &["-wal", "-shm", "-journal"]; + +pub fn dir(database_path: &Path) -> PathBuf { + database_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("backups") +} + +fn stem(database_path: &Path) -> String { + database_path + .file_stem() + .map(|stem| stem.to_string_lossy().into_owned()) + .unwrap_or_else(|| "database".to_owned()) +} + +fn timestamp() -> String { + chrono::Local::now().format("%Y%m%d-%H%M%S").to_string() +} + +/// A consistent copy without blocking on the journal, unlike a file copy +pub async fn snapshot(pool: &DbPool, database_path: &Path) -> Result { + let dir = dir(database_path); + std::fs::create_dir_all(&dir)?; + + let target = dir.join(format!("{}-{}.db", stem(database_path), timestamp())); + + sqlx::query("VACUUM INTO ?") + .bind(target.to_string_lossy().as_ref()) + .execute(pool) + .await?; + + Ok(target) +} + +/// Newest first +pub fn list(database_path: &Path) -> Vec { + let prefix = format!("{}-", stem(database_path)); + + let Ok(entries) = std::fs::read_dir(dir(database_path)) else { + return Vec::new(); + }; + + let mut found: Vec = entries + .flatten() + .map(|entry| entry.path()) + .filter(|path| { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| { + name.ends_with(".db") + && name.strip_prefix(&prefix).is_some_and(|rest| { + rest.starts_with(|c: char| c.is_ascii_digit()) + }) + }) + }) + .collect(); + + found.sort(); + found.reverse(); + found +} + +pub fn prune(database_path: &Path, keep: usize) { + for old in list(database_path).into_iter().skip(keep) { + if let Err(err) = std::fs::remove_file(&old) { + tracing::warn!(path = %old.display(), %err, "could not remove old snapshot"); + } + } +} + +/// Moves the live database into `backups/` and returns where it went. The +/// caller is left with no database at `database_path`, which is what both +/// restore and reset want before they continue. +pub fn set_aside(database_path: &Path) -> Result, DbError> { + if !database_path.exists() { + return Ok(None); + } + + let dir = dir(database_path); + std::fs::create_dir_all(&dir)?; + + let target = dir.join(format!( + "{}-replaced-{}.db", + stem(database_path), + timestamp() + )); + std::fs::rename(database_path, &target)?; + + for suffix in SIDECARS { + let sidecar = sidecar(database_path, suffix); + if sidecar.exists() { + let _ = std::fs::remove_file(sidecar); + } + } + + Ok(Some(target)) +} + +fn sidecar(database_path: &Path, suffix: &str) -> PathBuf { + let mut name = database_path.as_os_str().to_os_string(); + name.push(suffix); + PathBuf::from(name) +} + +/// Puts `snapshot` back, keeping the current database in `backups/` first +pub fn restore(database_path: &Path, snapshot: &Path) -> Result, DbError> { + let replaced = set_aside(database_path)?; + std::fs::copy(snapshot, database_path)?; + + tracing::info!( + snapshot = %snapshot.display(), + "restored the database from a snapshot" + ); + + Ok(replaced) +} + +/// Starts over. The old database is moved aside, never deleted, so it is +/// recoverable by hand afterwards. +pub fn reset(database_path: &Path) -> Result, DbError> { + let replaced = set_aside(database_path)?; + + tracing::warn!( + replaced = ?replaced.as_ref().map(|path| path.display().to_string()), + "reset the database" + ); + + Ok(replaced) +} + +#[cfg(test)] +mod tests { + use super::*; + use async_tempfile::TempDir; + use sqlx::SqlitePool; + + async fn pool_at(path: &Path) -> SqlitePool { + SqlitePool::connect(&format!("sqlite://{}?mode=rwc", path.display())) + .await + .expect("open sqlite") + } + + #[tokio::test] + async fn a_snapshot_is_a_usable_database() { + let dir = TempDir::new().await.expect("tempdir"); + let db = dir.dir_path().join("user_data.db"); + + let pool = pool_at(&db).await; + sqlx::query("CREATE TABLE demo (id INTEGER PRIMARY KEY)") + .execute(&pool) + .await + .expect("create"); + + let snapshot = snapshot(&pool, &db).await.expect("snapshot"); + assert!(snapshot.exists()); + + let restored = pool_at(&snapshot).await; + let found: Option = + sqlx::query_scalar("SELECT 1 FROM sqlite_master WHERE name = 'demo'") + .fetch_optional(&restored) + .await + .expect("probe"); + assert!(found.is_some()); + } + + #[tokio::test] + async fn restoring_keeps_the_replaced_database() { + let dir = TempDir::new().await.expect("tempdir"); + let db = dir.dir_path().join("user_data.db"); + + let pool = pool_at(&db).await; + sqlx::query("CREATE TABLE before_snapshot (id INTEGER PRIMARY KEY)") + .execute(&pool) + .await + .expect("create"); + let snapshot = snapshot(&pool, &db).await.expect("snapshot"); + + sqlx::query("CREATE TABLE after_snapshot (id INTEGER PRIMARY KEY)") + .execute(&pool) + .await + .expect("create"); + pool.close().await; + + let replaced = restore(&db, &snapshot).expect("restore").expect("moved"); + + let now = pool_at(&db).await; + let after: Option = + sqlx::query_scalar("SELECT 1 FROM sqlite_master WHERE name = 'after_snapshot'") + .fetch_optional(&now) + .await + .expect("probe"); + assert!(after.is_none(), "the snapshot should be what is live now"); + + let kept = pool_at(&replaced).await; + let after: Option = + sqlx::query_scalar("SELECT 1 FROM sqlite_master WHERE name = 'after_snapshot'") + .fetch_optional(&kept) + .await + .expect("probe"); + assert!(after.is_some(), "the replaced database must still be intact"); + } + + #[tokio::test] + async fn a_reset_moves_the_database_rather_than_deleting_it() { + let dir = TempDir::new().await.expect("tempdir"); + let db = dir.dir_path().join("user_data.db"); + + let pool = pool_at(&db).await; + sqlx::query("CREATE TABLE demo (id INTEGER PRIMARY KEY)") + .execute(&pool) + .await + .expect("create"); + pool.close().await; + + let moved = reset(&db).expect("reset").expect("moved"); + + assert!(!db.exists()); + assert!(moved.exists()); + } + + #[tokio::test] + async fn a_set_aside_file_is_not_offered_as_a_snapshot() { + let dir = TempDir::new().await.expect("tempdir"); + let db = dir.dir_path().join("user_data.db"); + + let pool = pool_at(&db).await; + sqlx::query("CREATE TABLE demo (id INTEGER PRIMARY KEY)") + .execute(&pool) + .await + .expect("create"); + let snapshot = snapshot(&pool, &db).await.expect("snapshot"); + pool.close().await; + + reset(&db).expect("reset"); + + assert_eq!(list(&db), vec![snapshot]); + } + + #[tokio::test] + async fn pruning_keeps_the_newest() { + let dir = TempDir::new().await.expect("tempdir"); + let db = dir.dir_path().join("user_data.db"); + let backups = self::dir(&db); + std::fs::create_dir_all(&backups).expect("mkdir"); + + for stamp in ["20260101-000000", "20260102-000000", "20260103-000000"] { + std::fs::write(backups.join(format!("user_data-{stamp}.db")), b"").expect("write"); + } + + prune(&db, 2); + + let left = list(&db); + assert_eq!(left.len(), 2); + assert!( + left[0].ends_with("user_data-20260103-000000.db"), + "the newest must survive" + ); + } +} diff --git a/packages/oneclient_db/src/lib.rs b/packages/oneclient_db/src/lib.rs index 67096aa7..b56b5a58 100644 --- a/packages/oneclient_db/src/lib.rs +++ b/packages/oneclient_db/src/lib.rs @@ -1,6 +1,8 @@ mod error; +mod migrate; mod pool; +pub mod backup; pub mod console; pub mod dao; pub mod models; diff --git a/packages/oneclient_db/src/migrate.rs b/packages/oneclient_db/src/migrate.rs new file mode 100644 index 00000000..abc6f213 --- /dev/null +++ b/packages/oneclient_db/src/migrate.rs @@ -0,0 +1,133 @@ +use std::path::Path; + +use sqlx::migrate::{MigrateError, Migrator}; + +use crate::{DbError, DbPool, backup}; + +const MAX_ATTEMPTS: u32 = 3; + +pub async fn run(pool: &DbPool, database_path: &Path) -> Result<(), DbError> { + let mut migrator = sqlx::migrate!(); + migrator.set_ignore_missing(true); + + if has_pending(pool, &migrator).await { + match backup::snapshot(pool, database_path).await { + Ok(path) => { + tracing::info!(path = %path.display(), "snapshotted the database before migrating"); + backup::prune(database_path, backup::KEEP); + } + // A migration that then succeeds is the common case, so a failed + // snapshot is not worth refusing to start over + Err(err) => tracing::warn!(%err, "could not snapshot the database before migrating"), + } + } + + let mut attempt = 1; + loop { + let error = match migrator.run(pool).await { + Ok(()) => return Ok(()), + Err(error) => error, + }; + + if attempt >= MAX_ATTEMPTS || !matches!(error, MigrateError::ExecuteMigration(..)) { + return Err(error.into()); + } + + tracing::warn!(attempt, %error, "migration failed, retrying"); + attempt += 1; + } +} + +/// False on a fresh database, where there is nothing yet worth snapshotting +async fn has_pending(pool: &DbPool, migrator: &Migrator) -> bool { + let applied: Vec = sqlx::query_scalar("SELECT version FROM _sqlx_migrations") + .fetch_all(pool) + .await + .unwrap_or_default(); + + !applied.is_empty() && migrator.iter().any(|m| !applied.contains(&m.version)) +} + +#[cfg(test)] +mod tests { + use super::*; + use async_tempfile::TempDir; + use sqlx::SqlitePool; + + async fn migrated() -> (TempDir, std::path::PathBuf, SqlitePool) { + let dir = TempDir::new().await.expect("tempdir"); + let path = dir.dir_path().join("user_data.db"); + let pool = SqlitePool::connect(&format!("sqlite://{}?mode=rwc", path.display())) + .await + .expect("open sqlite"); + + run(&pool, &path).await.expect("migrations run"); + (dir, path, pool) + } + + #[tokio::test] + async fn migrates_a_fresh_database() { + let (_dir, _path, pool) = migrated().await; + + let column: Option = + sqlx::query_scalar("SELECT 1 FROM pragma_table_info('setting_profiles') WHERE name = ?") + .bind("browser_update_mode") + .fetch_optional(&pool) + .await + .expect("probe"); + + assert!(column.is_some()); + } + + #[tokio::test] + async fn running_twice_is_a_no_op() { + let (_dir, path, pool) = migrated().await; + run(&pool, &path).await.expect("second run"); + } + + #[tokio::test] + async fn tolerates_a_migration_this_build_does_not_ship() { + let (_dir, path, pool) = migrated().await; + + sqlx::query( + "INSERT INTO _sqlx_migrations + (version, description, success, checksum, execution_time) + VALUES (?, 'from a newer build', TRUE, x'00', -1)", + ) + .bind(20990101120000i64) + .execute(&pool) + .await + .expect("record a migration from the future"); + + run(&pool, &path).await.expect("a downgrade must still start"); + } + + #[tokio::test] + async fn a_fresh_database_is_not_snapshotted() { + let (_dir, path, _pool) = migrated().await; + assert!(backup::list(&path).is_empty()); + } + + #[tokio::test] + async fn nothing_outstanding_once_migrated() { + let (_dir, _path, pool) = migrated().await; + assert!(!has_pending(&pool, &sqlx::migrate!()).await); + } + + #[tokio::test] + async fn an_unapplied_migration_is_outstanding() { + let (_dir, _path, pool) = migrated().await; + + let latest: i64 = sqlx::query_scalar("SELECT MAX(version) FROM _sqlx_migrations") + .fetch_one(&pool) + .await + .expect("latest version"); + sqlx::query("DELETE FROM _sqlx_migrations WHERE version = ?") + .bind(latest) + .execute(&pool) + .await + .expect("forget it"); + + assert!(has_pending(&pool, &sqlx::migrate!()).await); + } +} diff --git a/packages/oneclient_db/src/pool.rs b/packages/oneclient_db/src/pool.rs index 4a9d1b2a..1cab41c2 100644 --- a/packages/oneclient_db/src/pool.rs +++ b/packages/oneclient_db/src/pool.rs @@ -30,7 +30,7 @@ pub async fn connect(database_path: impl AsRef) -> Result .connect_with(options) .await?; - sqlx::migrate!().run(&pool).await?; + crate::migrate::run(&pool, database_path).await?; tracing::info!("database ready"); diff --git a/packages/oneclient_net/src/error.rs b/packages/oneclient_net/src/error.rs index ee9b53e1..048d33c3 100644 --- a/packages/oneclient_net/src/error.rs +++ b/packages/oneclient_net/src/error.rs @@ -73,17 +73,118 @@ pub enum RequestError { InvalidHeaderValue(#[from] reqwest::header::InvalidHeaderValue), } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NetworkFailure { + Certificate, + Handshake, + Dns, + Generic, +} + +impl NetworkFailure { + #[must_use] + pub fn user_message(self) -> &'static str { + match self { + Self::Certificate => { + "Something on this network or machine is intercepting secure connections, so OneClient cannot verify it is really talking to the servers it needs. This is usually antivirus HTTPS scanning, a school or workplace filter, or a VPN. Try another network, turn off HTTPS/SSL scanning in your antivirus, or ask your network administrator to allow OneClient." + } + Self::Handshake => { + "OneClient reached the server but the two could not agree on a secure connection. A proxy, filter, or antivirus in between is usually using outdated encryption settings. Try another network or turn off HTTPS/SSL scanning in your antivirus." + } + Self::Dns => { + "OneClient could not look up the address of the server. Check your internet connection, disable any VPN or proxy, and try a different DNS server such as 1.1.1.1." + } + Self::Generic => { + "OneClient could not reach the server. Check your internet connection, and any firewall, proxy, or VPN that could be blocking it." + } + } + } + + /// Retrying or waiting cannot clear these the user has to change something + #[must_use] + pub fn is_tls_interception(self) -> bool { + matches!(self, Self::Certificate | Self::Handshake) + } +} + +#[must_use] +pub fn classify_network_failure(err: &reqwest::Error) -> NetworkFailure { + let chain = error_chain(err).to_ascii_lowercase(); + + // Certificate before handshake a rejected certificate also aborts the + // handshake and the certificate wording is the more specific of the two + const CERTIFICATE: &[&str] = &[ + "invalid peer certificate", + "unknownissuer", + "certificate", + "certexpired", + "notvalidforname", + ]; + const HANDSHAKE: &[&str] = &[ + "handshake", + "received fatal alert", + "peer misbehaved", + "no cipher suites in common", + "protocol version", + "unsupported protocol", + ]; + const DNS: &[&str] = &[ + "dns error", + "failed to lookup address", + "no record found", + "nodename nor servname", + "name or service not known", + "no such host", + ]; + + let matches = |markers: &[&str]| markers.iter().any(|marker| chain.contains(marker)); + + if matches(CERTIFICATE) { + NetworkFailure::Certificate + } else if matches(HANDSHAKE) { + NetworkFailure::Handshake + } else if matches(DNS) { + NetworkFailure::Dns + } else { + NetworkFailure::Generic + } +} + impl RequestError { /// Whether this is the user's environment rather than a launcher bug offline - /// connection refused timed out disk full + /// connection refused timed out disk full intercepted TLS #[must_use] pub fn is_transient(&self) -> bool { match self { - Self::ReqwestError(source) => source.is_timeout() || source.is_connect(), + Self::ReqwestError(source) => { + source.is_timeout() + || source.is_connect() + || classify_network_failure(source).is_tls_interception() + } Self::IOError(source) => is_transient_io(source), _ => false, } } + + #[must_use] + pub fn network_failure(&self) -> Option { + let Self::ReqwestError(source) = self else { + return None; + }; + + let failure = classify_network_failure(source); + + if failure != NetworkFailure::Generic || source.is_connect() || source.is_timeout() { + Some(failure) + } else { + None + } + } + + #[must_use] + pub fn user_message(&self) -> Option<&'static str> { + self.network_failure().map(NetworkFailure::user_message) + } } fn is_transient_io(error: &polyio::IOError) -> bool { diff --git a/packages/oneclient_net/src/lib.rs b/packages/oneclient_net/src/lib.rs index ce9d6975..c9ae2b12 100644 --- a/packages/oneclient_net/src/lib.rs +++ b/packages/oneclient_net/src/lib.rs @@ -1,6 +1,3 @@ -//! Endpoints and credentials arrive as data ([`NetConfig`]) so the transport -//! does not depend on the launcher state that owns the client - mod cache; mod config; mod download; @@ -14,7 +11,7 @@ pub mod status; pub use cache::{EtagPolicy, Fetched, commit_etag, fetch_cached}; pub use config::NetConfig; pub use download::{download_verified, fetch_verified, matches_on_disk}; -pub use error::{RequestError, error_chain}; +pub use error::{NetworkFailure, RequestError, classify_network_failure, error_chain}; pub use request::*; pub use response::*; pub use service::RequestClient;