#95: Include identifier in error messages for secondary manifests

mtkennerly · Dec 21, 2023 · 807e4a9 · 807e4a9
1 parent 789cd7e
commit 807e4a9
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 20 deletions.
diff --git 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
@@ -79,8 +79,11 @@ pub enum SyncDirection {
 pub enum Error {
     ManifestInvalid {
         why: String,
+        identifier: Option<String>,
+    },
+    ManifestCannotBeUpdated {
+        identifier: Option<String>,
     },
-    ManifestCannotBeUpdated,
     ConfigInvalid {
         why: String,
     },

diff --git 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
@@ -299,7 +299,10 @@ impl Manifest {
     }
 
     pub fn load() -> Result<Self, Error> {
-        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<Option<ManifestUpdate>, 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()),
         }
     }