From d13d61f89eefa52413a710e311547c59745764b0 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 3 Oct 2025 18:17:02 +0200 Subject: [PATCH 1/5] Improve and simplify commands errors --- src/github/webhook.rs | 3 +-- src/handlers.rs | 34 +++++++++++++++++++++++++----- src/handlers/assign.rs | 13 +++++------- src/handlers/close.rs | 6 ++---- src/handlers/concern.rs | 11 +++++----- src/handlers/major_change.rs | 31 ++++++++-------------------- src/handlers/nominate.rs | 10 ++------- src/handlers/ping.rs | 40 +++++++++--------------------------- src/handlers/relabel.rs | 10 ++++----- src/handlers/shortcut.rs | 9 ++++---- 10 files changed, 71 insertions(+), 96 deletions(-) diff --git a/src/github/webhook.rs b/src/github/webhook.rs index 22914012..91e76aa7 100644 --- a/src/github/webhook.rs +++ b/src/github/webhook.rs @@ -274,11 +274,10 @@ async fn process_payload( } } if !message.is_empty() { + log::info!("user error: {}", message); if let Some(issue) = event.issue() { let cmnt = ErrorComment::new(issue, message); cmnt.post(&ctx.github).await?; - } else { - log::error!("handling event failed: {:?}", message); } } if other_error { diff --git a/src/handlers.rs b/src/handlers.rs index 35383ce3..d7efd19b 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -27,6 +27,26 @@ impl fmt::Display for HandlerError { } } +#[derive(Debug)] +pub struct UserError(String); + +impl std::error::Error for UserError {} + +impl fmt::Display for UserError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(&self.0) + } +} + +macro_rules! inform { + (msg:literal $(,)?) => { + anyhow::bail!(crate::handlers::UserError($msg.into())) + }; + ($err:expr $(,)?) => { + anyhow::bail!(crate::handlers::UserError($err.into())) + }; +} + mod assign; mod autolabel; mod backport; @@ -368,11 +388,15 @@ macro_rules! command_handlers { if let Some(config) = &config.$name { $name::handle_command(ctx, config, event, command) .await - .unwrap_or_else(|err| { - errors.push(HandlerError::Other(err.context(format!( - "error when processing {} command handler", - stringify!($name) - )))) + .unwrap_or_else(|mut err| { + if let Some(err) = err.downcast_mut::() { + errors.push(HandlerError::Message(std::mem::take(&mut err.0))); + } else { + errors.push(HandlerError::Other(err.context(format!( + "error when processing {} command handler", + stringify!($name) + )))); + } }); } else { errors.push(HandlerError::Message(format!( diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index 42e9f3e9..34606a83 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -525,10 +525,7 @@ pub(super) async fn handle_command( let issue = event.issue().unwrap(); if issue.is_pr() { if !issue.is_open() { - issue - .post_comment(&ctx.github, "Assignment is not allowed on a closed PR.") - .await?; - return Ok(()); + inform!("Assignment is not allowed on a closed PR."); } if matches!( event, @@ -612,7 +609,7 @@ pub(super) async fn handle_command( AssignCommand::Claim => event.user().login.clone(), AssignCommand::AssignUser { username } => { if !is_team_member && username != event.user().login { - bail!("Only Rust team members can assign other users"); + inform!("Only Rust team members can assign other users"); } username.clone() } @@ -627,7 +624,7 @@ pub(super) async fn handle_command( e.apply(&ctx.github, String::new()).await?; return Ok(()); } else { - bail!("Cannot release another user's assignment"); + inform!("Cannot release another user's assignment"); } } else { let current = &event.user().login; @@ -639,11 +636,11 @@ pub(super) async fn handle_command( e.apply(&ctx.github, String::new()).await?; return Ok(()); } else { - bail!("Cannot release unassigned issue"); + inform!("Cannot release unassigned issue"); } }; } - AssignCommand::RequestReview { .. } => bail!("r? is only allowed on PRs."), + AssignCommand::RequestReview { .. } => inform!("r? is only allowed on PRs."), }; // Don't re-assign if aleady assigned, e.g. on comment edit if issue.contain_assignee(&to_assign) { diff --git a/src/handlers/close.rs b/src/handlers/close.rs index fdb77986..6539cb71 100644 --- a/src/handlers/close.rs +++ b/src/handlers/close.rs @@ -1,6 +1,6 @@ //! Allows to close an issue or a PR -use crate::{config::CloseConfig, github::Event, handlers::Context, interactions::ErrorComment}; +use crate::{config::CloseConfig, github::Event, handlers::Context}; use parser::command::close::CloseCommand; pub(super) async fn handle_command( @@ -16,9 +16,7 @@ pub(super) async fn handle_command( .await .unwrap_or(false); if !is_team_member { - let cmnt = ErrorComment::new(&issue, "Only team members can close issues."); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!("Only team members can close issues."); } issue.close(&ctx.github).await?; Ok(()) diff --git a/src/handlers/concern.rs b/src/handlers/concern.rs index 4db9a480..45960a36 100644 --- a/src/handlers/concern.rs +++ b/src/handlers/concern.rs @@ -6,7 +6,7 @@ use crate::{ config::ConcernConfig, github::{Event, Label}, handlers::Context, - interactions::{EditIssueBody, ErrorComment}, + interactions::EditIssueBody, }; use parser::command::concern::ConcernCommand; @@ -37,7 +37,7 @@ pub(super) async fn handle_command( cmd: ConcernCommand, ) -> anyhow::Result<()> { let Event::IssueComment(issue_comment) = event else { - bail!("concern issued on something other than a issue") + inform!("Concerns can only be issued on an issue"); }; let Some(comment_url) = event.html_url() else { bail!("unable to retrieve the comment url") @@ -81,10 +81,9 @@ pub(super) async fn handle_command( issue.number, issue_comment.comment.user, ); - ErrorComment::new(&issue, "Only team members in the [team repo](https://github.com/rust-lang/team) can add or resolve concerns.") - .post(&ctx.github) - .await?; - return Ok(()); + inform!( + "Only team members in the [team repo](https://github.com/rust-lang/team) can add or resolve concerns." + ); } let mut client = ctx.db.get().await; diff --git a/src/handlers/major_change.rs b/src/handlers/major_change.rs index 463146f2..74807140 100644 --- a/src/handlers/major_change.rs +++ b/src/handlers/major_change.rs @@ -6,7 +6,6 @@ use crate::{ config::MajorChangeConfig, github::{Event, Issue, IssuesAction, IssuesEvent, Label, ZulipGitHubReference}, handlers::Context, - interactions::ErrorComment, }; use anyhow::Context as _; use async_trait::async_trait; @@ -113,15 +112,10 @@ pub(super) async fn handle_input( .iter() .any(|l| l.name == config.enabling_label) { - let cmnt = ErrorComment::new( - &event.issue, - format!( - "This issue is not ready for proposals; it lacks the `{}` label.", - config.enabling_label - ), - ); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!(format!( + "This issue is not ready for proposals; it lacks the `{}` label.", + config.enabling_label + )); } let (zulip_msg, label_to_add) = match cmd { Invocation::NewProposal => ( @@ -253,15 +247,10 @@ pub(super) async fn handle_command( .iter() .any(|l| l.name == config.enabling_label) { - let cmnt = ErrorComment::new( - &issue, - &format!( - "This issue cannot be seconded; it lacks the `{}` label.", - config.enabling_label - ), - ); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!(format!( + "This issue cannot be seconded; it lacks the `{}` label.", + config.enabling_label + )); } let is_team_member = event @@ -272,9 +261,7 @@ pub(super) async fn handle_command( .unwrap_or(false); if !is_team_member { - let cmnt = ErrorComment::new(&issue, "Only team members can second issues."); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!("Only team members can second issues."); } let has_concerns = if let Some(concerns_label) = &config.concerns_label { diff --git a/src/handlers/nominate.rs b/src/handlers/nominate.rs index 49c110a8..640ecdce 100644 --- a/src/handlers/nominate.rs +++ b/src/handlers/nominate.rs @@ -21,15 +21,9 @@ pub(super) async fn handle_command( }; if !is_team_member { - let cmnt = ErrorComment::new( - &event.issue().unwrap(), - format!( - "Nominating and approving issues and pull requests is restricted to members of\ - the Rust teams." - ), + inform!( + "Nominating and approving issues and pull requests is restricted to members of the Rust teams." ); - cmnt.post(&ctx.github).await?; - return Ok(()); } let issue_labels = event.issue().unwrap().labels(); diff --git a/src/handlers/ping.rs b/src/handlers/ping.rs index 86c361e6..290f6513 100644 --- a/src/handlers/ping.rs +++ b/src/handlers/ping.rs @@ -8,7 +8,6 @@ use crate::{ config::PingConfig, github::{self, Event}, handlers::Context, - interactions::ErrorComment, }; use parser::command::ping::PingCommand; @@ -25,42 +24,27 @@ pub(super) async fn handle_command( }; if !is_team_member { - let cmnt = ErrorComment::new( - &event.issue().unwrap(), - format!("Only Rust team members can ping teams."), - ); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!("Only Rust team members can ping teams."); } let (gh_team, config) = match config.get_by_name(&team_name.team) { Some(v) => v, None => { - let cmnt = ErrorComment::new( - &event.issue().unwrap(), - format!( - "This team (`{}`) cannot be pinged via this command; \ + inform!(format!( + "This team (`{}`) cannot be pinged via this command; \ it may need to be added to `triagebot.toml` on the default branch.", - team_name.team, - ), - ); - cmnt.post(&ctx.github).await?; - return Ok(()); + team_name.team, + )); } }; let team = ctx.team.get_team(&gh_team).await?; let team = match team { Some(team) => team, None => { - let cmnt = ErrorComment::new( - &event.issue().unwrap(), - format!( - "This team (`{}`) does not exist in the team repository.", - team_name.team, - ), - ); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!(format!( + "This team (`{}`) does not exist in the team repository.", + team_name.team, + )); } }; @@ -76,11 +60,7 @@ pub(super) async fn handle_command( ) .await { - let cmnt = ErrorComment::new( - &event.issue().unwrap(), - format!("Error adding team label (`{}`): {:?}.", label, err), - ); - cmnt.post(&ctx.github).await?; + inform!(format!("Error adding team label (`{}`): {:?}.", label, err)); } } diff --git a/src/handlers/relabel.rs b/src/handlers/relabel.rs index 9eed2f8e..cff7cd9d 100644 --- a/src/handlers/relabel.rs +++ b/src/handlers/relabel.rs @@ -17,7 +17,6 @@ use crate::{ github::UnknownLabels, github::{self, Event}, handlers::Context, - interactions::ErrorComment, }; use parser::command::relabel::{LabelDelta, RelabelCommand}; @@ -28,7 +27,7 @@ pub(super) async fn handle_command( input: RelabelCommand, ) -> anyhow::Result<()> { let Some(issue) = event.issue() else { - anyhow::bail!("event is not an issue"); + inform!("Can only add and remove labels on an issue"); }; // Check label authorization for the current user @@ -47,10 +46,9 @@ pub(super) async fn handle_command( )), Err(err) => Some(err), }; - if let Some(msg) = err { - let cmnt = ErrorComment::new(issue, msg); - cmnt.post(&ctx.github).await?; - return Ok(()); + if let Some(err) = err { + // bail-out and inform the user why + inform!(err); } } diff --git a/src/handlers/shortcut.rs b/src/handlers/shortcut.rs index 1ad6a0f3..c2c878ad 100644 --- a/src/handlers/shortcut.rs +++ b/src/handlers/shortcut.rs @@ -7,7 +7,6 @@ use crate::{ db::issue_data::IssueData, github::{Event, Label}, handlers::Context, - interactions::ErrorComment, }; use octocrab::models::AuthorAssociation; use parser::command::shortcut::ShortcutCommand; @@ -31,10 +30,10 @@ pub(super) async fn handle_command( let issue = event.issue().unwrap(); // NOTE: if shortcuts available to issues are created, they need to be allowed here if !issue.is_pr() { - let msg = format!("The \"{:?}\" shortcut only works on pull requests.", input); - let cmnt = ErrorComment::new(&issue, msg); - cmnt.post(&ctx.github).await?; - return Ok(()); + inform!(format!( + "The \"{:?}\" shortcut only works on pull requests.", + input + )); } let issue_labels = issue.labels(); From 310007f73dc5452dd1f46f41045637f8cd4d6b58 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 3 Oct 2025 18:22:56 +0200 Subject: [PATCH 2/5] Move around some structs --- src/handlers.rs | 78 ++++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index d7efd19b..82bbdc95 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -10,34 +10,6 @@ use std::fmt; use std::sync::Arc; use tracing as log; -#[derive(Debug)] -pub enum HandlerError { - Message(String), - Other(anyhow::Error), -} - -impl std::error::Error for HandlerError {} - -impl fmt::Display for HandlerError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - HandlerError::Message(msg) => write!(f, "{}", msg), - HandlerError::Other(_) => write!(f, "An internal error occurred."), - } - } -} - -#[derive(Debug)] -pub struct UserError(String); - -impl std::error::Error for UserError {} - -impl fmt::Display for UserError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str(&self.0) - } -} - macro_rules! inform { (msg:literal $(,)?) => { anyhow::bail!(crate::handlers::UserError($msg.into())) @@ -81,6 +53,19 @@ mod shortcut; mod transfer; pub mod types_planning_updates; +pub struct Context { + pub github: GithubClient, + pub zulip: ZulipClient, + pub team: TeamClient, + pub db: crate::db::ClientPool, + pub username: String, + pub octocrab: Octocrab, + /// Represents the workqueue (assigned open PRs) of individual reviewers. + /// tokio's RwLock is used to avoid deadlocks, since we run on a single-threaded tokio runtime. + pub workqueue: Arc>, + pub gha_logs: Arc>, +} + pub async fn handle(ctx: &Context, host: &str, event: &Event) -> Vec { let config = config::get(&ctx.github, event.repo()).await; if let Err(e) = &config { @@ -440,15 +425,30 @@ command_handlers! { transfer: Transfer, } -pub struct Context { - pub github: GithubClient, - pub zulip: ZulipClient, - pub team: TeamClient, - pub db: crate::db::ClientPool, - pub username: String, - pub octocrab: Octocrab, - /// Represents the workqueue (assigned open PRs) of individual reviewers. - /// tokio's RwLock is used to avoid deadlocks, since we run on a single-threaded tokio runtime. - pub workqueue: Arc>, - pub gha_logs: Arc>, +#[derive(Debug)] +pub enum HandlerError { + Message(String), + Other(anyhow::Error), +} + +impl std::error::Error for HandlerError {} + +impl fmt::Display for HandlerError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + HandlerError::Message(msg) => write!(f, "{}", msg), + HandlerError::Other(_) => write!(f, "An internal error occurred."), + } + } +} + +#[derive(Debug)] +pub struct UserError(String); + +impl std::error::Error for UserError {} + +impl fmt::Display for UserError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(&self.0) + } } From e2144427e3c44c34b231378e5746ffad690a829b Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 3 Oct 2025 18:55:24 +0200 Subject: [PATCH 3/5] Remove useless match arm --- src/handlers.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index 82bbdc95..d63fdac9 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -11,9 +11,6 @@ use std::sync::Arc; use tracing as log; macro_rules! inform { - (msg:literal $(,)?) => { - anyhow::bail!(crate::handlers::UserError($msg.into())) - }; ($err:expr $(,)?) => { anyhow::bail!(crate::handlers::UserError($err.into())) }; From adc93ef44942090d370d7592f93e71658d068a5f Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 4 Oct 2025 16:00:04 +0200 Subject: [PATCH 4/5] Handle custom handlers in a more principled way --- src/handlers.rs | 208 +++++++++++++++++++----------------------------- 1 file changed, 82 insertions(+), 126 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index d63fdac9..0b2c42a5 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -15,6 +15,37 @@ macro_rules! inform { anyhow::bail!(crate::handlers::UserError($err.into())) }; } +macro_rules! custom_handlers { + ($errors:ident -> $($name:ident: $hd:expr,)*) => {{ + // Process the handlers concurrently + let results = futures::join!( + $( + async { + async { + $hd + } + .await + .map_err(|e: anyhow::Error| { + HandlerError::Other(e.context(format!( + "error when processing {} handler", + stringify!($name) + ))) + }) + } + ),* + ); + + // Destructure the results into named variables + let ($($name,)*) = results; + + // Push errors for each handler + $( + if let Err(e) = $name { + $errors.push(e); + } + )* + }} +} mod assign; mod autolabel; @@ -78,132 +109,57 @@ pub async fn handle(ctx: &Context, host: &str, event: &Event) -> Vec + project_goals: project_goals::handle(ctx, event).await, + notification: notification::handle(ctx, event).await, + rustc_commits: rustc_commits::handle(ctx, event).await, + milestone_prs: milestone_prs::handle(ctx, event).await, + relnotes: relnotes::handle(ctx, event).await, + check_commits: { + if let Ok(config) = &config { + check_commits::handle(ctx, host, event, &config).await?; + } + Ok(()) + }, + rendered_link: { + if let Some(rendered_link_config) = config.as_ref().ok().and_then(|c| c.rendered_link.as_ref()) + { + rendered_link::handle(ctx, event, rendered_link_config).await? + } + Ok(()) + }, + bot_pull_requests: { + if config.as_ref().is_ok_and(|c| c.bot_pull_requests.is_some()) { + bot_pull_requests::handle(ctx, event).await?; + } + Ok(()) + }, + review_submitted: { + if let Some(config) = config.as_ref().ok().and_then(|c| c.review_submitted.as_ref()) { + review_submitted::handle(ctx, event, config).await?; + } + Ok(()) + }, + review_changes_since: { + if let Some(config) = config.as_ref().ok().and_then(|c| c.review_changes_since.as_ref()) { + review_changes_since::handle(ctx, host, event, config).await?; + } + Ok(()) + }, + github_releases: { + if let Some(ghr_config) = config.as_ref().ok().and_then(|c| c.github_releases.as_ref()) { + github_releases::handle(ctx, event, ghr_config).await?; + } + Ok(()) + }, + merge_conflicts: { + if let Some(conflict_config) = config.as_ref().ok().and_then(|c| c.merge_conflicts.as_ref()) { + merge_conflicts::handle(ctx, event, conflict_config).await?; + } + Ok(()) + }, + }; errors } From 66ce675ceed884f889064bb59fcabbf94bd2496e Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 4 Oct 2025 16:09:16 +0200 Subject: [PATCH 5/5] Rearrange handlers macro impl and usage --- src/handlers.rs | 281 ++++++----------------------------------- src/handlers/macros.rs | 210 ++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 244 deletions(-) create mode 100644 src/handlers/macros.rs diff --git a/src/handlers.rs b/src/handlers.rs index 0b2c42a5..9a4169e9 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -15,37 +15,9 @@ macro_rules! inform { anyhow::bail!(crate::handlers::UserError($err.into())) }; } -macro_rules! custom_handlers { - ($errors:ident -> $($name:ident: $hd:expr,)*) => {{ - // Process the handlers concurrently - let results = futures::join!( - $( - async { - async { - $hd - } - .await - .map_err(|e: anyhow::Error| { - HandlerError::Other(e.context(format!( - "error when processing {} handler", - stringify!($name) - ))) - }) - } - ),* - ); - // Destructure the results into named variables - let ($($name,)*) = results; - - // Push errors for each handler - $( - if let Err(e) = $name { - $errors.push(e); - } - )* - }} -} +#[macro_use] +mod macros; mod assign; mod autolabel; @@ -94,6 +66,41 @@ pub struct Context { pub gha_logs: Arc>, } +// Handle events that happened on issues +// +// This is for events that happen only on issues or pull requests (e.g. label changes or assignments). +// Each module in the list must contain the functions `parse_input` and `handle_input`. +issue_handlers! { + assign, + autolabel, + backport, + issue_links, + major_change, + mentions, + notify_zulip, + review_requested, + pr_tracking, +} + +// Handle commands in comments/issues body +// +// This is for handlers for commands parsed by the `parser` crate. +// Each variant of `parser::command::Command` must be in this list, +// preceded by the module containing the corresponding `handle_command` function +command_handlers! { + assign: Assign, + nominate: Nominate, + ping: Ping, + prioritize: Prioritize, + relabel: Relabel, + major_change: Second, + shortcut: Shortcut, + close: Close, + note: Note, + concern: Concern, + transfer: Transfer, +} + pub async fn handle(ctx: &Context, host: &str, event: &Event) -> Vec { let config = config::get(&ctx.github, event.repo()).await; if let Err(e) = &config { @@ -164,220 +171,6 @@ pub async fn handle(ctx: &Context, host: &str, event: &Event) -> Vec { - async fn handle_issue( - ctx: &Context, - event: &IssuesEvent, - config: &Arc, - errors: &mut Vec, - ) { - // Process the issue handlers concurrently - let results = futures::join!( - $( - async { - match $name::parse_input(ctx, event, config.$name.as_ref()).await { - Err(err) => Err(HandlerError::Message(err)), - Ok(Some(input)) => { - if let Some(config) = &config.$name { - $name::handle_input(ctx, config, event, input) - .await - .map_err(|e| { - HandlerError::Other(e.context(format!( - "error when processing {} handler", - stringify!($name) - ))) - }) - } else { - Err(HandlerError::Message(format!( - "The feature `{}` is not enabled in this repository.\n\ - To enable it add its section in the `triagebot.toml` \ - in the root of the repository.", - stringify!($name) - ))) - } - } - Ok(None) => Ok(()) - } - } - ),* - ); - - // Destructure the results into named variables - let ($($name,)*) = results; - - // Push errors for each handler - $( - if let Err(e) = $name { - errors.push(e); - } - )* - } - } -} - -// Handle events that happened on issues -// -// This is for events that happen only on issues or pull requests (e.g. label changes or assignments). -// Each module in the list must contain the functions `parse_input` and `handle_input`. -issue_handlers! { - assign, - autolabel, - backport, - issue_links, - major_change, - mentions, - notify_zulip, - review_requested, - pr_tracking, -} - -macro_rules! command_handlers { - ($($name:ident: $enum:ident,)*) => { - async fn handle_command( - ctx: &Context, - event: &Event, - config: &Result, ConfigurationError>, - body: &str, - errors: &mut Vec, - ) { - match event { - // always handle new PRs / issues - Event::Issue(IssuesEvent { action: IssuesAction::Opened, .. }) => {}, - Event::Issue(IssuesEvent { action: IssuesAction::Edited, .. }) => { - // if the issue was edited, but we don't get a `changes[body]` diff, it means only the title was edited, not the body. - // don't process the same commands twice. - if event.comment_from().is_none() { - log::debug!("skipping title-only edit event"); - return; - } - }, - Event::Issue(e) => { - // no change in issue's body for these events, so skip - log::debug!("skipping event, issue was {:?}", e.action); - return; - } - Event::IssueComment(e) => { - match e.action { - IssueCommentAction::Created => {} - IssueCommentAction::Edited => { - if event.comment_from().is_none() { - // We are not entirely sure why this happens. - // Sometimes when someone posts a PR review, - // GitHub sends an "edited" event with no - // changes just before the "created" event. - log::debug!("skipping issue comment edit without changes"); - return; - } - } - IssueCommentAction::Deleted => { - // don't execute commands again when comment is deleted - log::debug!("skipping event, comment was {:?}", e.action); - return; - } - } - } - Event::Push(_) | Event::Create(_) => { - log::debug!("skipping unsupported event"); - return; - } - } - - let input = Input::new(&body, vec![&ctx.username, "triagebot"]); - let commands = if let Some(previous) = event.comment_from() { - let prev_commands = Input::new(&previous, vec![&ctx.username, "triagebot"]).collect::>(); - input.filter(|cmd| !prev_commands.contains(cmd)).collect::>() - } else { - input.collect() - }; - - log::info!("Comment parsed to {:?}", commands); - - if commands.is_empty() { - return; - } - - let config = match config { - Ok(config) => config, - Err(e @ ConfigurationError::Missing) => { - // r? is conventionally used to mean "hey, can you review" - // even if the repo doesn't have a triagebot.toml. In that - // case, just ignore it. - if commands - .iter() - .all(|cmd| matches!(cmd, Command::Assign(Ok(AssignCommand::RequestReview { .. })))) - { - return; - } - return errors.push(HandlerError::Message(e.to_string())); - } - Err(e @ ConfigurationError::Toml(_)) => { - return errors.push(HandlerError::Message(e.to_string())); - } - Err(e @ ConfigurationError::Http(_)) => { - return errors.push(HandlerError::Other(e.clone().into())); - } - }; - - for command in commands { - match command { - $( - Command::$enum(Ok(command)) => { - if let Some(config) = &config.$name { - $name::handle_command(ctx, config, event, command) - .await - .unwrap_or_else(|mut err| { - if let Some(err) = err.downcast_mut::() { - errors.push(HandlerError::Message(std::mem::take(&mut err.0))); - } else { - errors.push(HandlerError::Other(err.context(format!( - "error when processing {} command handler", - stringify!($name) - )))); - } - }); - } else { - errors.push(HandlerError::Message(format!( - "The feature `{}` is not enabled in this repository.\n\ - To enable it add its section in the `triagebot.toml` \ - in the root of the repository.", - stringify!($name) - ))); - } - } - Command::$enum(Err(err)) => { - errors.push(HandlerError::Message(format!( - "Parsing {} command in [comment]({}) failed: {}", - stringify!($name), - event.html_url().expect("has html url"), - err - ))); - })* - } - } - } - } -} - -// Handle commands in comments/issues body -// -// This is for handlers for commands parsed by the `parser` crate. -// Each variant of `parser::command::Command` must be in this list, -// preceded by the module containing the corresponding `handle_command` function -command_handlers! { - assign: Assign, - nominate: Nominate, - ping: Ping, - prioritize: Prioritize, - relabel: Relabel, - major_change: Second, - shortcut: Shortcut, - close: Close, - note: Note, - concern: Concern, - transfer: Transfer, -} - #[derive(Debug)] pub enum HandlerError { Message(String), diff --git a/src/handlers/macros.rs b/src/handlers/macros.rs new file mode 100644 index 00000000..2a77973e --- /dev/null +++ b/src/handlers/macros.rs @@ -0,0 +1,210 @@ +macro_rules! issue_handlers { + ($($name:ident,)*) => { + async fn handle_issue( + ctx: &Context, + event: &IssuesEvent, + config: &Arc, + errors: &mut Vec, + ) { + // Process the issue handlers concurrently + let results = futures::join!( + $( + async { + match $name::parse_input(ctx, event, config.$name.as_ref()).await { + Err(err) => Err(HandlerError::Message(err)), + Ok(Some(input)) => { + if let Some(config) = &config.$name { + $name::handle_input(ctx, config, event, input) + .await + .map_err(|e| { + HandlerError::Other(e.context(format!( + "error when processing {} handler", + stringify!($name) + ))) + }) + } else { + Err(HandlerError::Message(format!( + "The feature `{}` is not enabled in this repository.\n\ + To enable it add its section in the `triagebot.toml` \ + in the root of the repository.", + stringify!($name) + ))) + } + } + Ok(None) => Ok(()) + } + } + ),* + ); + + // Destructure the results into named variables + let ($($name,)*) = results; + + // Push errors for each handler + $( + if let Err(e) = $name { + errors.push(e); + } + )* + } + } +} + +macro_rules! command_handlers { + ($($name:ident: $enum:ident,)*) => { + async fn handle_command( + ctx: &Context, + event: &Event, + config: &Result, ConfigurationError>, + body: &str, + errors: &mut Vec, + ) { + match event { + // always handle new PRs / issues + Event::Issue(IssuesEvent { action: IssuesAction::Opened, .. }) => {}, + Event::Issue(IssuesEvent { action: IssuesAction::Edited, .. }) => { + // if the issue was edited, but we don't get a `changes[body]` diff, it means only the title was edited, not the body. + // don't process the same commands twice. + if event.comment_from().is_none() { + log::debug!("skipping title-only edit event"); + return; + } + }, + Event::Issue(e) => { + // no change in issue's body for these events, so skip + log::debug!("skipping event, issue was {:?}", e.action); + return; + } + Event::IssueComment(e) => { + match e.action { + IssueCommentAction::Created => {} + IssueCommentAction::Edited => { + if event.comment_from().is_none() { + // We are not entirely sure why this happens. + // Sometimes when someone posts a PR review, + // GitHub sends an "edited" event with no + // changes just before the "created" event. + log::debug!("skipping issue comment edit without changes"); + return; + } + } + IssueCommentAction::Deleted => { + // don't execute commands again when comment is deleted + log::debug!("skipping event, comment was {:?}", e.action); + return; + } + } + } + Event::Push(_) | Event::Create(_) => { + log::debug!("skipping unsupported event"); + return; + } + } + + let input = Input::new(&body, vec![&ctx.username, "triagebot"]); + let commands = if let Some(previous) = event.comment_from() { + let prev_commands = Input::new(&previous, vec![&ctx.username, "triagebot"]).collect::>(); + input.filter(|cmd| !prev_commands.contains(cmd)).collect::>() + } else { + input.collect() + }; + + log::info!("Comment parsed to {:?}", commands); + + if commands.is_empty() { + return; + } + + let config = match config { + Ok(config) => config, + Err(e @ ConfigurationError::Missing) => { + // r? is conventionally used to mean "hey, can you review" + // even if the repo doesn't have a triagebot.toml. In that + // case, just ignore it. + if commands + .iter() + .all(|cmd| matches!(cmd, Command::Assign(Ok(AssignCommand::RequestReview { .. })))) + { + return; + } + return errors.push(HandlerError::Message(e.to_string())); + } + Err(e @ ConfigurationError::Toml(_)) => { + return errors.push(HandlerError::Message(e.to_string())); + } + Err(e @ ConfigurationError::Http(_)) => { + return errors.push(HandlerError::Other(e.clone().into())); + } + }; + + for command in commands { + match command { + $( + Command::$enum(Ok(command)) => { + if let Some(config) = &config.$name { + $name::handle_command(ctx, config, event, command) + .await + .unwrap_or_else(|mut err| { + if let Some(err) = err.downcast_mut::() { + errors.push(HandlerError::Message(std::mem::take(&mut err.0))); + } else { + errors.push(HandlerError::Other(err.context(format!( + "error when processing {} command handler", + stringify!($name) + )))); + } + }); + } else { + errors.push(HandlerError::Message(format!( + "The feature `{}` is not enabled in this repository.\n\ + To enable it add its section in the `triagebot.toml` \ + in the root of the repository.", + stringify!($name) + ))); + } + } + Command::$enum(Err(err)) => { + errors.push(HandlerError::Message(format!( + "Parsing {} command in [comment]({}) failed: {}", + stringify!($name), + event.html_url().expect("has html url"), + err + ))); + })* + } + } + } + } +} + +macro_rules! custom_handlers { + ($errors:ident -> $($name:ident: $hd:expr,)*) => {{ + // Process the handlers concurrently + let results = futures::join!( + $( + async { + async { + $hd + } + .await + .map_err(|e: anyhow::Error| { + HandlerError::Other(e.context(format!( + "error when processing {} handler", + stringify!($name) + ))) + }) + } + ),* + ); + + // Destructure the results into named variables + let ($($name,)*) = results; + + // Push errors for each handler + $( + if let Err(e) = $name { + $errors.push(e); + } + )* + }} +}