From 807e4a9e6c01555634a33cbd557cf0c48ac58c66 Mon Sep 17 00:00:00 2001 From: mtkennerly Date: Thu, 21 Dec 2023 16:14:17 +0800 Subject: [PATCH] #95: Include identifier in error messages for secondary manifests --- src/lang.rs | 16 ++++++++++------ src/prelude.rs | 5 ++++- src/resource/config.rs | 14 ++++++++++---- src/resource/manifest.rs | 28 +++++++++++++++++++--------- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/lang.rs b/src/lang.rs index 0bd750a9..f217da21 100644 --- a/src/lang.rs +++ b/src/lang.rs @@ -293,8 +293,8 @@ impl Translator { pub fn handle_error(&self, error: &Error) -> String { match error { Error::ConfigInvalid { why } => self.config_is_invalid(why), - Error::ManifestInvalid { why } => self.manifest_is_invalid(why), - Error::ManifestCannotBeUpdated => self.manifest_cannot_be_updated(), + Error::ManifestInvalid { why, identifier } => self.manifest_is_invalid(why, identifier.as_deref()), + Error::ManifestCannotBeUpdated { identifier } => self.manifest_cannot_be_updated(identifier.as_deref()), Error::CliUnrecognizedGames { games } => self.cli_unrecognized_games(games), Error::CliUnableToRequestConfirmation => self.cli_unable_to_request_confirmation(), Error::CliBackupIdWithMultipleGames => self.cli_backup_id_with_multiple_games(), @@ -706,12 +706,16 @@ impl Translator { format!("{}\n{}", translate("config-is-invalid"), why) } - pub fn manifest_is_invalid(&self, why: &str) -> String { - format!("{}\n{}", translate("manifest-is-invalid"), why) + pub fn manifest_is_invalid(&self, why: &str, identifier: Option<&str>) -> String { + let message = translate("manifest-is-invalid"); + let identifier = identifier.map(|x| format!(" ({})", x)).unwrap_or("".to_string()); + format!("{}{}\n{}", message, identifier, why) } - pub fn manifest_cannot_be_updated(&self) -> String { - translate("manifest-cannot-be-updated") + pub fn manifest_cannot_be_updated(&self, identifier: Option<&str>) -> String { + let message = translate("manifest-cannot-be-updated"); + let identifier = identifier.map(|x| format!(" ({})", x)).unwrap_or("".to_string()); + format!("{}{}", message, identifier) } pub fn cannot_prepare_backup_target(&self, target: &StrictPath) -> String { diff --git a/src/prelude.rs b/src/prelude.rs index 75655f00..fc57abe8 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -79,8 +79,11 @@ pub enum SyncDirection { pub enum Error { ManifestInvalid { why: String, + identifier: Option, + }, + ManifestCannotBeUpdated { + identifier: Option, }, - ManifestCannotBeUpdated, ConfigInvalid { why: String, }, diff --git a/src/resource/config.rs b/src/resource/config.rs index 1c78212a..ef0de4f5 100644 --- a/src/resource/config.rs +++ b/src/resource/config.rs @@ -76,13 +76,19 @@ impl ManifestConfig { .iter() .filter_map(|x| match x { SecondaryManifestConfig::Local { path } => { - let manifest = Manifest::load_from_existing(&path.as_std_path_buf()).ok()?; - Some((path.clone(), manifest)) + let manifest = Manifest::load_from_existing(&path.as_std_path_buf()); + if let Err(e) = &manifest { + log::error!("Cannot load secondary manifest: {} | {}", path.render(), e); + } + Some((path.clone(), manifest.ok()?)) } SecondaryManifestConfig::Remote { url } => { let path = Manifest::path_for(url, false); - let manifest = Manifest::load_from_existing(&path.as_std_path_buf()).ok()?; - Some((path.clone(), manifest)) + let manifest = Manifest::load_from(&path.as_std_path_buf()); + if let Err(e) = &manifest { + log::error!("Cannot load manifest: {} | {}", path.render(), e); + } + Some((path.clone(), manifest.ok()?)) } }) .collect() diff --git a/src/resource/manifest.rs b/src/resource/manifest.rs index 7a899245..ee127e40 100644 --- a/src/resource/manifest.rs +++ b/src/resource/manifest.rs @@ -299,7 +299,10 @@ impl Manifest { } pub fn load() -> Result { - ResourceFile::load().map_err(|e| Error::ManifestInvalid { why: format!("{}", e) }) + ResourceFile::load().map_err(|e| Error::ManifestInvalid { + why: format!("{}", e), + identifier: None, + }) } pub fn should_update(url: &str, cache: &cache::Manifests, force: bool, primary: bool) -> bool { @@ -342,6 +345,11 @@ impl Manifest { force: bool, primary: bool, ) -> Result, Error> { + let identifier = (!primary).then(|| url.to_string()); + let cannot_update = || Error::ManifestCannotBeUpdated { + identifier: identifier.clone(), + }; + if !Self::should_update(url, cache, force, primary) { return Ok(None); } @@ -355,21 +363,23 @@ impl Manifest { req = req.header(reqwest::header::IF_NONE_MATCH, etag); } } - let mut res = req.send().map_err(|_e| Error::ManifestCannotBeUpdated)?; + let mut res = req.send().map_err(|_e| cannot_update())?; match res.status() { reqwest::StatusCode::OK => { - std::fs::create_dir_all(app_dir()).map_err(|_| Error::ManifestCannotBeUpdated)?; + std::fs::create_dir_all(app_dir()).map_err(|_| cannot_update())?; // Ensure that the manifest data is valid before we save it. let mut manifest_bytes = vec![]; - res.copy_to(&mut manifest_bytes) - .map_err(|_| Error::ManifestCannotBeUpdated)?; - let manifest_string = String::from_utf8(manifest_bytes).map_err(|_| Error::ManifestCannotBeUpdated)?; + res.copy_to(&mut manifest_bytes).map_err(|_| cannot_update())?; + let manifest_string = String::from_utf8(manifest_bytes).map_err(|_| cannot_update())?; if let Err(e) = Self::load_from_string(&manifest_string) { - return Err(Error::ManifestInvalid { why: e.to_string() }); + return Err(Error::ManifestInvalid { + why: e.to_string(), + identifier: identifier.clone(), + }); } - std::fs::write(path.as_std_path_buf(), manifest_string).map_err(|_| Error::ManifestCannotBeUpdated)?; + std::fs::write(path.as_std_path_buf(), manifest_string).map_err(|_| cannot_update())?; let new_etag = res .headers() @@ -389,7 +399,7 @@ impl Manifest { timestamp: chrono::offset::Utc::now(), modified: false, })), - _ => Err(Error::ManifestCannotBeUpdated), + _ => Err(cannot_update()), } }