diff --git a/libimagstore/src/hook/aspect.rs b/libimagstore/src/hook/aspect.rs index 29d8c375..8c80b6cb 100644 --- a/libimagstore/src/hook/aspect.rs +++ b/libimagstore/src/hook/aspect.rs @@ -1,3 +1,5 @@ +use libimagerror::trace::trace_error; + use store::FileLockEntry; use storeid::StoreId; use hook::Hook; @@ -38,34 +40,32 @@ impl Aspect { impl StoreIdAccessor for Aspect { fn access(&self, id: &StoreId) -> HookResult<()> { - use crossbeam; - let accessors : Vec = self.hooks.iter().map(|h| h.accessor()).collect(); if !accessors.iter().all(|a| is_match!(*a, HDA::StoreIdAccess(_))) { return Err(HE::new(HEK::AccessTypeViolation, None)); } - let threads : Vec> = accessors + accessors .iter() - .map(|accessor| { - crossbeam::scope(|scope| { - scope.spawn(|| { - match *accessor { - HDA::StoreIdAccess(accessor) => accessor.access(id), - _ => unreachable!(), - } - .map_err(|_| ()) // TODO: We're losing the error cause here - }) - }) - }) - .map(|i| i.join().map_err(|_| HE::new(HEK::HookExecutionError, None))) - .collect(); + .fold(Ok(()), |acc, accessor| { + acc.and_then(|_| { + let res = match accessor { + &HDA::StoreIdAccess(accessor) => accessor.access(id), + _ => unreachable!(), + }; - threads - .into_iter() - .fold(Ok(()), |acc, elem| { - acc.and_then(|a| { - elem.map(|_| a).map_err(|_| HE::new(HEK::HookExecutionError, None)) + match res { + Ok(res) => Ok(res), + Err(e) => { + if !e.is_aborting() { + trace_error(&e); + // ignore error if it is not aborting, as we printed it already + Ok(()) + } else { + Err(e) + } + } + } }) }) } @@ -83,52 +83,63 @@ impl MutableHookDataAccessor for Aspect { return Err(HE::new(HEK::AccessTypeViolation, None)); } - for accessor in accessors { - match accessor { - HDA::MutableAccess(accessor) => try!(accessor.access_mut(fle)), + // TODO: Naiive implementation. + // More sophisticated version would check whether there are _chunks_ of + // NonMutableAccess accessors and execute these chunks in parallel. We do not have + // performance concerns yet, so this is okay. + accessors.iter().fold(Ok(()), |acc, accessor| { + acc.and_then(|_| { + let res = match accessor { + &HDA::MutableAccess(ref accessor) => accessor.access_mut(fle), + &HDA::NonMutableAccess(ref accessor) => accessor.access(fle), + _ => unreachable!(), + }; - // TODO: Naiive implementation. - // More sophisticated version would check whether there are _chunks_ of - // NonMutableAccess accessors and execute these chunks in parallel. We do not have - // performance concerns yet, so this is okay. - HDA::NonMutableAccess(accessor) => try!(accessor.access(fle)), - _ => unreachable!(), - } - } - Ok(()) + match res { + Ok(res) => Ok(res), + Err(e) => { + if !e.is_aborting() { + trace_error(&e); + // ignore error if it is not aborting, as we printed it already + Ok(()) + } else { + Err(e) + } + } + } + }) + }) } } impl NonMutableHookDataAccessor for Aspect { fn access(&self, fle: &FileLockEntry) -> HookResult<()> { - use crossbeam; - let accessors : Vec = self.hooks.iter().map(|h| h.accessor()).collect(); if !accessors.iter().all(|a| is_match!(*a, HDA::NonMutableAccess(_))) { return Err(HE::new(HEK::AccessTypeViolation, None)); } - let threads : Vec> = accessors + accessors .iter() - .map(|accessor| { - crossbeam::scope(|scope| { - scope.spawn(|| { - match *accessor { - HDA::NonMutableAccess(accessor) => accessor.access(fle), - _ => unreachable!(), - } - .map_err(|_| ()) // TODO: We're losing the error cause here - }) - }) - }) - .map(|i| i.join().map_err(|_| HE::new(HEK::HookExecutionError, None))) - .collect(); + .fold(Ok(()), |acc, accessor| { + acc.and_then(|_| { + let res = match accessor { + &HDA::NonMutableAccess(accessor) => accessor.access(fle), + _ => unreachable!(), + }; - threads - .into_iter() - .fold(Ok(()), |acc, elem| { - acc.and_then(|a| { - elem.map(|_| a).map_err(|_| HE::new(HEK::HookExecutionError, None)) + match res { + Ok(res) => Ok(res), + Err(e) => { + if !e.is_aborting() { + trace_error(&e); + // ignore error if it is not aborting, as we printed it already + Ok(()) + } else { + Err(e) + } + } + } }) }) } diff --git a/libimagstore/src/hook/error.rs b/libimagstore/src/hook/error.rs index afe9ef07..77095844 100644 --- a/libimagstore/src/hook/error.rs +++ b/libimagstore/src/hook/error.rs @@ -1,10 +1,23 @@ -generate_error_module!( - generate_error_types!(HookError, HookErrorKind, - HookExecutionError => "Hook exec error", - AccessTypeViolation => "Hook access type violation" - ); +use std::convert::Into; +generate_error_imports!(); + +generate_custom_error_types!(HookError, HookErrorKind, CustomData, + HookExecutionError => "Hook exec error", + AccessTypeViolation => "Hook access type violation" ); -pub use self::error::HookError; -pub use self::error::HookErrorKind; +#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Copy)] +pub struct CustomData { + aborting: bool, +} +impl HookError { + + pub fn is_aborting(&self) -> bool { + match self.custom_data { + Some(b) => b.aborting, + None => true + } + } + +} diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs index d2c8edfd..36b05a0c 100644 --- a/libimagstore/src/store.rs +++ b/libimagstore/src/store.rs @@ -28,10 +28,13 @@ use storeid::{IntoStoreId, StoreId, StoreIdIterator}; use lazyfile::LazyFile; use hook::aspect::Aspect; +use hook::error::HookErrorKind; +use hook::result::HookResult; use hook::accessor::{ MutableHookDataAccessor, StoreIdAccessor}; use hook::position::HookPosition; use hook::Hook; +use libimagerror::trace::trace_error; use libimagerror::into::IntoError; @@ -301,7 +304,13 @@ impl Store { pub fn create<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { let id = self.storify_id(id.into_storeid()); if let Err(e) = self.execute_hooks_for_id(self.pre_create_aspects.clone(), &id) { - return Err(e); + if e.is_aborting() { + return Err(e) + .map_err(Box::new) + .map_err(|e| SEK::PreHookExecuteError.into_error_with_cause(e)) + } else { + trace_error(&e); + } } let mut hsmap = match self.entries.write() { @@ -332,7 +341,13 @@ impl Store { pub fn retrieve<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { let id = self.storify_id(id.into_storeid()); if let Err(e) = self.execute_hooks_for_id(self.pre_retrieve_aspects.clone(), &id) { - return Err(e); + if e.is_aborting() { + return Err(e) + .map_err(Box::new) + .map_err(|e| SEK::PreHookExecuteError.into_error_with_cause(e)) + } else { + trace_error(&e); + } } self.entries @@ -426,7 +441,13 @@ impl Store { /// Return the `FileLockEntry` and write to disk pub fn update<'a>(&'a self, mut entry: FileLockEntry<'a>) -> Result<()> { if let Err(e) = self.execute_hooks_for_mut_file(self.pre_update_aspects.clone(), &mut entry) { - return Err(e); + if e.is_aborting() { + return Err(e) + .map_err(Box::new) + .map_err(|e| SEK::PreHookExecuteError.into_error_with_cause(e)) + } else { + trace_error(&e); + } } if let Err(e) = self._update(&entry) { @@ -434,6 +455,7 @@ impl Store { } self.execute_hooks_for_mut_file(self.post_update_aspects.clone(), &mut entry) + .map_err(|e| SE::new(SEK::PreHookExecuteError, Some(Box::new(e)))) } /// Internal method to write to the filesystem store. @@ -482,7 +504,13 @@ impl Store { pub fn delete(&self, id: S) -> Result<()> { let id = self.storify_id(id.into_storeid()); if let Err(e) = self.execute_hooks_for_id(self.pre_delete_aspects.clone(), &id) { - return Err(e); + if e.is_aborting() { + return Err(e).map_err(|e| { + SE::new(SEK::PreHookExecuteError, Some(Box::new(e))) + }) + } else { + trace_error(&e); + } } let mut entries = match self.entries.write() { @@ -502,6 +530,7 @@ impl Store { } self.execute_hooks_for_id(self.post_delete_aspects.clone(), &id) + .map_err(|e| SE::new(SEK::PreHookExecuteError, Some(Box::new(e)))) } /// Gets the path where this store is on the disk @@ -566,29 +595,29 @@ impl Store { fn execute_hooks_for_id(&self, aspects: Arc>>, id: &StoreId) - -> Result<()> + -> HookResult<()> { match aspects.lock() { - Err(_) => return Err(SE::new(SEK::PreHookExecuteError, None)), + Err(_) => return Err(HookErrorKind::HookExecutionError.into()), Ok(g) => g }.iter().fold(Ok(()), |acc, aspect| { debug!("[Aspect][exec]: {:?}", aspect); acc.and_then(|_| (aspect as &StoreIdAccessor).access(id)) - }).map_err(|e| SE::new(SEK::PreHookExecuteError, Some(Box::new(e)))) + }).map_err(Box::new).map_err(|e| HookErrorKind::HookExecutionError.into_error_with_cause(e)) } fn execute_hooks_for_mut_file(&self, aspects: Arc>>, fle: &mut FileLockEntry) - -> Result<()> + -> HookResult<()> { match aspects.lock() { - Err(_) => return Err(SE::new(SEK::PreHookExecuteError, None)), + Err(_) => return Err(HookErrorKind::HookExecutionError.into()), Ok(g) => g }.iter().fold(Ok(()), |acc, aspect| { debug!("[Aspect][exec]: {:?}", aspect); acc.and_then(|_| aspect.access_mut(fle)) - }).map_err(|e| SE::new(SEK::PreHookExecuteError, Some(Box::new(e)))) + }).map_err(Box::new).map_err(|e| HookErrorKind::HookExecutionError.into_error_with_cause(e)) } }