Improve and simplify commands errors

Author: Urgau

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 {

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::<UserError>() {
+                                        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!(

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) {

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(())

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;

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 {

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();

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));
         }
     }
 

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);
         }
     }
 

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();