From 2761ba917c8c2cf537d60d933cedc82de4ef58b5 Mon Sep 17 00:00:00 2001
From: Joshua Nelson <jnelson@cloudflare.com>
Date: Tue, 12 Apr 2022 19:00:58 -0500
Subject: [PATCH] Comment on issues when `@rustbot label` is given an invalid
 label

Previously, the label was just silently ignored, along with all other labels
in the same command. This tells the user what went wrong, and also adds all valid labels
---
 src/github.rs             | 52 ++++++++++++++++++++++++++++++++-------
 src/handlers/autolabel.rs | 13 +++++++++-
 src/interactions.rs       |  1 -
 3 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/src/github.rs b/src/github.rs
index 0293ec1c4..02e58ffdb 100644
--- a/src/github.rs
+++ b/src/github.rs
@@ -366,17 +366,36 @@ impl IssueRepository {
         )
     }
 
-    async fn has_label(&self, client: &GithubClient, label: &str) -> bool {
+    async fn has_label(&self, client: &GithubClient, label: &str) -> anyhow::Result<bool> {
         #[allow(clippy::redundant_pattern_matching)]
         let url = format!("{}/labels/{}", self.url(), label);
-        match client.send_req(client.get(&url)).await {
-            Ok(_) => true,
-            // XXX: Error handling if the request failed for reasons beyond 'label didn't exist'
-            Err(_) => false,
+        match client._send_req(client.get(&url)).await {
+            Ok((_, _)) => Ok(true),
+            Err(e) => {
+                if e.downcast_ref::<reqwest::Error>().map_or(false, |e| e.status() == Some(StatusCode::NOT_FOUND)) {
+                    Ok(false)
+                } else {
+                    Err(e)
+                }
+            }
         }
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct UnknownLabels {
+    labels: Vec<String>,
+}
+
+// NOTE: This is used to post the Github comment; make sure it's valid markdown.
+impl fmt::Display for UnknownLabels {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Unknown labels: {}", &self.labels.join(", "))
+    }
+}
+
+impl std::error::Error for UnknownLabels {}
+
 impl Issue {
     pub fn to_zulip_github_reference(&self) -> ZulipGitHubReference {
         ZulipGitHubReference {
@@ -519,18 +538,27 @@ impl Issue {
             return Ok(());
         }
 
-        for label in &labels {
-            if !self.repository().has_label(client, &label).await {
-                anyhow::bail!("Label {} does not exist in {}", label, self.global_id());
+        let mut unknown_labels = vec![];
+        let mut known_labels = vec![];
+        for label in labels {
+            if !self.repository().has_label(client, &label).await? {
+                unknown_labels.push(label);
+            } else {
+                known_labels.push(label);
             }
         }
 
+        if !unknown_labels.is_empty() {
+            return Err(UnknownLabels { labels: unknown_labels }.into());
+        }
+
         #[derive(serde::Serialize)]
         struct LabelsReq {
             labels: Vec<String>,
         }
+
         client
-            ._send_req(client.post(&url).json(&LabelsReq { labels }))
+            ._send_req(client.post(&url).json(&LabelsReq { labels: known_labels }))
             .await
             .context("failed to add labels")?;
 
@@ -1382,6 +1410,12 @@ pub trait IssuesQuery {
 mod tests {
     use super::*;
 
+    #[test]
+    fn display_labels() {
+        let x = UnknownLabels { labels: vec!["A-bootstrap".into(), "xxx".into()] };
+        assert_eq!(x.to_string(), "Unknown labels: A-bootstrap, xxx");
+    }
+
     #[test]
     fn extract_one_file() {
         let input = r##"\
diff --git a/src/handlers/autolabel.rs b/src/handlers/autolabel.rs
index b08c5d43a..cb99489d9 100644
--- a/src/handlers/autolabel.rs
+++ b/src/handlers/autolabel.rs
@@ -125,7 +125,18 @@ pub(super) async fn handle_input(
     event: &IssuesEvent,
     input: AutolabelInput,
 ) -> anyhow::Result<()> {
-    event.issue.add_labels(&ctx.github, input.add).await?;
+    match event.issue.add_labels(&ctx.github, input.add).await {
+        Ok(()) => {}
+        Err(e) => {
+            use crate::github::UnknownLabels;
+            if let Some(err @ UnknownLabels { .. }) = e.downcast_ref() {
+                event.issue.post_comment(&ctx.github, &err.to_string()).await.context("failed to post missing label comment")?;
+                return Ok(());
+            }
+            return Err(e);
+        }
+    }
+
     for label in input.remove {
         event
             .issue
diff --git a/src/interactions.rs b/src/interactions.rs
index f5bf30868..ef1e71d47 100644
--- a/src/interactions.rs
+++ b/src/interactions.rs
@@ -1,6 +1,5 @@
 use crate::github::{GithubClient, Issue};
 use std::fmt::Write;
-use tracing as log;
 
 pub struct ErrorComment<'a> {
     issue: &'a Issue,