d82c077 feat! return errors on fallible errors, instead of panic — radicle-ci-broker

Radicle CI broker

feat! return errors on fallible errors, instead of panic

Lars Wirzenius committed 1 year ago

commit d82c077579a51fabf65932c0119cfe1a7cacff93
parent 07fa94d40627ee6fe8f908c47603c92db5896894

2 files changed +54 -37

modified src/event.rs

@@ -155,6 +155,10 @@ impl NodeEventSource {

 /// Possible errors from accessing the local Radicle node.
 #[derive(Debug, thiserror::Error)]
 pub enum NodeEventError {
     /// Regex compilation error.
     #[error("programming error in regular expression {0:?}")]
     Regex(&'static str, regex::Error),
     /// Node control socket does not exist.
     #[error("node control socket does not exist: {0}")]
     NoControlSocket(PathBuf),

@@ -400,7 +404,7 @@ impl BrokerEvent {

                 trace!("is_allowed: {prefix} old={old:?}");
                 trace!("is_allowed: {prefix} filter={filter:?}");
                 let parsed = parse_ref(name);
                 let parsed = parse_ref(name)?;
                 trace!("is_allowed: {prefix} parsed={parsed:?}");
                 match filter {

@@ -446,36 +450,37 @@ impl BrokerEvent {

     /// Extract the NID from the RefString.
     /// The RefString will start with `refs/namespaces/<nid>/...`
     pub fn nid(&self) -> Option<NodeId> {
     pub fn nid(&self) -> Result<Option<NodeId>, NodeEventError> {
         if let Some(name) = self.name() {
             parse_nid_from_refstring(name)
             Ok(parse_nid_from_refstring(name)?)
         } else {
             None
             Ok(None)
         }
     }
     pub fn patch_id(&self) -> Option<Oid> {
     pub fn patch_id(&self) -> Result<Option<Oid>, NodeEventError> {
         if let Some(name) = self.name() {
             if let Some(ParsedRef::Patch(oid)) = parse_ref(name) {
                 return Some(oid);
             if let Some(ParsedRef::Patch(oid)) = parse_ref(name)? {
                 return Ok(Some(oid));
             }
         }
         None
         Ok(None)
     }
 }
 /// Extract the NID from a the ref string in a repository.
 /// The RefString should start with `refs/namespaces/<nid>/...`
 pub fn parse_nid_from_refstring(name: &RefString) -> Option<NodeId> {
     let pat = Regex::new(r"^refs/namespaces/(?P<nid>[^/]+)/").unwrap();
 pub fn parse_nid_from_refstring(name: &RefString) -> Result<Option<NodeId>, NodeEventError> {
     const PAT: &str = r"^refs/namespaces/(?P<nid>[^/]+)/";
     let pat = Regex::new(PAT).map_err(|e| NodeEventError::Regex(PAT, e))?;
     if let Some(captures) = pat.captures(name.as_str()) {
         if let Some(m) = captures.name("nid") {
             if let Ok(parsed) = m.as_str().parse() {
                 return Some(parsed);
                 return Ok(Some(parsed));
             }
         }
     }
     None
     Ok(None)
 }
 #[cfg(test)]

@@ -506,7 +511,7 @@ mod test_broker_event {

         let name = RefString::try_from("main")?;
         let oid = Oid::from_str("11d03559fb10183b0f14331175f254fbb077159a")?;
         let be = BrokerEvent::new(&rid, &name, &oid, None);
         assert_eq!(be.nid(), None);
         assert_eq!(be.nid()?, None);
         Ok(())
     }

@@ -516,7 +521,7 @@ mod test_broker_event {

         let name = RefString::try_from("something/else/main")?;
         let oid = Oid::from_str("11d03559fb10183b0f14331175f254fbb077159a")?;
         let be = BrokerEvent::new(&rid, &name, &oid, None);
         assert_eq!(be.nid(), None);
         assert_eq!(be.nid()?, None);
         Ok(())
     }

@@ -529,7 +534,7 @@ mod test_broker_event {

         let nid = NodeId::from_str("z6MkgEMYod7Hxfy9qCvDv5hYHkZ4ciWmLFgfvm3Wn1b2w2FV")?;
         let oid = Oid::from_str("11d03559fb10183b0f14331175f254fbb077159a")?;
         let be = BrokerEvent::new(&rid, &name, &oid, None);
         assert_eq!(be.nid(), Some(nid));
         assert_eq!(be.nid()?, Some(nid));
         Ok(())
     }
 }

@@ -558,32 +563,35 @@ pub enum ParsedRef {

 ///     }
 /// }
 /// ```
 pub fn parse_ref(s: &str) -> Option<ParsedRef> {
 pub fn parse_ref(s: &str) -> Result<Option<ParsedRef>, NodeEventError> {
     trace!("parse_ref: s={s:?}");
     let patch_re = Regex::new(r"^refs/namespaces/[^/]+/refs/heads/patches/([^/]+)$").unwrap();
     const PAT_PATCH: &str = r"^refs/namespaces/[^/]+/refs/heads/patches/([^/]+)$";
     let patch_re = Regex::new(PAT_PATCH).map_err(|e| NodeEventError::Regex(PAT_PATCH, e))?;
     if let Some(patch_captures) = patch_re.captures(s) {
         trace!("parse_ref: patch_captures={patch_captures:?}");
         if let Some(patch_id) = patch_captures.get(1) {
             trace!("parse_ref: patch_id={patch_id:?}");
             let patch_id_str = patch_id.as_str();
             let oid = Oid::try_from(patch_id_str).unwrap();
             let oid = Oid::try_from(patch_id_str)
                 .map_err(|e| NodeEventError::ParseOid(patch_id_str.into(), e))?;
             trace!("parse_ref: patch oid={oid:?}");
             return Some(ParsedRef::Patch(oid));
             return Ok(Some(ParsedRef::Patch(oid)));
         }
     }
     let push_re = Regex::new(r"^refs/namespaces/[^/]+/refs/heads/(.+)$").unwrap();
     const PAT_BRANCH: &str = r"^refs/namespaces/[^/]+/refs/heads/(.+)$";
     let push_re = Regex::new(PAT_BRANCH).map_err(|e| NodeEventError::Regex(PAT_BRANCH, e))?;
     if let Some(push_captures) = push_re.captures(s) {
         trace!("parse_ref: push_captures={push_captures:?}");
         if let Some(branch) = push_captures.get(1) {
             trace!("parse_ref: branch={branch:?}");
             return Some(ParsedRef::Push(branch.as_str().to_string()));
             return Ok(Some(ParsedRef::Push(branch.as_str().to_string())));
         }
     }
     trace!("parse_ref: neither push nor patch");
     None
     Ok(None)
 }
 #[cfg(test)]

@@ -600,31 +608,34 @@ mod test_parse_ref {

     use super::{log_init, parse_ref, Oid, ParsedRef};
     #[test]
     fn plain_branch_name_is_none() {
     fn plain_branch_name_is_none() -> anyhow::Result<()> {
         log_init();
         assert_eq!(parse_ref("main"), None);
         assert_eq!(parse_ref("main")?, None);
         Ok(())
     }
     #[test]
     fn namespaced_branch() {
     fn namespaced_branch() -> anyhow::Result<()> {
         log_init();
         assert_eq!(
             parse_ref(
                 "refs/namespaces/z6MkfBU2cwcZfaE6Z1dLqb7Ve7u4pdgbSo9tP6qUVsqFn2xv/refs/heads/main"
             ),
             )?,
             Some(ParsedRef::Push("main".into()))
         );
         Ok(())
     }
     #[test]
     fn namespaced_branch_with_slashes() {
     fn namespaced_branch_with_slashes() -> anyhow::Result<()> {
         log_init();
         assert_eq!(
             parse_ref(
                 "refs/namespaces/z6MkfBU2cwcZfaE6Z1dLqb7Ve7u4pdgbSo9tP6qUVsqFn2xv/refs/heads/liw/cob/draft/v2"
             ),
             )?,
             Some(ParsedRef::Push("liw/cob/draft/v2".into()))
         );
         Ok(())
     }
     #[test]

@@ -632,7 +643,7 @@ mod test_parse_ref {

         log_init();
         const SHA: &str = "0a4c69183fc8b8d849f5ab977d70f2a1f4788bca";
         assert_eq!(
             parse_ref(&format!("refs/namespaces/NID/refs/heads/patches/{SHA}")),
             parse_ref(&format!("refs/namespaces/NID/refs/heads/patches/{SHA}"))?,
             Some(ParsedRef::Patch(Oid::try_from(SHA)?))
         );
         Ok(())

@@ -670,7 +681,7 @@ mod test {

             "refs/namespaces/NID/refs/heads/patches/9183ed6232687d3105482960cecb01a53018b80a";
         assert_eq!(
             parse_ref(patch_ref),
             parse_ref(patch_ref)?,
             Some(ParsedRef::Patch(Oid::try_from(
                 "9183ed6232687d3105482960cecb01a53018b80a"
             )?))

@@ -679,11 +690,11 @@ mod test {

     }
     #[test]
     fn test_parse_push_ref() {
     fn test_parse_push_ref() -> anyhow::Result<()> {
         log_init();
         let push_ref =
             "refs/namespaces/z6MkuhvCnrcow7vzkyQzkuFixzpTa42iC2Cfa4DA8HRLCmys/refs/heads/main";
         let parsed_ref = parse_ref(push_ref);
         let parsed_ref = parse_ref(push_ref)?;
         eprintln!("parsed_ref={parsed_ref:#?}");
         assert!(parsed_ref.is_some());
         if let Some(ref parsed) = parsed_ref {

@@ -692,14 +703,16 @@ mod test {

                 _ => panic!("Expected Push ref"),
             }
         }
         Ok(())
     }
     #[test]
     fn test_parse_invalid_ref() {
     fn test_parse_invalid_ref() -> anyhow::Result<()> {
         log_init();
         let invalid_ref = "invalid_ref";
         let parsed_ref = parse_ref(invalid_ref);
         let parsed_ref = parse_ref(invalid_ref)?;
         assert!(parsed_ref.is_none());
         Ok(())
     }
     #[test]

modified src/msg.rs

@@ -153,7 +153,7 @@ impl<'a> RequestBuilder<'a> {

             }
         };
         match parse_ref(name) {
         match parse_ref(name)? {
             None => Err(MessageError::NoEventHandler),
             Some(ParsedRef::Patch(_oid)) => Ok(Request::Trigger {
                 common: EventCommonFields {

@@ -192,7 +192,7 @@ impl<'a> RequestBuilder<'a> {

         profile: &Profile,
         author: Author,
     ) -> Result<PatchEvent, MessageError> {
         let patch_id = event.patch_id().ok_or(MessageError::Trigger)?;
         let patch_id = event.patch_id()?.ok_or(MessageError::Trigger)?;
         let patch = patch::Patches::open(&repository)?
             .get(&patch_id.into())?
             .ok_or(MessageError::Trigger)?;

@@ -369,7 +369,7 @@ fn did_to_author(profile: &Profile, did: &Did) -> Result<Author, MessageError> {

 }
 fn extract_author(profile: &Profile, event: &BrokerEvent) -> Result<Author, MessageError> {
     let nid = match event.nid() {
     let nid = match event.nid()? {
         Some(nid) => nid,
         None => {
             return Err(MessageError::Trigger);

@@ -741,6 +741,10 @@ pub enum MessageError {

     #[error(transparent)]
     StorageError(#[from] radicle::storage::Error),
     /// Error from event module.
     #[error(transparent)]
     EventError(#[from] crate::event::NodeEventError),
     /// Error from Radicle repository.
     #[error(transparent)]
     RepositoryError(#[from] radicle::storage::RepositoryError),