Compare commits

...

6 commits

Author SHA1 Message Date
c11026c5be Better handling of id errors from issue API calls
All checks were successful
/ build (push) Successful in 1m7s
2025-12-09 16:27:00 +01:00
83a610134c Add better panic messages where relevant 2025-12-09 16:27:00 +01:00
d10b164b61 Properly catch errors during URL building 2025-12-09 16:27:00 +01:00
3d73094192 Refactor rendering code to prevent illegal states
Store the rendered tree information such that it is impossible to
represent a set of data that is incorrect.
2025-12-09 16:27:00 +01:00
6d6fffcb4a Reduce error potentials in type label parsing 2025-12-09 15:33:57 +01:00
b66e8c8d9c Replace dirty unwrap() with expect("TODO")
This makes it obvious where better error handling is still needed.
2025-12-09 15:29:21 +01:00
5 changed files with 75 additions and 52 deletions

View file

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

View file

@ -28,7 +28,7 @@ pub async fn make_bot_comment(
.await?; .await?;
Ok(BotCommentInfo { Ok(BotCommentInfo {
id: res.id.unwrap(), id: res.id.context("Missing id for the bot comment")?,
body: initial_message.to_owned(), body: initial_message.to_owned(),
}) })
} }
@ -55,12 +55,16 @@ pub async fn find_bot_comment(
let maybe_bot_comment = comments let maybe_bot_comment = comments
.iter() .iter()
.rev() .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 { Ok(maybe_bot_comment
body: c.body.clone().unwrap_or("".to_owned()), .map(|c| -> anyhow::Result<_> {
id: c.id.unwrap(), 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. /// 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() .iter()
.filter(|l| l.name.as_deref() == Some("Stale")) .filter(|l| l.name.as_deref() == Some("Stale"))
.next() .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 { if let Some(stale_label_id) = stale_label_id {
log::info!("Removing `Stale` label from issue #{issue_number}..."); log::info!("Removing `Stale` label from issue #{issue_number}...");

View file

@ -60,14 +60,21 @@ async fn run() -> anyhow::Result<()> {
"{}/{}", "{}/{}",
meta.issue.repository.owner, meta.issue.repository.name 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 repo_auth_url = token
let mut repo_auth_url = repo_url.clone(); .as_ref()
repo_auth_url.set_username("forgejo-actions").unwrap(); .map(|token| -> Result<_, ()> {
repo_auth_url.set_password(Some(&token)).unwrap(); let mut repo_auth_url = repo_url.clone();
repo_auth_url 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() { let auth = if let Some(token) = token.as_ref() {
forgejo_api::Auth::Token(token) forgejo_api::Auth::Token(token)
@ -138,7 +145,7 @@ async fn run() -> anyhow::Result<()> {
} }
'issues: for issue in tree.iter_issues() { '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 hash = subtree.stable_hash();
let (bot_comment, _is_new) = issue::find_or_make_bot_comment(&ctx, issue) let (bot_comment, _is_new) = issue::find_or_make_bot_comment(&ctx, issue)

View file

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

View file

@ -165,8 +165,12 @@ impl Tree {
} }
pub fn add_dependency_by_issue_number(&mut self, dependant: u64, dependency: u64) { pub fn add_dependency_by_issue_number(&mut self, dependant: u64, dependency: u64) {
let a = self.find_element_by_issue_number(dependant).unwrap(); let a = self
let b = self.find_element_by_issue_number(dependency).unwrap(); .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, ()); self.graph.add_edge(a, b, ());
} }
@ -422,7 +426,8 @@ impl<'a> Subtree<'a> {
.collect(); .collect();
edges.sort(); 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) sha256::digest(json_data)
} }
} }