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..9a4169e9 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -10,22 +10,14 @@ use std::fmt; use std::sync::Arc; use tracing as log; -#[derive(Debug)] -pub enum HandlerError { - Message(String), - Other(anyhow::Error), +macro_rules! inform { + ($err:expr $(,)?) => { + anyhow::bail!(crate::handlers::UserError($err.into())) + }; } -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."), - } - } -} +#[macro_use] +mod macros; mod assign; mod autolabel; @@ -61,201 +53,17 @@ mod shortcut; mod transfer; pub mod types_planning_updates; -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 { - log::warn!("configuration error {}: {e}", event.repo().full_name); - } - let mut errors = Vec::new(); - - if let (Ok(config), Event::Issue(event)) = (config.as_ref(), event) { - handle_issue(ctx, event, config, &mut errors).await; - } - - if let Some(body) = event.comment_body() { - handle_command(ctx, event, &config, body, &mut errors).await; - } - - if let Ok(config) = &config { - if let Err(e) = check_commits::handle(ctx, host, event, &config).await { - log::error!( - "failed to process event {:?} with `check_commits` handler: {:?}", - event, - e - ); - } - } - - if let Err(e) = project_goals::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with `project_goals` handler: {:?}", - event, - e - ); - } - - if let Err(e) = notification::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with notification handler: {:?}", - event, - e - ); - } - - if let Err(e) = rustc_commits::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with rustc_commits handler: {:?}", - event, - e - ); - } - - if let Err(e) = milestone_prs::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with milestone_prs handler: {:?}", - event, - e - ); - } - - if let Some(rendered_link_config) = config.as_ref().ok().and_then(|c| c.rendered_link.as_ref()) - { - if let Err(e) = rendered_link::handle(ctx, event, rendered_link_config).await { - log::error!( - "failed to process event {:?} with rendered_link handler: {:?}", - event, - e - ); - } - } - - if let Err(e) = relnotes::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with relnotes handler: {:?}", - event, - e - ); - } - - if config.as_ref().is_ok_and(|c| c.bot_pull_requests.is_some()) { - if let Err(e) = bot_pull_requests::handle(ctx, event).await { - log::error!( - "failed to process event {:?} with bot_pull_requests handler: {:?}", - event, - e - ) - } - } - - if let Some(config) = config - .as_ref() - .ok() - .and_then(|c| c.review_submitted.as_ref()) - { - if let Err(e) = review_submitted::handle(ctx, event, config).await { - log::error!( - "failed to process event {:?} with review_submitted handler: {:?}", - event, - e - ) - } - } - - if let Some(config) = config - .as_ref() - .ok() - .and_then(|c| c.review_changes_since.as_ref()) - { - if let Err(e) = review_changes_since::handle(ctx, host, event, config).await { - log::error!( - "failed to process event {:?} with review_changes_since handler: {:?}", - event, - e - ) - } - } - - if let Some(ghr_config) = config - .as_ref() - .ok() - .and_then(|c| c.github_releases.as_ref()) - { - if let Err(e) = github_releases::handle(ctx, event, ghr_config).await { - log::error!( - "failed to process event {:?} with github_releases handler: {:?}", - event, - e - ); - } - } - - if let Some(conflict_config) = config - .as_ref() - .ok() - .and_then(|c| c.merge_conflicts.as_ref()) - { - if let Err(e) = merge_conflicts::handle(ctx, event, conflict_config).await { - log::error!( - "failed to process event {:?} with merge_conflicts handler: {:?}", - event, - e - ); - } - } - - errors -} - -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); - } - )* - } - } +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>, } // Handle events that happened on issues @@ -274,129 +82,6 @@ issue_handlers! { 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(|err| { - 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. @@ -416,15 +101,100 @@ 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>, +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 { + log::warn!("configuration error {}: {e}", event.repo().full_name); + } + let mut errors = Vec::new(); + + if let (Ok(config), Event::Issue(event)) = (config.as_ref(), event) { + handle_issue(ctx, event, config, &mut errors).await; + } + + if let Some(body) = event.comment_body() { + handle_command(ctx, event, &config, body, &mut errors).await; + } + + // custom handlers (prefer issue_handlers! for issue event handler) + custom_handlers! { errors -> + 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 +} + +#[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) + } } 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/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); + } + )* + }} +} 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();