From 525da2fccbd90f3ca62fbba1e7a67ee619bee65f Mon Sep 17 00:00:00 2001 From: rituraj-tripathy_pinegit Date: Mon, 14 Jul 2025 13:07:25 +0530 Subject: [PATCH 01/23] implemented a rate limitter --- src/github/mod.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index d4fee07..df92003 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -12,6 +12,17 @@ use reqwest::{ header::{AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT}, }; use thiserror::Error; +use tokio::sync::Mutex; +use std::{ + sync::Arc, + time::Instant, +}; + +#[derive(Debug)] +struct RateLimitState { + remaining: u32, + reset_at: Instant, +} #[derive(Debug, Error)] pub enum GithubError { @@ -102,6 +113,7 @@ pub struct Labels; pub struct DefaultGithubClient { client: Client, graphql_url: String, + rate_limit: Arc>, } impl DefaultGithubClient { @@ -113,10 +125,13 @@ impl DefaultGithubClient { headers.insert(USER_AGENT, HeaderValue::from_static("github-activity-rs")); let client = reqwest::Client::builder().default_headers(headers).build()?; - + let initial_state = RateLimitState { + remaining: u32::MAX, + reset_at: Instant::now(), + }; tracing::debug!("HTTP client built successfully."); - Ok(Self { client, graphql_url: graphql_url.to_string() }) + Ok(Self { client, graphql_url: graphql_url.to_string(), rate_limit: Arc::new(Mutex::new(initial_state))}) } /// Re-usable configuration for exponential backoff. @@ -142,6 +157,10 @@ impl DefaultGithubClient { { // closure that Backoff expects let operation = || async { + + //0. Rate limit guard + self.rate_limit_guard().await; + // 1. Build the request let request_body = Q::build_query(variables.clone()); @@ -154,6 +173,9 @@ impl DefaultGithubClient { }, )?; + //2.5 Update rate limit state from headers + self.update_rate_limit_from_headers(resp.headers()); + // 3. HTTP-status check if !resp.status().is_success() { let status = resp.status(); @@ -235,6 +257,48 @@ impl DefaultGithubClient { // kick off the retry loop retry(Self::backoff_config(), operation).await } + + async fn rate_limit_guard(&self) { + let mut state = self.rate_limit.lock().await; + + // define a safety threshold + let threshold = 10; + if state.remaining <= threshold { + let now = Instant::now(); + if now < state.reset_at { + let wait = state.reset_at - now; + tracing::info!( + "Approaching rate limit ({} left). Sleeping {:?} until reset...", + state.remaining, + wait + ); + tokio::time::sleep(wait).await; + } + } + } + + fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { + let mut state = futures::executor::block_on(self.rate_limit.lock()); + if let (Some(rem), Some(reset)) = ( + headers.get("X-RateLimit-Remaining"), + headers.get("X-RateLimit-Reset"), + ) { + if let (Ok(rem), Ok(reset_ts)) = (rem.to_str(), reset.to_str()) { + if let (Ok(rem_n), Ok(reset_unix)) = (rem.parse::(), reset_ts.parse::()) { + state.remaining = rem_n; + // GitHub resets at a UNIX timestamp in seconds + let reset_in = reset_unix + .saturating_sub(chrono::Utc::now().timestamp() as u64); + state.reset_at = Instant::now() + Duration::from_secs(reset_in); + tracing::debug!( + "Rate limit updated: {} remaining, resets in {}s", + rem_n, + reset_in + ); + } + } + } + } } #[async_trait] From 3c275fc6e23ceb17105423a8d33f01c7905bd315 Mon Sep 17 00:00:00 2001 From: rituraj-tripathy_pinegit Date: Mon, 14 Jul 2025 22:55:55 +0530 Subject: [PATCH 02/23] adding threshold to config --- src/config.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/config.rs b/src/config.rs index 5901d60..0b550bd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,6 +6,7 @@ const DEFAULT_POLL_INTERVAL: u64 = 10; const DEFAULT_REPOS_PER_USER: usize = 20; const DEFAULT_LABELS_PER_REPO: usize = 10; const DEFAULT_MAX_CONCURRENCY: usize = 10; +const DEFAULT_RATE_LIMIT_THRESHOLD: u32 = 10; #[derive(Debug)] pub struct Config { @@ -17,6 +18,7 @@ pub struct Config { pub max_repos_per_user: usize, pub max_labels_per_repo: usize, pub max_concurrency: usize, + pub rate_limit_threshold: u32, } impl Config { @@ -44,6 +46,11 @@ impl Config { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_MAX_CONCURRENCY), + rate_limit_threshold: env::var("RATE_LIMIT_THRESHOLD") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(DEFAULT_RATE_LIMIT_THRESHOLD), // Default threshold for rate limiting + }) } } From a436850bfcd42719029e67a21235e1e4060ab9ee Mon Sep 17 00:00:00 2001 From: rituraj-tripathy_pinegit Date: Sun, 20 Jul 2025 08:47:44 +0530 Subject: [PATCH 03/23] fixing the update rate limit --- src/github/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index df92003..ee50dfd 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -259,7 +259,7 @@ impl DefaultGithubClient { } async fn rate_limit_guard(&self) { - let mut state = self.rate_limit.lock().await; + let state = self.rate_limit.lock().await; // define a safety threshold let threshold = 10; @@ -277,8 +277,8 @@ impl DefaultGithubClient { } } - fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { - let mut state = futures::executor::block_on(self.rate_limit.lock()); + async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { + let mut state = self.rate_limit.lock().await; if let (Some(rem), Some(reset)) = ( headers.get("X-RateLimit-Remaining"), headers.get("X-RateLimit-Reset"), From 3f1c76ed358c534d0c6844e9e50cf47bf9426192 Mon Sep 17 00:00:00 2001 From: rituraj-tripathy_pinegit Date: Sun, 20 Jul 2025 09:41:02 +0530 Subject: [PATCH 04/23] minor change --- src/github/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index ee50dfd..df92003 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -259,7 +259,7 @@ impl DefaultGithubClient { } async fn rate_limit_guard(&self) { - let state = self.rate_limit.lock().await; + let mut state = self.rate_limit.lock().await; // define a safety threshold let threshold = 10; @@ -277,8 +277,8 @@ impl DefaultGithubClient { } } - async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { - let mut state = self.rate_limit.lock().await; + fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { + let mut state = futures::executor::block_on(self.rate_limit.lock()); if let (Some(rem), Some(reset)) = ( headers.get("X-RateLimit-Remaining"), headers.get("X-RateLimit-Reset"), From 82bc48e3d099d1f75d439723314c390cec8d4fea Mon Sep 17 00:00:00 2001 From: riturajFi Date: Sun, 20 Jul 2025 10:04:09 +0530 Subject: [PATCH 05/23] dropping locka guard and reading the states only --- src/github/mod.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index df92003..8fd03ce 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -259,17 +259,20 @@ impl DefaultGithubClient { } async fn rate_limit_guard(&self) { - let mut state = self.rate_limit.lock().await; + let (remaining, reset_at) = { + let state = self.rate_limit.lock().await; // acquire lock + (state.remaining, state.reset_at) + }; // define a safety threshold let threshold = 10; - if state.remaining <= threshold { + if remaining <= threshold { let now = Instant::now(); - if now < state.reset_at { - let wait = state.reset_at - now; + if now < reset_at { + let wait = reset_at - now; tracing::info!( "Approaching rate limit ({} left). Sleeping {:?} until reset...", - state.remaining, + remaining, wait ); tokio::time::sleep(wait).await; From 591086c18502eea49895fe778dbc1dc51c8ade24 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Sun, 20 Jul 2025 10:05:44 +0530 Subject: [PATCH 06/23] minor commit --- src/github/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 8fd03ce..141cadc 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -174,7 +174,7 @@ impl DefaultGithubClient { )?; //2.5 Update rate limit state from headers - self.update_rate_limit_from_headers(resp.headers()); + self.update_rate_limit_from_headers(resp.headers()).await; // 3. HTTP-status check if !resp.status().is_success() { @@ -280,8 +280,8 @@ impl DefaultGithubClient { } } - fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { - let mut state = futures::executor::block_on(self.rate_limit.lock()); + async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { + let mut state = self.rate_limit.lock().await; if let (Some(rem), Some(reset)) = ( headers.get("X-RateLimit-Remaining"), headers.get("X-RateLimit-Reset"), From e049d6c3f9a88b53c78fc1ba0a29f6ab82090a04 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Sun, 20 Jul 2025 10:14:19 +0530 Subject: [PATCH 07/23] added jitter --- src/github/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 141cadc..8bb568f 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -2,6 +2,7 @@ mod tests; use std::{collections::HashSet, time::Duration}; +use rand::Rng; use async_trait::async_trait; use backoff::{Error as BackoffError, ExponentialBackoff, future::retry}; @@ -275,7 +276,11 @@ impl DefaultGithubClient { remaining, wait ); - tokio::time::sleep(wait).await; + + // Sleep until the rate limit resets + //added a jitter to avoid thundering herd problem + let jitter_ms = rand::thread_rng().gen_range(0..500); + tokio::time::sleep(jitter_ms).await; } } } From befb1e06e11e0b4649a1171cded4d534afe93ba1 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Sun, 20 Jul 2025 11:18:58 +0530 Subject: [PATCH 08/23] fixed the jitter issue --- Cargo.lock | 1 + Cargo.toml | 1 + src/github/mod.rs | 11 +++++++---- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b53694b..1b752a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -727,6 +727,7 @@ dependencies = [ "graphql_client", "lazy_static", "mockall", + "rand 0.9.1", "reqwest 0.12.15", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 1f7eb45..6cf1d95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,3 +24,4 @@ thiserror = "2.0.12" tracing = "0.1.41" tracing-subscriber = { version = "0.3.19", features = ["env-filter", "fmt"] } futures = "0.3.31" +rand = "0.9.1" diff --git a/src/github/mod.rs b/src/github/mod.rs index 8bb568f..256fb0d 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -2,7 +2,7 @@ mod tests; use std::{collections::HashSet, time::Duration}; -use rand::Rng; +use rand::{rng, Rng}; use async_trait::async_trait; use backoff::{Error as BackoffError, ExponentialBackoff, future::retry}; @@ -279,8 +279,9 @@ impl DefaultGithubClient { // Sleep until the rate limit resets //added a jitter to avoid thundering herd problem - let jitter_ms = rand::thread_rng().gen_range(0..500); - tokio::time::sleep(jitter_ms).await; + let max_jitter = wait.as_millis() as u64 / 10; + let jitter_ms = rng().random_range(0..=max_jitter); + tokio::time::sleep(wait + Duration::from_millis(jitter_ms)).await; } } } @@ -291,13 +292,15 @@ impl DefaultGithubClient { headers.get("X-RateLimit-Remaining"), headers.get("X-RateLimit-Reset"), ) { + + if let (Ok(rem), Ok(reset_ts)) = (rem.to_str(), reset.to_str()) { if let (Ok(rem_n), Ok(reset_unix)) = (rem.parse::(), reset_ts.parse::()) { state.remaining = rem_n; // GitHub resets at a UNIX timestamp in seconds let reset_in = reset_unix .saturating_sub(chrono::Utc::now().timestamp() as u64); - state.reset_at = Instant::now() + Duration::from_secs(reset_in); + state.reset_at = Instant::now() + Duration::from_secs(reset_in); tracing::debug!( "Rate limit updated: {} remaining, resets in {}s", rem_n, From 0184f8f5dffb4b034073b914143bb61c661e58b8 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Sun, 20 Jul 2025 20:55:14 +0530 Subject: [PATCH 09/23] fixed error handling for headers --- src/config.rs | 6 +-- src/github/mod.rs | 120 +++++++++++++++++++++++++++++++++++--------- src/github/tests.rs | 33 ++++++++++-- src/main.rs | 1 + src/poller/mod.rs | 3 ++ 5 files changed, 132 insertions(+), 31 deletions(-) diff --git a/src/config.rs b/src/config.rs index 0b550bd..00a4e28 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,7 +6,7 @@ const DEFAULT_POLL_INTERVAL: u64 = 10; const DEFAULT_REPOS_PER_USER: usize = 20; const DEFAULT_LABELS_PER_REPO: usize = 10; const DEFAULT_MAX_CONCURRENCY: usize = 10; -const DEFAULT_RATE_LIMIT_THRESHOLD: u32 = 10; +const DEFAULT_RATE_LIMIT_THRESHOLD: u64 = 10; #[derive(Debug)] pub struct Config { @@ -18,7 +18,7 @@ pub struct Config { pub max_repos_per_user: usize, pub max_labels_per_repo: usize, pub max_concurrency: usize, - pub rate_limit_threshold: u32, + pub rate_limit_threshold: u64, } impl Config { @@ -49,7 +49,7 @@ impl Config { rate_limit_threshold: env::var("RATE_LIMIT_THRESHOLD") .ok() .and_then(|v| v.parse().ok()) - .unwrap_or(DEFAULT_RATE_LIMIT_THRESHOLD), // Default threshold for rate limiting + .unwrap_or(DEFAULT_RATE_LIMIT_THRESHOLD), }) } diff --git a/src/github/mod.rs b/src/github/mod.rs index 256fb0d..77998e5 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -45,6 +45,8 @@ pub enum GithubError { RateLimited, #[error("GitHub authentication failed")] Unauthorized, + #[error("Failed to parse header: {0}")] + HeaderError(String), } // Helper function to check if a GraphQL error is retryable @@ -115,10 +117,11 @@ pub struct DefaultGithubClient { client: Client, graphql_url: String, rate_limit: Arc>, + rate_limit_threshhold: u64 } impl DefaultGithubClient { - pub fn new(github_token: &str, graphql_url: &str) -> Result { + pub fn new(github_token: &str, graphql_url: &str, rate_limit_threshhold: u64) -> Result { // Build the HTTP client with the GitHub token. let mut headers = HeaderMap::new(); @@ -132,7 +135,7 @@ impl DefaultGithubClient { }; tracing::debug!("HTTP client built successfully."); - Ok(Self { client, graphql_url: graphql_url.to_string(), rate_limit: Arc::new(Mutex::new(initial_state))}) + Ok(Self { client, graphql_url: graphql_url.to_string(), rate_limit: Arc::new(Mutex::new(initial_state)), rate_limit_threshhold}) } /// Re-usable configuration for exponential backoff. @@ -176,6 +179,10 @@ impl DefaultGithubClient { //2.5 Update rate limit state from headers self.update_rate_limit_from_headers(resp.headers()).await; + if let Err(e) = self.update_rate_limit_from_headers(resp.headers()).await { + // Option A: warn and continue + tracing::warn!("Could not update rate-limit info: {}", e); + } // 3. HTTP-status check if !resp.status().is_success() { @@ -259,6 +266,7 @@ impl DefaultGithubClient { retry(Self::backoff_config(), operation).await } + /// Rate limit guard that sleeps until the rate limit resets if we're close to the threshold. async fn rate_limit_guard(&self) { let (remaining, reset_at) = { let state = self.rate_limit.lock().await; // acquire lock @@ -266,7 +274,7 @@ impl DefaultGithubClient { }; // define a safety threshold - let threshold = 10; + let threshold = self.rate_limit_threshhold as u32; if remaining <= threshold { let now = Instant::now(); if now < reset_at { @@ -286,30 +294,92 @@ impl DefaultGithubClient { } } - async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) { + // Update the rate limit state from the HTTP headers. + // async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) -> Result<(), GithubError> { + // let mut state = self.rate_limit.lock().await; + // let rem_val = headers.get("X-RateLimit-Remaining").or_else(|| { + // tracing::error!("{}", "Missing X-RateLimit-Remaining header".to_string()); + // GithubError::HeaderError(("Missing X-RateLimit-Remaining header".to_string())); + // }); + // if let (Some(rem), Some(reset)) = ( + // headers.get("X-RateLimit-Remaining"), + // headers.get("X-RateLimit-Reset"), + // ) { + + + // if let (Ok(rem), Ok(reset_ts)) = (rem.to_str(), reset.to_str()) { + // if let (Ok(rem_n), Ok(reset_unix)) = (rem.parse::(), reset_ts.parse::()) { + // state.remaining = rem_n; + // // GitHub resets at a UNIX timestamp in seconds + // let reset_in = reset_unix + // .saturating_sub(chrono::Utc::now().timestamp() as u64); + // state.reset_at = Instant::now() + Duration::from_secs(reset_in); + // tracing::debug!( + // "Rate limit updated: {} remaining, resets in {}s", + // rem_n, + // reset_in + // ); + // } + // } + // } + // } + + async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) + -> Result<(), GithubError> + { + // Names are case-insensitive in HeaderMap + let rem_val = headers + .get("X-RateLimit-Remaining") + .ok_or_else(|| { + let msg = "Missing X-RateLimit-Remaining header".to_string(); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + let rem_str = rem_val.to_str().map_err(|e| { + let msg = format!("Invalid X-RateLimit-Remaining value: {e}"); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + let remaining = rem_str.parse::().map_err(|e| { + let msg = format!("Cannot parse remaining as u32: {e}"); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + + let reset_val = headers + .get("X-RateLimit-Reset") + .ok_or_else(|| { + let msg = "Missing X-RateLimit-Reset header".to_string(); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + let reset_str = reset_val.to_str().map_err(|e| { + let msg = format!("Invalid X-RateLimit-Reset value: {e}"); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + let reset_unix = reset_str.parse::().map_err(|e| { + let msg = format!("Cannot parse reset timestamp as u64: {e}"); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; + + // All good — update the shared state let mut state = self.rate_limit.lock().await; - if let (Some(rem), Some(reset)) = ( - headers.get("X-RateLimit-Remaining"), - headers.get("X-RateLimit-Reset"), - ) { - - - if let (Ok(rem), Ok(reset_ts)) = (rem.to_str(), reset.to_str()) { - if let (Ok(rem_n), Ok(reset_unix)) = (rem.parse::(), reset_ts.parse::()) { - state.remaining = rem_n; - // GitHub resets at a UNIX timestamp in seconds - let reset_in = reset_unix - .saturating_sub(chrono::Utc::now().timestamp() as u64); - state.reset_at = Instant::now() + Duration::from_secs(reset_in); - tracing::debug!( - "Rate limit updated: {} remaining, resets in {}s", - rem_n, - reset_in - ); - } - } - } + state.remaining = remaining; + let reset_in = reset_unix + .saturating_sub(chrono::Utc::now().timestamp() as u64); + state.reset_at = Instant::now() + Duration::from_secs(reset_in); + + tracing::debug!( + "Rate limit updated: {} remaining, resets in {}s", + remaining, + reset_in + ); + Ok(()) } + + } #[async_trait] diff --git a/src/github/tests.rs b/src/github/tests.rs index e533ee3..25c4f41 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -1,10 +1,8 @@ use std::collections::HashMap; - use super::*; - #[test] fn test_new_github_client() { - let client = DefaultGithubClient::new("test_token", "https://api.github.com/graphql"); + let client = DefaultGithubClient::new("test_token", "https://api.github.com/graphql", 10); assert!(client.is_ok()); } @@ -37,3 +35,32 @@ fn test_is_not_retryable_graphql_error() { assert!(!is_retryable_graphql_error(&error)); } + +#[tokio::test] + async fn test_update_rate_limit_from_headers() { + let client = DefaultGithubClient::new("fake", "https://api.github.com/graphql", 5) + .expect("client init"); + + // Build fake headers with remaining=3, reset in 60s + let mut headers = HeaderMap::new(); + headers.insert( + "X-RateLimit-Remaining", + HeaderValue::from_static("3"), + ); + let reset_ts = (chrono::Utc::now().timestamp() as u64) + 60; + headers.insert( + "X-RateLimit-Reset", + HeaderValue::from_str(&reset_ts.to_string()).unwrap(), + ); + + client.update_rate_limit_from_headers(&headers).await; + + let state = client.rate_limit.lock().await; + assert_eq!(state.remaining, 3); + // We expect reset_at ≈ now + 60s (within a small delta) + let diff = state + .reset_at + .checked_duration_since(Instant::now()) + .unwrap(); + assert!(diff >= Duration::from_secs(59) && diff <= Duration::from_secs(61)); + } \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index 9c2dee9..396240d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -56,6 +56,7 @@ async fn run() -> Result<(), Box> { let github_client = Arc::new(github::DefaultGithubClient::new( &config.github_token, &config.github_graphql_url, + config.rate_limit_threshold )?); let messaging_service = Arc::new(TelegramMessagingService::new(bot.clone())); diff --git a/src/poller/mod.rs b/src/poller/mod.rs index 8609ec8..33f1763 100644 --- a/src/poller/mod.rs +++ b/src/poller/mod.rs @@ -195,6 +195,9 @@ impl GithubPoller { ); return Err(PollerError::Github(github_error)); } + GithubError::HeaderError(_) => { + tracing::error!( + "Header error while polling issues for repository"); } }, unexpected_error => { tracing::error!( From 5ded0b97aeecf8602be3cbe8f89fb73f2ec758d8 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Tue, 22 Jul 2025 11:41:44 +0530 Subject: [PATCH 10/23] refactor: removed commented function for cleanup --- src/github/mod.rs | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 77998e5..d34981f 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -294,36 +294,6 @@ impl DefaultGithubClient { } } - // Update the rate limit state from the HTTP headers. - // async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) -> Result<(), GithubError> { - // let mut state = self.rate_limit.lock().await; - // let rem_val = headers.get("X-RateLimit-Remaining").or_else(|| { - // tracing::error!("{}", "Missing X-RateLimit-Remaining header".to_string()); - // GithubError::HeaderError(("Missing X-RateLimit-Remaining header".to_string())); - // }); - // if let (Some(rem), Some(reset)) = ( - // headers.get("X-RateLimit-Remaining"), - // headers.get("X-RateLimit-Reset"), - // ) { - - - // if let (Ok(rem), Ok(reset_ts)) = (rem.to_str(), reset.to_str()) { - // if let (Ok(rem_n), Ok(reset_unix)) = (rem.parse::(), reset_ts.parse::()) { - // state.remaining = rem_n; - // // GitHub resets at a UNIX timestamp in seconds - // let reset_in = reset_unix - // .saturating_sub(chrono::Utc::now().timestamp() as u64); - // state.reset_at = Instant::now() + Duration::from_secs(reset_in); - // tracing::debug!( - // "Rate limit updated: {} remaining, resets in {}s", - // rem_n, - // reset_in - // ); - // } - // } - // } - // } - async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) -> Result<(), GithubError> { From 39fda41e5017bd11fdf46182399a870dd881fd17 Mon Sep 17 00:00:00 2001 From: isSerge Date: Tue, 22 Jul 2025 14:47:40 +0700 Subject: [PATCH 11/23] docs: update installation instructions to include prerequisites and database setup (#76) --- readme.md | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/readme.md b/readme.md index 56cb8ba..c01ef07 100644 --- a/readme.md +++ b/readme.md @@ -73,7 +73,15 @@ src cd good-first-bot-rs ``` -2. **Set up environment variables:** +2. **Prerequisites:** + + - **Rust:** Ensure you have [Rust](https://www.rust-lang.org/tools/install) installed. + - **sqlx-cli:** Install the SQLx command-line tool for database migrations. + ```bash + cargo install sqlx-cli + ``` + +3. **Set up environment variables:** Create a .env file in the project root with the following keys (values are examples): @@ -103,10 +111,18 @@ MAX_LABELS_PER_REPO=5 - MAX_LABELS_PER_REPO: (Optional) Maximum number of labels per repository a user can track. Default is 10 -3. **Install Dependencies:** +4. **Database Setup:** + +Run the following command to set up the database and apply migrations: + +```bash +mkdir data +sqlx database setup +``` + +5. **Install Dependencies:** -Ensure you have [Rust](https://www.rust-lang.org/tools/install) installed. Then, -in the project directory run: +In the project directory run: ```bash cargo build From bbd3fe32a1d7ef6e9843bd05461fa87719720ee2 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Tue, 22 Jul 2025 18:46:57 +0530 Subject: [PATCH 12/23] fix:removed double call to update_rate_limit --- src/github/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index d34981f..750bedf 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -177,14 +177,13 @@ impl DefaultGithubClient { }, )?; - //2.5 Update rate limit state from headers - self.update_rate_limit_from_headers(resp.headers()).await; + //3 Update rate limit state from headers if let Err(e) = self.update_rate_limit_from_headers(resp.headers()).await { // Option A: warn and continue tracing::warn!("Could not update rate-limit info: {}", e); } - // 3. HTTP-status check + // 4. HTTP-status check if !resp.status().is_success() { let status = resp.status(); let text = resp.text().await.unwrap_or_else(|e| { @@ -228,7 +227,7 @@ impl DefaultGithubClient { return Err(be); } - // 4. Parse JSON + // 5. Parse JSON let body: Response = resp.json().await.map_err(|e| { tracing::warn!("Failed to parse JSON: {e}. Retrying..."); BackoffError::transient(GithubError::GraphQLApiError(format!( @@ -236,7 +235,7 @@ impl DefaultGithubClient { ))) })?; - // 5. GraphQL errors? + // 6. GraphQL errors? if let Some(errors) = &body.errors { let is_rate_limit_error = errors.iter().any(|e| { e.message.to_lowercase().contains("rate limit") || is_retryable_graphql_error(e) @@ -253,7 +252,7 @@ impl DefaultGithubClient { } } - // 6. Unwrap the data or permanent-fail + // 7. Unwrap the data or permanent-fail body.data.ok_or_else(|| { tracing::error!("GraphQL response had no data field; permanent failure"); BackoffError::permanent(GithubError::GraphQLApiError( From 2656e1b08c4e5069fe8797bd1ad6438350fa87b6 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Tue, 22 Jul 2025 19:10:24 +0530 Subject: [PATCH 13/23] refactor:fixed formatting and changed some field names --- src/bot_handler/callbacks/toggle_label.rs | 5 +- src/config.rs | 1 - src/github/mod.rs | 82 ++++++++++------------- src/github/tests.rs | 47 ++++++------- src/main.rs | 2 +- src/messaging/utils.rs | 5 +- src/poller/mod.rs | 4 +- 7 files changed, 65 insertions(+), 81 deletions(-) diff --git a/src/bot_handler/callbacks/toggle_label.rs b/src/bot_handler/callbacks/toggle_label.rs index f97c29e..8a5d9df 100644 --- a/src/bot_handler/callbacks/toggle_label.rs +++ b/src/bot_handler/callbacks/toggle_label.rs @@ -18,10 +18,11 @@ pub async fn handle(ctx: Context<'_>, label_name: &str, label_page: usize) -> Bo let (repo_id, from_page) = match dialogue_state { Some(CommandState::ViewingRepoLabels { repo_id, from_page }) => (repo_id, from_page), - _ => + _ => { return Err(BotHandlerError::InvalidInput( "Invalid state: expected ViewingRepoLabels".to_string(), - )), + )); + } }; let repo = diff --git a/src/config.rs b/src/config.rs index 00a4e28..bd35c78 100644 --- a/src/config.rs +++ b/src/config.rs @@ -50,7 +50,6 @@ impl Config { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_RATE_LIMIT_THRESHOLD), - }) } } diff --git a/src/github/mod.rs b/src/github/mod.rs index 750bedf..5ce6bbb 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -1,8 +1,8 @@ #[cfg(test)] mod tests; +use rand::{Rng, rng}; use std::{collections::HashSet, time::Duration}; -use rand::{rng, Rng}; use async_trait::async_trait; use backoff::{Error as BackoffError, ExponentialBackoff, future::retry}; @@ -12,12 +12,9 @@ use reqwest::{ Client, header::{AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT}, }; +use std::{sync::Arc, time::Instant}; use thiserror::Error; use tokio::sync::Mutex; -use std::{ - sync::Arc, - time::Instant, -}; #[derive(Debug)] struct RateLimitState { @@ -117,11 +114,15 @@ pub struct DefaultGithubClient { client: Client, graphql_url: String, rate_limit: Arc>, - rate_limit_threshhold: u64 + rate_limit_threshold: u64, } impl DefaultGithubClient { - pub fn new(github_token: &str, graphql_url: &str, rate_limit_threshhold: u64) -> Result { + pub fn new( + github_token: &str, + graphql_url: &str, + rate_limit_threshold: u64, + ) -> Result { // Build the HTTP client with the GitHub token. let mut headers = HeaderMap::new(); @@ -129,13 +130,15 @@ impl DefaultGithubClient { headers.insert(USER_AGENT, HeaderValue::from_static("github-activity-rs")); let client = reqwest::Client::builder().default_headers(headers).build()?; - let initial_state = RateLimitState { - remaining: u32::MAX, - reset_at: Instant::now(), - }; + let initial_state = RateLimitState { remaining: u32::MAX, reset_at: Instant::now() }; tracing::debug!("HTTP client built successfully."); - Ok(Self { client, graphql_url: graphql_url.to_string(), rate_limit: Arc::new(Mutex::new(initial_state)), rate_limit_threshhold}) + Ok(Self { + client, + graphql_url: graphql_url.to_string(), + rate_limit: Arc::new(Mutex::new(initial_state)), + rate_limit_threshold, + }) } /// Re-usable configuration for exponential backoff. @@ -161,7 +164,6 @@ impl DefaultGithubClient { { // closure that Backoff expects let operation = || async { - //0. Rate limit guard self.rate_limit_guard().await; @@ -212,8 +214,9 @@ impl DefaultGithubClient { )) } } - reqwest::StatusCode::NOT_FOUND => - GithubError::GraphQLApiError(format!("HTTP Not Found ({status}): {text}")), + reqwest::StatusCode::NOT_FOUND => { + GithubError::GraphQLApiError(format!("HTTP Not Found ({status}): {text}")) + } _ => GithubError::GraphQLApiError(format!("HTTP Error ({status}): {text}")), }; @@ -221,7 +224,9 @@ impl DefaultGithubClient { GithubError::RateLimited => BackoffError::transient(github_err), _ if status.is_server_error() || status == reqwest::StatusCode::TOO_MANY_REQUESTS => - BackoffError::transient(github_err), + { + BackoffError::transient(github_err) + } _ => BackoffError::permanent(github_err), }; return Err(be); @@ -268,12 +273,12 @@ impl DefaultGithubClient { /// Rate limit guard that sleeps until the rate limit resets if we're close to the threshold. async fn rate_limit_guard(&self) { let (remaining, reset_at) = { - let state = self.rate_limit.lock().await; // acquire lock + let state = self.rate_limit.lock().await; // acquire lock (state.remaining, state.reset_at) }; // define a safety threshold - let threshold = self.rate_limit_threshhold as u32; + let threshold = self.rate_limit_threshold as u32; if remaining <= threshold { let now = Instant::now(); if now < reset_at { @@ -287,23 +292,19 @@ impl DefaultGithubClient { // Sleep until the rate limit resets //added a jitter to avoid thundering herd problem let max_jitter = wait.as_millis() as u64 / 10; - let jitter_ms = rng().random_range(0..=max_jitter); + let jitter_ms = rng().random_range(0..=max_jitter); tokio::time::sleep(wait + Duration::from_millis(jitter_ms)).await; } } } - async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) - -> Result<(), GithubError> - { + async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) -> Result<(), GithubError> { // Names are case-insensitive in HeaderMap - let rem_val = headers - .get("X-RateLimit-Remaining") - .ok_or_else(|| { - let msg = "Missing X-RateLimit-Remaining header".to_string(); - tracing::error!("{}", msg); - GithubError::HeaderError(msg) - })?; + let rem_val = headers.get("X-RateLimit-Remaining").ok_or_else(|| { + let msg = "Missing X-RateLimit-Remaining header".to_string(); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; let rem_str = rem_val.to_str().map_err(|e| { let msg = format!("Invalid X-RateLimit-Remaining value: {e}"); tracing::error!("{}", msg); @@ -315,13 +316,11 @@ impl DefaultGithubClient { GithubError::HeaderError(msg) })?; - let reset_val = headers - .get("X-RateLimit-Reset") - .ok_or_else(|| { - let msg = "Missing X-RateLimit-Reset header".to_string(); - tracing::error!("{}", msg); - GithubError::HeaderError(msg) - })?; + let reset_val = headers.get("X-RateLimit-Reset").ok_or_else(|| { + let msg = "Missing X-RateLimit-Reset header".to_string(); + tracing::error!("{}", msg); + GithubError::HeaderError(msg) + })?; let reset_str = reset_val.to_str().map_err(|e| { let msg = format!("Invalid X-RateLimit-Reset value: {e}"); tracing::error!("{}", msg); @@ -336,19 +335,12 @@ impl DefaultGithubClient { // All good — update the shared state let mut state = self.rate_limit.lock().await; state.remaining = remaining; - let reset_in = reset_unix - .saturating_sub(chrono::Utc::now().timestamp() as u64); + let reset_in = reset_unix.saturating_sub(chrono::Utc::now().timestamp() as u64); state.reset_at = Instant::now() + Duration::from_secs(reset_in); - tracing::debug!( - "Rate limit updated: {} remaining, resets in {}s", - remaining, - reset_in - ); + tracing::debug!("Rate limit updated: {} remaining, resets in {}s", remaining, reset_in); Ok(()) } - - } #[async_trait] diff --git a/src/github/tests.rs b/src/github/tests.rs index 25c4f41..76a5b94 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -1,5 +1,5 @@ -use std::collections::HashMap; use super::*; +use std::collections::HashMap; #[test] fn test_new_github_client() { let client = DefaultGithubClient::new("test_token", "https://api.github.com/graphql", 10); @@ -37,30 +37,21 @@ fn test_is_not_retryable_graphql_error() { } #[tokio::test] - async fn test_update_rate_limit_from_headers() { - let client = DefaultGithubClient::new("fake", "https://api.github.com/graphql", 5) - .expect("client init"); - - // Build fake headers with remaining=3, reset in 60s - let mut headers = HeaderMap::new(); - headers.insert( - "X-RateLimit-Remaining", - HeaderValue::from_static("3"), - ); - let reset_ts = (chrono::Utc::now().timestamp() as u64) + 60; - headers.insert( - "X-RateLimit-Reset", - HeaderValue::from_str(&reset_ts.to_string()).unwrap(), - ); - - client.update_rate_limit_from_headers(&headers).await; - - let state = client.rate_limit.lock().await; - assert_eq!(state.remaining, 3); - // We expect reset_at ≈ now + 60s (within a small delta) - let diff = state - .reset_at - .checked_duration_since(Instant::now()) - .unwrap(); - assert!(diff >= Duration::from_secs(59) && diff <= Duration::from_secs(61)); - } \ No newline at end of file +async fn test_update_rate_limit_from_headers() { + let client = + DefaultGithubClient::new("fake", "https://api.github.com/graphql", 5).expect("client init"); + + // Build fake headers with remaining=3, reset in 60s + let mut headers = HeaderMap::new(); + headers.insert("X-RateLimit-Remaining", HeaderValue::from_static("3")); + let reset_ts = (chrono::Utc::now().timestamp() as u64) + 60; + headers.insert("X-RateLimit-Reset", HeaderValue::from_str(&reset_ts.to_string()).unwrap()); + + client.update_rate_limit_from_headers(&headers).await; + + let state = client.rate_limit.lock().await; + assert_eq!(state.remaining, 3); + // We expect reset_at ≈ now + 60s (within a small delta) + let diff = state.reset_at.checked_duration_since(Instant::now()).unwrap(); + assert!(diff >= Duration::from_secs(59) && diff <= Duration::from_secs(61)); +} diff --git a/src/main.rs b/src/main.rs index 396240d..f3ff22c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -56,7 +56,7 @@ async fn run() -> Result<(), Box> { let github_client = Arc::new(github::DefaultGithubClient::new( &config.github_token, &config.github_graphql_url, - config.rate_limit_threshold + config.rate_limit_threshold, )?); let messaging_service = Arc::new(TelegramMessagingService::new(bot.clone())); diff --git a/src/messaging/utils.rs b/src/messaging/utils.rs index d84658f..11da912 100644 --- a/src/messaging/utils.rs +++ b/src/messaging/utils.rs @@ -13,8 +13,9 @@ pub fn github_color_to_emoji(hex_color: &str) -> &str { "fef2c0" | "fbca04" | "e4e669" | "ffeb3b" | "f9e076" | "fadc73" => "🟡", // Greens - "0e8a16" | "006b75" | "5ab302" | "a2eeef" | "008672" | "c2e0c6" | "1aa34a" | "4caf50" => - "🟢", + "0e8a16" | "006b75" | "5ab302" | "a2eeef" | "008672" | "c2e0c6" | "1aa34a" | "4caf50" => { + "🟢" + } // Blues / Teals "0052cc" | "c5def5" | "0075ca" | "1d76db" | "89d2fc" | "00bcd4" | "b3f4f4" => "🔵", diff --git a/src/poller/mod.rs b/src/poller/mod.rs index 33f1763..3e80f59 100644 --- a/src/poller/mod.rs +++ b/src/poller/mod.rs @@ -196,8 +196,8 @@ impl GithubPoller { return Err(PollerError::Github(github_error)); } GithubError::HeaderError(_) => { - tracing::error!( - "Header error while polling issues for repository"); } + tracing::error!("Header error while polling issues for repository"); + } }, unexpected_error => { tracing::error!( From 0f7641d26f4ce275c3a76ee26a3b0f2fc4cd369c Mon Sep 17 00:00:00 2001 From: riturajFi Date: Wed, 23 Jul 2025 22:55:40 +0530 Subject: [PATCH 14/23] template for testing the rate limit funciton --- src/github/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/github/tests.rs b/src/github/tests.rs index 76a5b94..3c89467 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -55,3 +55,7 @@ async fn test_update_rate_limit_from_headers() { let diff = state.reset_at.checked_duration_since(Instant::now()).unwrap(); assert!(diff >= Duration::from_secs(59) && diff <= Duration::from_secs(61)); } + +async fn test_rate_limit_guard(){ + +} From ca5bca8eea1c753b8719e2964a4b727fd4cb80d6 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 01:53:40 +0530 Subject: [PATCH 15/23] test:adding test for rate_limmitting --- src/github/mod.rs | 2 +- src/github/tests.rs | 38 +++++++++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 5ce6bbb..3f52a41 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -273,7 +273,7 @@ impl DefaultGithubClient { /// Rate limit guard that sleeps until the rate limit resets if we're close to the threshold. async fn rate_limit_guard(&self) { let (remaining, reset_at) = { - let state = self.rate_limit.lock().await; // acquire lock + let state = self.rate_limit.lock().await; (state.remaining, state.reset_at) }; diff --git a/src/github/tests.rs b/src/github/tests.rs index 3c89467..a87101f 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -56,6 +56,38 @@ async fn test_update_rate_limit_from_headers() { assert!(diff >= Duration::from_secs(59) && diff <= Duration::from_secs(61)); } -async fn test_rate_limit_guard(){ - -} +#[tokio::test(flavor = "multi_thread")] +async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { + // -------- Arrange -------- + let threshold = 5; + let client = DefaultGithubClient::new("fake_token", "https://example.com/graphql", threshold as u64) + .expect("client"); + + // Force a short wait window + const WAIT_MS: u64 = 40; + { + let mut state = client.rate_limit.lock().await; + state.remaining = threshold; // at or below threshold + state.reset_at = Instant::now() + Duration::from_millis(WAIT_MS); + } + + // Bounds: wait .. wait + wait/10 (+ a little fudge for scheduler noise) + let expected_min = Duration::from_millis(WAIT_MS); + let expected_max = Duration::from_millis(WAIT_MS + WAIT_MS / 10); + let fudge = Duration::from_millis(10); + + // -------- Act -------- + let start = Instant::now(); + client.rate_limit_guard().await; + let elapsed = start.elapsed(); + + // -------- Assert -------- + assert!( + elapsed >= expected_min, + "Guard returned too fast: {:?} < {:?}", elapsed, expected_min + ); + assert!( + elapsed <= expected_max + fudge, + "Guard slept too long: {:?} > {:?}", elapsed, expected_max + fudge + ); +} \ No newline at end of file From 8db7442536998a1296389164bc8f52e63ca36c1a Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 01:55:06 +0530 Subject: [PATCH 16/23] minor change --- src/github/tests.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/github/tests.rs b/src/github/tests.rs index a87101f..f9d98ce 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -63,15 +63,13 @@ async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { let client = DefaultGithubClient::new("fake_token", "https://example.com/graphql", threshold as u64) .expect("client"); - // Force a short wait window const WAIT_MS: u64 = 40; { let mut state = client.rate_limit.lock().await; - state.remaining = threshold; // at or below threshold + state.remaining = threshold; state.reset_at = Instant::now() + Duration::from_millis(WAIT_MS); } - // Bounds: wait .. wait + wait/10 (+ a little fudge for scheduler noise) let expected_min = Duration::from_millis(WAIT_MS); let expected_max = Duration::from_millis(WAIT_MS + WAIT_MS / 10); let fudge = Duration::from_millis(10); From fcef88669d9fbfb2206555c2a54755ac0a505e80 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 02:17:08 +0530 Subject: [PATCH 17/23] jitter tests --- src/github/tests.rs | 119 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/src/github/tests.rs b/src/github/tests.rs index f9d98ce..241be72 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -88,4 +88,123 @@ async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { elapsed <= expected_max + fudge, "Guard slept too long: {:?} > {:?}", elapsed, expected_max + fudge ); +} + +/// Helper: set the shared rate-limit state so the guard will sleep `wait_ms` plus jitter. +async fn prime_state(client: &DefaultGithubClient, remaining: u32, wait_ms: u64) { + let mut s = client.rate_limit.lock().await; + s.remaining = remaining; + s.reset_at = Instant::now() + Duration::from_millis(wait_ms); +} + +/// Helper: run the guard once and return how long it actually waited. +async fn measure_sleep(client: &DefaultGithubClient) -> Duration { + let start = Instant::now(); + client.rate_limit_guard().await; + start.elapsed() +} + +/// 1) Always inside [wait, wait + 10%] (with a little fudge for scheduler noise) +#[tokio::test(flavor = "multi_thread")] +async fn jitter_is_within_bounds() { + const THRESHOLD: u64 = 5; + const WAIT_MS: u64 = 50; + const FUDGE_MS: u64 = 8; + + let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) + .expect("client"); + + // Force a sleep path + prime_state(&client, THRESHOLD as u32, WAIT_MS).await; + + let expected_min = Duration::from_millis(WAIT_MS); + let expected_max = Duration::from_millis(WAIT_MS + WAIT_MS / 10); // 10% jitter + let fudge = Duration::from_millis(FUDGE_MS); + + let elapsed = measure_sleep(&client).await; + + assert!( + elapsed >= expected_min, + "Returned too fast: {:?} < {:?}", + elapsed, + expected_min + ); + assert!( + elapsed <= expected_max + fudge, + "Slept too long: {:?} > {:?}", + elapsed, + expected_max + fudge + ); +} + +/// 2) We actually *get* jitter sometimes (i.e., not always exactly WAIT_MS). +/// Run the guard a bunch of times and check the spread of durations. +#[tokio::test(flavor = "multi_thread")] +async fn jitter_varies_across_runs() { + const THRESHOLD: u64 = 3; + const WAIT_MS: u64 = 40; + const RUNS: usize = 20; + const FUDGE_MS: u64 = 8; + + let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) + .expect("client"); + + let mut samples = Vec::with_capacity(RUNS); + + for _ in 0..RUNS { + prime_state(&client, THRESHOLD as u32, WAIT_MS).await; + samples.push(measure_sleep(&client).await); + } + + let min = samples.iter().min().cloned().unwrap(); + let max = samples.iter().max().cloned().unwrap(); + + let base = Duration::from_millis(WAIT_MS); + let jitter_span = Duration::from_millis(WAIT_MS / 10); + + // Same bounds check as safety + for (i, dur) in samples.iter().enumerate() { + assert!( + *dur >= base, + "Run {i}: {:?} < base {:?}", + dur, + base + ); + assert!( + *dur <= base + jitter_span + Duration::from_millis(FUDGE_MS), + "Run {i}: {:?} > upper bound {:?}", + dur, + base + jitter_span + Duration::from_millis(FUDGE_MS) + ); + } + + // And now confirm we saw at least ~some spread (non‑deterministic, so we only require >1ms spread). + assert!( + max > min + Duration::from_millis(1), + "Jitter didn't vary enough: min={:?}, max={:?}", + min, + max + ); +} + +/// 3) When wait == 0, max_jitter == 0 → no sleep at all. +#[tokio::test(flavor = "multi_thread")] +async fn no_jitter_when_wait_is_zero() { + const THRESHOLD: u64 = 1; + let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) + .expect("client"); + + // Force path where remaining <= threshold but reset_at == now + let mut s = client.rate_limit.lock().await; + s.remaining = THRESHOLD as u32; + s.reset_at = Instant::now(); // so wait = 0 + drop(s); + + // Should return basically immediately + let elapsed = measure_sleep(&client).await; + assert!( + elapsed < Duration::from_millis(2), + "Guard unexpectedly slept: {:?}", + elapsed + ); } \ No newline at end of file From e507c2479a9d8d20c92707fc1fff4c44fd9f0bc7 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 11:54:58 +0530 Subject: [PATCH 18/23] fix: add threshold as an input parameter --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index db0cd03..2c96fae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,7 @@ pub async fn run() -> Result<(), Box> { let github_client = Arc::new(github::DefaultGithubClient::new( &config.github_token, &config.github_graphql_url, + config.rate_limit_threshold, )?); let messaging_service = Arc::new(TelegramMessagingService::new(bot.clone())); From fd4fd1653acfe0707ad5133c0c83ab8fdc342e51 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 12:04:17 +0530 Subject: [PATCH 19/23] fix: better error handling message for failing to parse header --- src/poller/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/poller/mod.rs b/src/poller/mod.rs index 56c9d82..a8ff316 100644 --- a/src/poller/mod.rs +++ b/src/poller/mod.rs @@ -199,8 +199,14 @@ impl GithubPoller { ); return Err(PollerError::Github(github_error)); } - GithubError::HeaderError(_) => { - tracing::error!("Header error while polling issues for repository"); + GithubError::HeaderError(msg) => { + tracing::warn!( + "Could not parse rate limit headers for repo {} (chat {}): {}. \ + Skipping this repo for this cycle.", + repo.name_with_owner, + chat_id, + msg + ); } }, unexpected_error => { From 92a2f916c9f5a816ecac6ee130b6af1b35e7669d Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 12:29:13 +0530 Subject: [PATCH 20/23] chore: formating --- src/github/tests.rs | 48 ++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/src/github/tests.rs b/src/github/tests.rs index 241be72..fede79d 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -60,8 +60,9 @@ async fn test_update_rate_limit_from_headers() { async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { // -------- Arrange -------- let threshold = 5; - let client = DefaultGithubClient::new("fake_token", "https://example.com/graphql", threshold as u64) - .expect("client"); + let client = + DefaultGithubClient::new("fake_token", "https://example.com/graphql", threshold as u64) + .expect("client"); const WAIT_MS: u64 = 40; { @@ -80,13 +81,12 @@ async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { let elapsed = start.elapsed(); // -------- Assert -------- - assert!( - elapsed >= expected_min, - "Guard returned too fast: {:?} < {:?}", elapsed, expected_min - ); + assert!(elapsed >= expected_min, "Guard returned too fast: {:?} < {:?}", elapsed, expected_min); assert!( elapsed <= expected_max + fudge, - "Guard slept too long: {:?} > {:?}", elapsed, expected_max + fudge + "Guard slept too long: {:?} > {:?}", + elapsed, + expected_max + fudge ); } @@ -111,8 +111,8 @@ async fn jitter_is_within_bounds() { const WAIT_MS: u64 = 50; const FUDGE_MS: u64 = 8; - let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) - .expect("client"); + let client = + DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD).expect("client"); // Force a sleep path prime_state(&client, THRESHOLD as u32, WAIT_MS).await; @@ -123,12 +123,7 @@ async fn jitter_is_within_bounds() { let elapsed = measure_sleep(&client).await; - assert!( - elapsed >= expected_min, - "Returned too fast: {:?} < {:?}", - elapsed, - expected_min - ); + assert!(elapsed >= expected_min, "Returned too fast: {:?} < {:?}", elapsed, expected_min); assert!( elapsed <= expected_max + fudge, "Slept too long: {:?} > {:?}", @@ -146,8 +141,8 @@ async fn jitter_varies_across_runs() { const RUNS: usize = 20; const FUDGE_MS: u64 = 8; - let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) - .expect("client"); + let client = + DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD).expect("client"); let mut samples = Vec::with_capacity(RUNS); @@ -164,12 +159,7 @@ async fn jitter_varies_across_runs() { // Same bounds check as safety for (i, dur) in samples.iter().enumerate() { - assert!( - *dur >= base, - "Run {i}: {:?} < base {:?}", - dur, - base - ); + assert!(*dur >= base, "Run {i}: {:?} < base {:?}", dur, base); assert!( *dur <= base + jitter_span + Duration::from_millis(FUDGE_MS), "Run {i}: {:?} > upper bound {:?}", @@ -191,8 +181,8 @@ async fn jitter_varies_across_runs() { #[tokio::test(flavor = "multi_thread")] async fn no_jitter_when_wait_is_zero() { const THRESHOLD: u64 = 1; - let client = DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD) - .expect("client"); + let client = + DefaultGithubClient::new("fake", "https://example/graphql", THRESHOLD).expect("client"); // Force path where remaining <= threshold but reset_at == now let mut s = client.rate_limit.lock().await; @@ -202,9 +192,5 @@ async fn no_jitter_when_wait_is_zero() { // Should return basically immediately let elapsed = measure_sleep(&client).await; - assert!( - elapsed < Duration::from_millis(2), - "Guard unexpectedly slept: {:?}", - elapsed - ); -} \ No newline at end of file + assert!(elapsed < Duration::from_millis(2), "Guard unexpectedly slept: {:?}", elapsed); +} From e6876cff2b6364e970cf5425f09005870f06e51e Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 12:31:42 +0530 Subject: [PATCH 21/23] chore: add docs for functions --- src/github/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/github/mod.rs b/src/github/mod.rs index e9a4cec..76add74 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -52,6 +52,8 @@ pub enum GithubError { /// An error indicating that the request was not authorized. #[error("GitHub authentication failed")] Unauthorized, + + /// An error indicating that a required header could not be parsed. #[error("Failed to parse header: {0}")] HeaderError(String), } @@ -314,6 +316,7 @@ impl DefaultGithubClient { } } + /// Update the rate limit state from the response headers. async fn update_rate_limit_from_headers(&self, headers: &HeaderMap) -> Result<(), GithubError> { // Names are case-insensitive in HeaderMap let rem_val = headers.get("X-RateLimit-Remaining").ok_or_else(|| { From 9f97b3a3d15fb602d047aee48062d5afa24ba7df Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 23:07:33 +0530 Subject: [PATCH 22/23] fix: cargo fmt --- src/github/mod.rs | 23 ++++++++++++----------- src/github/tests.rs | 12 ++++++++---- src/messaging/utils.rs | 5 ++--- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 76add74..ebfd63a 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -2,18 +2,21 @@ #[cfg(test)] mod tests; -use rand::{Rng, rng}; -use std::{collections::HashSet, time::Duration}; +use std::{ + collections::HashSet, + sync::Arc, + time::{Duration, Instant}, +}; use async_trait::async_trait; use backoff::{Error as BackoffError, ExponentialBackoff, future::retry}; use graphql_client::{GraphQLQuery, Response}; use mockall::automock; +use rand::{Rng, rng}; use reqwest::{ Client, header::{AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT}, }; -use std::{sync::Arc, time::Instant}; use thiserror::Error; use tokio::sync::Mutex; @@ -182,7 +185,7 @@ impl DefaultGithubClient { { // closure that Backoff expects let operation = || async { - //0. Rate limit guard + // 0. Rate limit guard self.rate_limit_guard().await; // 1. Build the request @@ -232,9 +235,8 @@ impl DefaultGithubClient { )) } } - reqwest::StatusCode::NOT_FOUND => { - GithubError::GraphQLApiError(format!("HTTP Not Found ({status}): {text}")) - } + reqwest::StatusCode::NOT_FOUND => + GithubError::GraphQLApiError(format!("HTTP Not Found ({status}): {text}")), _ => GithubError::GraphQLApiError(format!("HTTP Error ({status}): {text}")), }; @@ -242,9 +244,7 @@ impl DefaultGithubClient { GithubError::RateLimited => BackoffError::transient(github_err), _ if status.is_server_error() || status == reqwest::StatusCode::TOO_MANY_REQUESTS => - { - BackoffError::transient(github_err) - } + BackoffError::transient(github_err), _ => BackoffError::permanent(github_err), }; return Err(be); @@ -288,7 +288,8 @@ impl DefaultGithubClient { retry(Self::backoff_config(), operation).await } - /// Rate limit guard that sleeps until the rate limit resets if we're close to the threshold. + /// Rate limit guard that sleeps until the rate limit resets if we're close + /// to the threshold. async fn rate_limit_guard(&self) { let (remaining, reset_at) = { let state = self.rate_limit.lock().await; diff --git a/src/github/tests.rs b/src/github/tests.rs index fede79d..ad33231 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -1,5 +1,6 @@ -use super::*; use std::collections::HashMap; + +use super::*; #[test] fn test_new_github_client() { let client = DefaultGithubClient::new("test_token", "https://api.github.com/graphql", 10); @@ -90,7 +91,8 @@ async fn rate_limit_guard_sleeps_when_below_threshold_and_before_reset() { ); } -/// Helper: set the shared rate-limit state so the guard will sleep `wait_ms` plus jitter. +/// Helper: set the shared rate-limit state so the guard will sleep `wait_ms` +/// plus jitter. async fn prime_state(client: &DefaultGithubClient, remaining: u32, wait_ms: u64) { let mut s = client.rate_limit.lock().await; s.remaining = remaining; @@ -104,7 +106,8 @@ async fn measure_sleep(client: &DefaultGithubClient) -> Duration { start.elapsed() } -/// 1) Always inside [wait, wait + 10%] (with a little fudge for scheduler noise) +/// 1) Always inside [wait, wait + 10%] (with a little fudge for scheduler +/// noise) #[tokio::test(flavor = "multi_thread")] async fn jitter_is_within_bounds() { const THRESHOLD: u64 = 5; @@ -168,7 +171,8 @@ async fn jitter_varies_across_runs() { ); } - // And now confirm we saw at least ~some spread (non‑deterministic, so we only require >1ms spread). + // And now confirm we saw at least ~some spread (non‑deterministic, so we only + ///require >1ms spread). assert!( max > min + Duration::from_millis(1), "Jitter didn't vary enough: min={:?}, max={:?}", diff --git a/src/messaging/utils.rs b/src/messaging/utils.rs index 11da912..d84658f 100644 --- a/src/messaging/utils.rs +++ b/src/messaging/utils.rs @@ -13,9 +13,8 @@ pub fn github_color_to_emoji(hex_color: &str) -> &str { "fef2c0" | "fbca04" | "e4e669" | "ffeb3b" | "f9e076" | "fadc73" => "🟡", // Greens - "0e8a16" | "006b75" | "5ab302" | "a2eeef" | "008672" | "c2e0c6" | "1aa34a" | "4caf50" => { - "🟢" - } + "0e8a16" | "006b75" | "5ab302" | "a2eeef" | "008672" | "c2e0c6" | "1aa34a" | "4caf50" => + "🟢", // Blues / Teals "0052cc" | "c5def5" | "0075ca" | "1d76db" | "89d2fc" | "00bcd4" | "b3f4f4" => "🔵", From b1513c72ec22dce440e47f39617a424e270ac687 Mon Sep 17 00:00:00 2001 From: riturajFi Date: Thu, 24 Jul 2025 23:16:44 +0530 Subject: [PATCH 23/23] chore: add docs to field --- src/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config.rs b/src/config.rs index 52a1067..b718c4a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -27,6 +27,7 @@ pub struct Config { pub max_labels_per_repo: usize, /// The maximum number of concurrent requests to make to the GitHub API. pub max_concurrency: usize, + /// The threshold before the bot should pause operations. pub rate_limit_threshold: u64, }