Просмотр исходного кода

Bail out if adding a non-existent label

This prevents creating typo'd labels through rustbot, as the GitHub add label
API will implicitly create a new label if asked to add it to an issue.

We also no longer attempt to remove labels if we failed to add labels, though we
can still add labels and then fail to remove them.
Mark Rousskov 3 лет назад
Родитель
Сommit
b6ccaa0fb4
2 измененных файлов с 21 добавлено и 7 удалено
  1. 16 0
      src/github.rs
  2. 5 7
      src/handlers/relabel.rs

+ 16 - 0
src/github.rs

@@ -365,6 +365,16 @@ impl IssueRepository {
             self.organization, self.repository
         )
     }
+
+    async fn has_label(&self, client: &GithubClient, label: &str) -> 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,
+        }
+    }
 }
 
 impl Issue {
@@ -509,6 +519,12 @@ 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());
+            }
+        }
+
         #[derive(serde::Serialize)]
         struct LabelsReq {
             labels: Vec<String>,

+ 5 - 7
src/handlers/relabel.rs

@@ -53,11 +53,7 @@ pub(super) async fn handle_command(
             LabelDelta::Remove(label) => {
                 results.push((
                     label,
-                    event
-                        .issue()
-                        .unwrap()
-                        .remove_label(&ctx.github, &label)
-                        .await,
+                    event.issue().unwrap().remove_label(&ctx.github, &label),
                 ));
             }
         }
@@ -75,16 +71,18 @@ pub(super) async fn handle_command(
             event.issue().unwrap().global_id(),
             e
         );
+        return Err(e);
     }
 
-    for (label, res) in &results {
-        if let Err(e) = res {
+    for (label, res) in results {
+        if let Err(e) = res.await {
             tracing::error!(
                 "failed to remove {:?} from issue {}: {:?}",
                 label,
                 event.issue().unwrap().global_id(),
                 e
             );
+            return Err(e);
         }
     }