chore: improve error messages printed by updater (#121)

* fix: us OSLog framework for logging on iOS

* cleanup

* chore: improve error messages printed by updater

* remove println

* cleanup

* warn -> info

* update set_config to not return anything

* revert pub test struct

* Update library/src/config.rs

Co-authored-by: Eric Seidel <eric@shorebird.dev>

---------

Co-authored-by: Eric Seidel <eric@shorebird.dev>

library/src/c_api/mod.rs

@@ -541,7 +541,10 @@ mod test {
         let c_params = parameters(&tmp_dir, fake_libapp_path.to_str().unwrap());
         // app_id is required or shorebird_init will fail.
         let c_yaml = c_string("app_id: bar");
-        assert!(!shorebird_init(&c_params, FileCallbacks::new(), c_yaml));
+
+        // This will return true, but log a warning and not change the already-
+        // set config.
+        assert!(shorebird_init(&c_params, FileCallbacks::new(), c_yaml));
         free_c_string(c_yaml);
         free_parameters(c_params);
     }

library/src/cache/updater_state.rs

@@ -107,7 +107,7 @@ impl UpdaterState {
             Ok(mut loaded) => {
                 if loaded.serialized_state.release_version != release_version {
                     info!(
-                        "release_version changed {} -> {}, clearing updater state",
+                        "release_version changed {} -> {}, creating new state",
                         loaded.serialized_state.release_version, release_version
                     );
                     let _ = loaded.patch_manager.reset();
@@ -117,7 +117,7 @@ impl UpdaterState {
             }
             Err(e) => {
                 if !is_file_not_found(&e) {
-                    warn!("Error loading state: {:#}, clearing state.", e);
+                    info!("No existing state file found: {:#}, creating new state.", e);
                 }
                 Self::create_new_and_save(storage_dir, release_version)
             }

library/src/config.rs

@@ -11,7 +11,7 @@ use std::sync::Mutex;
 
 // https://stackoverflow.com/questions/67087597/is-it-possible-to-use-rusts-log-info-for-tests
 #[cfg(test)]
-use std::println as debug; // Workaround to use println! for logs.
+use std::{println as warn, println as debug}; // Workaround to use println! for logs.
 
 // cbindgen looks for const, ignore these so it doesn't warn about them.
 
@@ -95,9 +95,15 @@ pub fn set_config(
     libapp_path: PathBuf,
     yaml: &YamlConfig,
     network_hooks: NetworkHooks,
-) -> anyhow::Result<()> {
-    with_config_mut(|config| {
-        anyhow::ensure!(config.is_none(), "shorebird_init has already been called.");
+) {
+    with_config_mut(|config: &mut Option<UpdateConfig>| {
+        if config.is_some() {
+            // This previously returned an error, but this happens regularly
+            // with apps that use Firebase Messaging, and logging it as an error
+            // has caused confusion.
+            warn!("Updater already initialized, ignoring second shorebird_init call.");
+            return;
+        }
 
         let mut code_cache_path = std::path::PathBuf::from(&app_config.code_cache_dir);
         code_cache_path.push("downloads");
@@ -125,9 +131,7 @@ pub fn set_config(
         };
         debug!("Updater configured with: {:?}", new_config);
         *config = Some(new_config);
-
-        Ok(())
-    })
+    });
 }
 
 // Arch/Platform names need to be kept in sync with the shorebird cli.
@@ -156,3 +160,61 @@ pub fn current_platform() -> &'static str {
     static PLATFORM: &str = "ios";
     PLATFORM
 }
+
+#[cfg(test)]
+mod tests {
+    use super::set_config;
+    use crate::{network::NetworkHooks, testing_reset_config, AppConfig, ExternalFileProvider};
+    use serial_test::serial;
+
+    #[derive(Debug, Clone)]
+    pub struct FakeExternalFileProvider {}
+    impl ExternalFileProvider for FakeExternalFileProvider {
+        fn open(&self) -> anyhow::Result<Box<dyn crate::ReadSeek>> {
+            Ok(Box::new(std::io::Cursor::new(vec![])))
+        }
+    }
+
+    fn fake_app_config() -> AppConfig {
+        AppConfig {
+            app_storage_dir: "/tmp".to_string(),
+            code_cache_dir: "/tmp".to_string(),
+            release_version: "1.0.0".to_string(),
+            original_libapp_paths: vec!["libapp.so".to_string()],
+        }
+    }
+
+    fn fake_yaml() -> crate::yaml::YamlConfig {
+        crate::yaml::YamlConfig {
+            app_id: "fake_app_id".to_string(),
+            channel: Some("fake_channel".to_string()),
+            auto_update: Some(true),
+            base_url: Some("fake_base_url".to_string()),
+        }
+    }
+
+    // These tests are serial because they modify global state.
+    #[serial]
+    #[test]
+    fn set_config_is_noop_on_subsequent_calls() {
+        testing_reset_config();
+
+        set_config(
+            fake_app_config(),
+            Box::new(FakeExternalFileProvider {}),
+            "first_path".into(),
+            &fake_yaml(),
+            NetworkHooks::default(),
+        );
+        set_config(
+            fake_app_config(),
+            Box::new(FakeExternalFileProvider {}),
+            "second_path".into(),
+            &fake_yaml(),
+            NetworkHooks::default(),
+        );
+
+        let config = super::with_config(|config| Ok(config.clone())).unwrap();
+        assert_eq!(config.libapp_path.to_str(), Some("first_path"));
+    }
+}

library/src/updater.rs

@@ -135,8 +135,9 @@ pub fn init(
         libapp_path,
         &config,
         NetworkHooks::default(),
-    )
-    .map_err(|err| UpdateError::InvalidState(err.to_string()))
+    );
+
+    Ok(())
 }
 
 pub fn should_auto_update() -> anyhow::Result<bool> {