Radish alpha
r
Radicle CI broker
Radicle
Git (anonymous pull)
Log in to clone via SSH
refactor(src/event.rs): handle errors without panic
Lars Wirzenius committed 1 year ago
commit 07fa94d40627ee6fe8f908c47603c92db5896894
parent 89f88e92182ca68c8a7ad89cad4ec3b15d8b3af3
1 file changed +27 -15
modified src/event.rs
@@ -91,15 +91,15 @@ impl NodeEventSource {
        self.allowed.push(filter);
    }

-
    fn allowed(&self, event: &BrokerEvent) -> bool {
+
    fn allowed(&self, event: &BrokerEvent) -> Result<bool, NodeEventError> {
        for filter in self.allowed.iter() {
-
            if !event.is_allowed(filter) {
+
            if !event.is_allowed(filter)? {
                trace!("event is not allowed");
-
                return false;
+
                return Ok(false);
            }
        }
        trace!("event is allowed");
-
        true
+
        Ok(true)
    }

    /// Get the allowed next event from an event source. This will
@@ -117,7 +117,7 @@ impl NodeEventSource {
                        if let Some(broker_events) = BrokerEvent::from_event(&event) {
                            info!("node event became {} broker events", broker_events.len());
                            for e in broker_events {
-
                                if self.allowed(&e) {
+
                                if self.allowed(&e)? {
                                    info!("allowed event {e:?}");
                                    result.push(e);
                                }
@@ -190,6 +190,10 @@ pub enum NodeEventError {
    /// An error parsing JSON as filters, from an in-memory string.
    #[error("failed to parser filters as JSON")]
    FiltersJsonString(#[source] serde_json::Error),
+

+
    /// An error parsing a Git object id as string into an Oid.
+
    #[error("failed to parse string as a Git object id: {0:?}")]
+
    ParseOid(String, #[source] radicle::git::raw::Error),
}

/// An event filter for allowing events. Or an "AND" combination of events.
@@ -366,14 +370,18 @@ impl BrokerEvent {
    }

    /// Is this broker event allowed by a filter?
-
    fn is_allowed(&self, filter: &EventFilter) -> bool {
+
    fn is_allowed(&self, filter: &EventFilter) -> Result<bool, NodeEventError> {
        debug!("is_allowed called: filter={filter:?}");
-
        let res = self.is_allowed_helper(filter, 0);
+
        let res = self.is_allowed_helper(filter, 0)?;
        debug!("is_allowed: res={res}");
-
        res
+
        Ok(res)
    }

-
    fn is_allowed_helper(&self, filter: &EventFilter, level: usize) -> bool {
+
    fn is_allowed_helper(
+
        &self,
+
        filter: &EventFilter,
+
        level: usize,
+
    ) -> Result<bool, NodeEventError> {
        let prefix = format!("{:width$}", " ", width = level * 4);

        trace!("is_allowed: {prefix} called {self:?}");
@@ -401,28 +409,32 @@ impl BrokerEvent {
                    EventFilter::Branch(wanted) => parsed == Some(ParsedRef::Push(wanted.into())),
                    EventFilter::AnyPatch => matches!(parsed, Some(ParsedRef::Patch(_))),
                    EventFilter::Patch(wanted) => {
-
                        parsed == Some(ParsedRef::Patch(Oid::try_from(wanted.as_str()).unwrap()))
+
                        let oid = Oid::try_from(wanted.as_str())
+
                            .map_err(|e| NodeEventError::ParseOid(wanted.into(), e))?;
+
                        parsed == Some(ParsedRef::Patch(oid))
                    }
                    EventFilter::AnyPatchRef => matches!(parsed, Some(ParsedRef::Patch(_))),
                    EventFilter::AnyPushRef => matches!(parsed, Some(ParsedRef::Push(_))),
                    EventFilter::PatchRef(wanted) => {
-
                        parsed == Some(ParsedRef::Patch(Oid::try_from(wanted.as_str()).unwrap()))
+
                        let oid = Oid::try_from(wanted.as_str())
+
                            .map_err(|e| NodeEventError::ParseOid(wanted.into(), e))?;
+
                        parsed == Some(ParsedRef::Patch(oid))
                    }
                    EventFilter::And(conds) => conds
                        .iter()
-
                        .all(|cond| self.is_allowed_helper(cond, level + 1)),
+
                        .all(|cond| self.is_allowed_helper(cond, level + 1).unwrap_or(false)),
                    EventFilter::Or(conds) => conds
                        .iter()
-
                        .any(|cond| self.is_allowed_helper(cond, level + 1)),
+
                        .any(|cond| self.is_allowed_helper(cond, level + 1).unwrap_or(false)),
                    EventFilter::Not(conds) => !conds
                        .iter()
-
                        .any(|cond| self.is_allowed_helper(cond, level + 1)),
+
                        .any(|cond| self.is_allowed_helper(cond, level + 1).unwrap_or(false)),
                }
            }
        };

        trace!("is_allowed: {prefix} allowed={allowed}");
-
        allowed
+
        Ok(allowed)
    }

    pub fn name(&self) -> Option<&RefString> {