From 052db391cc6757ad6688744a6c1fead19ee71bf1 Mon Sep 17 00:00:00 2001 From: apiraino Date: Wed, 12 Jun 2024 12:27:59 +0200 Subject: [PATCH] Guard against overflowing GitHub ID comments Recently we overflowed the comment ID counter for comments on GitHub. This causes some comment IDs from this URL https://rfcbot.rs/api/all to be invalid. This patch is a follow-up to this commit https://github.com/rust-lang/rfcbot-rs/commit/f9d84fa85b54296896bcbb2cf04049f783912ea6 to avoid some tooling (notably meetings agenda generators) crashing when retrieving comments with invalid IDs. More info about this incident at: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/rfcbot.20asleep/near/442898636 --- src/github.rs | 28 +++++++++++++++++++++------- src/rfcbot.rs | 6 +++--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/github.rs b/src/github.rs index 9936249d..fe319572 100644 --- a/src/github.rs +++ b/src/github.rs @@ -510,7 +510,7 @@ impl Issue { self.state == IssueState::Open } - pub async fn get_comment(&self, client: &GithubClient, id: u64) -> anyhow::Result { + pub async fn get_comment(&self, client: &GithubClient, id: i32) -> anyhow::Result { let comment_url = format!("{}/issues/comments/{}", self.repository().url(client), id); let comment = client.json(client.get(&comment_url)).await?; Ok(comment) @@ -1825,11 +1825,25 @@ impl<'q> IssuesQuery for Query<'q> { issue.html_url, fcp.fcp.fk_bot_tracking_comment ); let bot_tracking_comment_content = quote_reply(&fcp.status_comment.body); - let fk_initiating_comment = fcp.fcp.fk_initiating_comment; - let init_comment = issue - .get_comment(&client, fk_initiating_comment.try_into()?) - .await?; + let (initiating_comment_html_url, initiating_comment_content) = + if u32::try_from(fk_initiating_comment).is_err() { + // We blew out the GH comment incremental counter (a i32 on their end) + // See: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/rfcbot.20asleep + log::debug!("Ignoring overflowed comment id from GitHub"); + ("".to_string(), "".to_string()) + } else { + let comment = issue + .get_comment(&client, fk_initiating_comment.try_into()?) + .await + .with_context(|| { + format!( + "failed to get first comment id={} for fcp={}", + fk_initiating_comment, fcp.fcp.id + ) + })?; + (comment.html_url, quote_reply(&comment.body)) + }; // TODO: agree with the team(s) a policy to emit actual mentions to remind FCP // voting member to cast their vote @@ -1837,8 +1851,8 @@ impl<'q> IssuesQuery for Query<'q> { Some(crate::actions::FCPDetails { bot_tracking_comment_html_url, bot_tracking_comment_content, - initiating_comment_html_url: init_comment.html_url.clone(), - initiating_comment_content: quote_reply(&init_comment.body), + initiating_comment_html_url, + initiating_comment_content, disposition: fcp .fcp .disposition diff --git a/src/rfcbot.rs b/src/rfcbot.rs index e3874548..da64f6aa 100644 --- a/src/rfcbot.rs +++ b/src/rfcbot.rs @@ -7,9 +7,9 @@ pub struct FCP { pub id: u32, pub fk_issue: u32, pub fk_initiator: u32, - pub fk_initiating_comment: u32, + pub fk_initiating_comment: i32, pub disposition: Option, - pub fk_bot_tracking_comment: u32, + pub fk_bot_tracking_comment: i32, pub fcp_start: Option, pub fcp_closed: bool, } @@ -50,7 +50,7 @@ pub struct FCPIssue { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct StatusComment { - pub id: u64, + pub id: i32, pub fk_issue: u32, pub fk_user: u32, pub body: String,