Handle errors in file watching

common/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "veloren-common"
 version = "0.3.0"
-authors = ["Joshua Barretto <joshua.s.barretto@gmail.com>", "Maciej Ćwięka <mckol363@gmail.com>"]
+authors = ["Joshua Barretto <joshua.s.barretto@gmail.com>", "Maciej Ćwięka <mckol363@gmail.com>, Imbris <imbrisf@gmail.com>"]
 edition = "2018"
 
 [dependencies]

common/src/assets/mod.rs

@@ -5,6 +5,7 @@ use dot_vox::DotVoxData;
 use hashbrown::HashMap;
 use image::DynamicImage;
 use lazy_static::lazy_static;
+use log::error;
 use serde_json::Value;
 use std::{
     any::Any,
@@ -101,6 +102,8 @@ pub fn load_watched<A: Asset + 'static>(
     specifier: &str,
     indicator: &mut watch::ReloadIndicator,
 ) -> Result<Arc<A>, Error> {
+    let asset = load(specifier)?;
+
     // Determine path to watch
     let mut path = unpack_specifier(specifier);
     let mut file_exists = false;
@@ -117,13 +120,14 @@ pub fn load_watched<A: Asset + 'static>(
         return Err(Error::NotFound(path.to_string_lossy().into_owned()));
     }
 
-    // Start watching first to detect any changes while the file is being loaded
     let owned_specifier = specifier.to_string();
-    indicator.add(path, move || {
+    indicator.add(specifier, move || {
-        // TODO: handle result
+        if let Err(err) = reload::<A>(&owned_specifier) {
-        reload::<A>(&owned_specifier);
+            error!("Error reloading {}: {:#?}", &owned_specifier, err);
+        }
     });
-    load(specifier)
+
+    Ok(asset)
 }
 
 /// The Asset trait, which is implemented by all structures that have their data stored in the

common/src/assets/watch.rs

@@ -1,7 +1,7 @@
 use crossbeam::channel::{select, unbounded, Receiver, Sender};
 use lazy_static::lazy_static;
 use log::warn;
-use notify::{Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher as _};
+use notify::{event::Flag, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher as _};
 use std::{
     collections::HashMap,
     path::PathBuf,
@@ -16,7 +16,7 @@ use std::{
 type Handler = Box<dyn Fn() + Send>;
 
 lazy_static! {
-    static ref WATCHER: Mutex<Sender<(PathBuf, Handler, Weak<AtomicBool>)>> =
+    static ref WATCHER_TX: Mutex<Sender<(PathBuf, Handler, Weak<AtomicBool>)>> =
         Mutex::new(Watcher::new().run());
 }
 
@@ -47,14 +47,19 @@ impl Watcher {
                 }
             }
             None => {
-                // TODO handle this result
+                if let Err(err) = self.watcher.watch(path.clone(), RecursiveMode::Recursive) {
-                self.watcher.watch(path.clone(), RecursiveMode::Recursive);
+                    warn!("Could not start watching {:#?} due to: {}", &path, err);
+                    return;
+                }
                 self.watching.insert(path, (handler, vec![signal]));
             }
         }
     }
     fn handle_event(&mut self, event: Event) {
-        // TODO: consider using specific modify variant
+        // Skip notice events
+        if let Some(Flag::Notice) = event.flag() {
+            return;
+        }
         if let Event {
             kind: EventKind::Modify(_),
             paths,
@@ -78,15 +83,17 @@ impl Watcher {
                         }
                         // If there is no one to signal stop watching this path
                         if signals.is_empty() {
-                            // TODO: handle this result
+                            if let Err(err) = self.watcher.unwatch(&path) {
-                            self.watcher.unwatch(&path);
+                                warn!("Error unwatching: {}", err);
+                            }
                             self.watching.remove(&path);
                         }
                     }
                     None => {
                         warn!("Watching {:#?} but there are no signals for this path. The path will be unwatched.", path);
-                        // TODO: handle this result
+                        if let Err(err) = self.watcher.unwatch(&path) {
-                        self.watcher.unwatch(path);
+                            warn!("Error unwatching: {}", err);
+                        }
                     }
                 }
             }
@@ -97,7 +104,6 @@ impl Watcher {
 
         thread::spawn(move || {
             loop {
-                // TODO: handle errors
                 select! {
                     recv(watch_rx) -> res => match res {
                         Ok((path, handler, signal)) => self.watch(path, handler, signal),
@@ -107,7 +113,7 @@ impl Watcher {
                     recv(self.event_rx) -> res => match res {
                         Ok(Ok(event)) => self.handle_event(event),
                         // Notify Error
-                        Ok(Err(_)) => (),
+                        Ok(Err(err)) => error!("Notify error: {}", err),
                         // Disconnected
                         Err(_) => (),
                     },
@@ -143,11 +149,17 @@ impl ReloadIndicator {
             self.paths.push(path.clone());
         };
 
-        // TODO: handle result
+        let mut path = super::ASSETS_PATH.clone();
-        WATCHER
+        path.push(specifier);
+
+        if WATCHER_TX
             .lock()
             .unwrap()
-            .send((path, Box::new(reloader), Arc::downgrade(&self.reloaded)));
+            .send((path, Box::new(reloader), Arc::downgrade(&self.reloaded)))
+            .is_err()
+        {
+            error!("Could not add. Asset watcher channel disconnected.");
+        }
     }
     // Returns true if the watched file was changed
     pub fn reloaded(&self) -> bool {