Better handling of id errors from issue API calls

Store the rendered tree information such that it is impossible to
represent a set of data that is incorrect.

techtree-manager/src/collect.rs

@@ -111,22 +111,18 @@ fn element_from_issue(issue: &forgejo_api::structs::Issue) -> anyhow::Result<cra
     let ty_labels: Vec<_> = labels
         .iter()
         .filter_map(|l| l.name.as_deref())
-        .filter(|l| l.starts_with("Type/"))
+        .filter_map(|l| l.strip_prefix("Type/"))
         .collect();
 
-    if ty_labels.len() == 0 {
-        anyhow::bail!("Issue #{issue_number} has no type label!");
-    }
-    if ty_labels.len() > 1 {
-        anyhow::bail!("Issue #{issue_number} has more than one type label!");
-    }
-
-    let ty = ty_labels
-        .first()
-        .unwrap()
-        .strip_prefix("Type/")
-        .unwrap()
-        .to_owned();
+    let ty = match &ty_labels[..] {
+        [ty] => ty.to_string(),
+        [] => {
+            anyhow::bail!("Issue #{issue_number} has no type label!");
+        }
+        [..] => {
+            anyhow::bail!("Issue #{issue_number} has more than one type label!");
+        }
+    };
 
     let has_completed_label = labels
         .iter()

techtree-manager/src/issue.rs

@@ -28,7 +28,7 @@ pub async fn make_bot_comment(
         .await?;
 
     Ok(BotCommentInfo {
-        id: res.id.unwrap(),
+        id: res.id.context("Missing id for the bot comment")?,
         body: initial_message.to_owned(),
     })
 }
@@ -55,12 +55,16 @@ pub async fn find_bot_comment(
     let maybe_bot_comment = comments
         .iter()
         .rev()
-        .find(|comment| comment.user.as_ref().unwrap().id == Some(-2));
+        .find(|comment| comment.user.as_ref().and_then(|u| u.id) == Some(-2));
 
-    Ok(maybe_bot_comment.map(|c| BotCommentInfo {
-        body: c.body.clone().unwrap_or("".to_owned()),
-        id: c.id.unwrap(),
-    }))
+    Ok(maybe_bot_comment
+        .map(|c| -> anyhow::Result<_> {
+            Ok(BotCommentInfo {
+                body: c.body.clone().unwrap_or("".to_owned()),
+                id: c.id.context("Missing id for the bot comment")?,
+            })
+        })
+        .transpose()?)
 }
 
 /// Find existing bot comment or create a new one.
@@ -155,7 +159,8 @@ pub async fn remove_stale_label(ctx: &crate::Context, issue_number: u64) -> anyh
         .iter()
         .filter(|l| l.name.as_deref() == Some("Stale"))
         .next()
-        .map(|l| l.id.unwrap());
+        .map(|l| l.id.ok_or(anyhow::anyhow!("`Stale` label has no ID")))
+        .transpose()?;
 
     if let Some(stale_label_id) = stale_label_id {
         log::info!("Removing `Stale` label from issue #{issue_number}...");

techtree-manager/src/main.rs

@@ -60,14 +60,21 @@ async fn run() -> anyhow::Result<()> {
             "{}/{}",
             meta.issue.repository.owner, meta.issue.repository.name
         ))
-        .unwrap();
+        .with_context(|| {
+            format!("failed building repository URL from GITHUB_SERVER_URL: {server_url}")
+        })?;
 
-    let repo_auth_url = token.as_ref().map(|token| {
-        let mut repo_auth_url = repo_url.clone();
-        repo_auth_url.set_username("forgejo-actions").unwrap();
-        repo_auth_url.set_password(Some(&token)).unwrap();
-        repo_auth_url
-    });
+    let repo_auth_url = token
+        .as_ref()
+        .map(|token| -> Result<_, ()> {
+            let mut repo_auth_url = repo_url.clone();
+            repo_auth_url.set_username("forgejo-actions")?;
+            repo_auth_url.set_password(Some(&token))?;
+            Ok(repo_auth_url)
+        })
+        .transpose()
+        .map_err(|_e| anyhow::anyhow!("Repo URL does not have correct format"))
+        .context("Failed adding auth info to repo URL")?;
 
     let auth = if let Some(token) = token.as_ref() {
         forgejo_api::Auth::Token(token)
@@ -138,7 +145,7 @@ async fn run() -> anyhow::Result<()> {
     }
 
     'issues: for issue in tree.iter_issues() {
-        let subtree = tree.subtree_for_issue(issue).unwrap();
+        let subtree = tree.subtree_for_issue(issue).expect("issue from tree not found in tree");
         let hash = subtree.stable_hash();
 
         let (bot_comment, _is_new) = issue::find_or_make_bot_comment(&ctx, issue)

techtree-manager/src/render.rs

@@ -16,9 +16,19 @@ impl SuccessExt for Command {
 }
 
 pub struct RenderedTree {
-    repo_dir: std::path::PathBuf,
-    dot_file: std::path::PathBuf,
-    svg_file: std::path::PathBuf,
+    pub repo_dir: std::path::PathBuf,
+    pub dot_file_name: std::ffi::OsString,
+    pub svg_file_name: std::ffi::OsString,
+}
+
+impl RenderedTree {
+    pub fn dot_file(&self) -> std::path::PathBuf {
+        self.repo_dir.join(&self.dot_file_name)
+    }
+
+    pub fn svg_file(&self) -> std::path::PathBuf {
+        self.repo_dir.join(&self.svg_file_name)
+    }
 }
 
 pub async fn render(
@@ -27,33 +37,33 @@ pub async fn render(
 ) -> anyhow::Result<RenderedTree> {
     let repo_dir = std::path::PathBuf::from("render-git");
 
-    if repo_dir.is_dir() {
-        log::info!("Found old {repo_dir:?} repository, removing...");
-        std::fs::remove_dir_all(&repo_dir).context("Failed to remove stale render repository")?;
+    let info = RenderedTree {
+        repo_dir,
+        dot_file_name: "techtree.dot".into(),
+        svg_file_name: "techtree.svg".into(),
+    };
+
+    if info.repo_dir.is_dir() {
+        log::info!("Found old {:?} repository, removing...", info.repo_dir);
+        std::fs::remove_dir_all(&info.repo_dir)
+            .context("Failed to remove stale render repository")?;
     }
 
-    std::fs::create_dir(&repo_dir).context("Failed creating directory for rendered graph")?;
-
-    let dot_file = repo_dir.join("techtree.dot");
-    let svg_file = repo_dir.join("techtree.svg");
+    std::fs::create_dir(&info.repo_dir).context("Failed creating directory for rendered graph")?;
 
     let dot_source = tree.to_dot();
-    std::fs::write(&dot_file, dot_source.as_bytes())
+    std::fs::write(&info.dot_file(), dot_source.as_bytes())
         .context("Failed to write `dot` source file")?;
 
     Command::new("dot")
         .args(["-T", "svg"])
         .arg("-o")
-        .arg(&svg_file)
-        .arg(&dot_file)
+        .arg(&info.svg_file())
+        .arg(&info.dot_file())
         .success()
         .context("Failed to generate svg graph from dot source")?;
 
-    Ok(RenderedTree {
-        repo_dir,
-        dot_file,
-        svg_file,
-    })
+    Ok(info)
 }
 
 pub async fn publish(ctx: &crate::Context, rendered: &RenderedTree) -> anyhow::Result<()> {
@@ -84,8 +94,8 @@ pub async fn publish(ctx: &crate::Context, rendered: &RenderedTree) -> anyhow::R
         .arg("-C")
         .arg(&rendered.repo_dir)
         .arg("add")
-        .arg(rendered.dot_file.strip_prefix(&rendered.repo_dir).unwrap())
-        .arg(rendered.svg_file.strip_prefix(&rendered.repo_dir).unwrap())
+        .arg(&rendered.dot_file_name)
+        .arg(&rendered.svg_file_name)
         .success()
         .context("Failed to add generated graph files to git index")?;
 

techtree-manager/src/tree.rs

@@ -165,8 +165,12 @@ impl Tree {
     }
 
     pub fn add_dependency_by_issue_number(&mut self, dependant: u64, dependency: u64) {
-        let a = self.find_element_by_issue_number(dependant).unwrap();
-        let b = self.find_element_by_issue_number(dependency).unwrap();
+        let a = self
+            .find_element_by_issue_number(dependant)
+            .expect("dependant element does not exist");
+        let b = self
+            .find_element_by_issue_number(dependency)
+            .expect("dependency element does not exist");
         self.graph.add_edge(a, b, ());
     }
 
@@ -422,7 +426,8 @@ impl<'a> Subtree<'a> {
             .collect();
         edges.sort();
 
-        let json_data = serde_json::ser::to_string(&(nodes, edges, HASH_EPOCH)).unwrap();
+        let json_data = serde_json::ser::to_string(&(nodes, edges, HASH_EPOCH))
+            .expect("serialization for a stable hash failed");
         sha256::digest(json_data)
     }
 }