diff --git a/src/github/webhook.rs b/src/github/webhook.rs index 229140125..91e76aa72 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 35383ce3a..89ba4ca20 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -10,21 +10,16 @@ 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."), - } - } +/// Creates a [`UserError`] with message. +/// +/// Should be used when an handler is in error due to the user action's (not a PR, +/// not a issue, not authorized, ...). +/// +/// Should be used like this `return user_error!("My error message.");`. +macro_rules! user_error { + ($err:expr $(,)?) => { + anyhow::Result::Err(anyhow::anyhow!(crate::handlers::UserError($err.into()))) + }; } mod assign; @@ -61,6 +56,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 { @@ -368,11 +376,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!( @@ -416,15 +428,33 @@ 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."), + } + } +} + +/// Represent a user error. +/// +/// The message will be shown to the user via comment posted by this bot. +#[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 42e9f3e9a..caaaf440a 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(()); + return user_error!("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"); + return user_error!("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"); + return user_error!("Cannot release another user's assignment"); } } else { let current = &event.user().login; @@ -639,11 +636,13 @@ pub(super) async fn handle_command( e.apply(&ctx.github, String::new()).await?; return Ok(()); } else { - bail!("Cannot release unassigned issue"); + return user_error!("Cannot release unassigned issue"); } }; } - AssignCommand::RequestReview { .. } => bail!("r? is only allowed on PRs."), + AssignCommand::RequestReview { .. } => { + return user_error!("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 fdb779866..539f6653d 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(()); + return user_error!("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 4db9a480e..bb2f10e9a 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") + return user_error!("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(()); + return user_error!( + "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 463146f22..34e18d936 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(()); + return user_error!(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(()); + return user_error!(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(()); + return user_error!("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 49c110a8c..f9a190b1f 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." - ), + return user_error!( + "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 86c361e69..f748a2f8b 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(()); + return user_error!("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; \ + return user_error!(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(()); + return user_error!(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?; + return user_error!(format!("Error adding team label (`{}`): {:?}.", label, err)); } } diff --git a/src/handlers/relabel.rs b/src/handlers/relabel.rs index 9eed2f8e7..eefc74264 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"); + return user_error!("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 + return user_error!(err); } } diff --git a/src/handlers/shortcut.rs b/src/handlers/shortcut.rs index 1ad6a0f36..7b174afd1 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(()); + return user_error!(format!( + "The \"{:?}\" shortcut only works on pull requests.", + input + )); } let issue_labels = issue.labels();