From fe11ceedfa716bdd1f93fd1886318d4cf798e311 Mon Sep 17 00:00:00 2001 From: Christopher Tee Date: Tue, 24 Jun 2025 15:11:41 -0400 Subject: [PATCH] Skip GitHub fast path when rate-limited (#13033) --- crates/uv-git/src/git.rs | 17 ++++++- crates/uv-git/src/lib.rs | 1 + crates/uv-git/src/rate_limit.rs | 37 ++++++++++++++ crates/uv-git/src/resolver.rs | 24 +++++++-- crates/uv-static/src/env_vars.rs | 4 ++ crates/uv/tests/it/edit.rs | 82 +++++++++++++++++++++++++++++++ crates/uv/tests/it/pip_install.rs | 58 ++++++++++++++++++++++ 7 files changed, 219 insertions(+), 4 deletions(-) create mode 100644 crates/uv-git/src/rate_limit.rs diff --git a/crates/uv-git/src/git.rs b/crates/uv-git/src/git.rs index 298c205ba..4ee4c2670 100644 --- a/crates/uv-git/src/git.rs +++ b/crates/uv-git/src/git.rs @@ -20,6 +20,8 @@ use uv_redacted::DisplaySafeUrl; use uv_static::EnvVars; use uv_version::version; +use crate::rate_limit::{GITHUB_RATE_LIMIT_STATUS, is_github_rate_limited}; + /// A file indicates that if present, `git reset` has been done and a repo /// checkout is ready to go. See [`GitCheckout::reset`] for why we need this. const CHECKOUT_READY_LOCK: &str = ".ok"; @@ -787,7 +789,15 @@ fn github_fast_path( } }; - let url = format!("https://api.github.com/repos/{owner}/{repo}/commits/{github_branch_name}"); + // Check if we're rate-limited by GitHub before determining the FastPathRev + if GITHUB_RATE_LIMIT_STATUS.is_active() { + debug!("Skipping GitHub fast path attempt for: {url} (rate-limited)"); + return Ok(FastPathRev::Indeterminate); + } + + let base_url = std::env::var(EnvVars::UV_GITHUB_FAST_PATH_URL) + .unwrap_or("https://api.github.com/repos".to_owned()); + let url = format!("{base_url}/{owner}/{repo}/commits/{github_branch_name}"); let runtime = tokio::runtime::Builder::new_current_thread() .enable_all() @@ -807,6 +817,11 @@ fn github_fast_path( let response = request.send().await?; + if is_github_rate_limited(&response) { + // Mark that we are being rate-limited by GitHub + GITHUB_RATE_LIMIT_STATUS.activate(); + } + // GitHub returns a 404 if the repository does not exist, and a 422 if it exists but GitHub // is unable to resolve the requested revision. response.error_for_status_ref()?; diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index ef23e58c2..716eb7538 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -7,5 +7,6 @@ pub use crate::source::{Fetch, GitSource, Reporter}; mod credentials; mod git; +mod rate_limit; mod resolver; mod source; diff --git a/crates/uv-git/src/rate_limit.rs b/crates/uv-git/src/rate_limit.rs new file mode 100644 index 000000000..4d277e652 --- /dev/null +++ b/crates/uv-git/src/rate_limit.rs @@ -0,0 +1,37 @@ +use reqwest::{Response, StatusCode}; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// A global state on whether we are being rate-limited by GitHub's REST API. +/// If we are, avoid "fast-path" attempts. +pub(crate) static GITHUB_RATE_LIMIT_STATUS: GitHubRateLimitStatus = GitHubRateLimitStatus::new(); + +/// GitHub REST API rate limit status tracker. +/// +/// ## Assumptions +/// +/// The rate limit timeout duration is much longer than the runtime of a `uv` command. +/// And so we do not need to invalidate this state based on `x-ratelimit-reset`. +#[derive(Debug)] +pub(crate) struct GitHubRateLimitStatus(AtomicBool); + +impl GitHubRateLimitStatus { + const fn new() -> Self { + Self(AtomicBool::new(false)) + } + + pub(crate) fn activate(&self) { + self.0.store(true, Ordering::Relaxed); + } + + pub(crate) fn is_active(&self) -> bool { + self.0.load(Ordering::Relaxed) + } +} + +/// Determine if GitHub is applying rate-limiting based on the response +pub(crate) fn is_github_rate_limited(response: &Response) -> bool { + // HTTP 403 and 429 are possible status codes in the event of a primary or secondary rate limit. + // Source: https://docs.github.com/en/rest/using-the-rest-api/troubleshooting-the-rest-api?apiVersion=2022-11-28#rate-limit-errors + let status_code = response.status(); + status_code == StatusCode::FORBIDDEN || status_code == StatusCode::TOO_MANY_REQUESTS +} diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs index 0881910b2..3c12fc589 100644 --- a/crates/uv-git/src/resolver.rs +++ b/crates/uv-git/src/resolver.rs @@ -15,7 +15,10 @@ use uv_git_types::{GitHubRepository, GitOid, GitReference, GitUrl}; use uv_static::EnvVars; use uv_version::version; -use crate::{Fetch, GitSource, Reporter}; +use crate::{ + Fetch, GitSource, Reporter, + rate_limit::{GITHUB_RATE_LIMIT_STATUS, is_github_rate_limited}, +}; #[derive(Debug, thiserror::Error)] pub enum GitResolverError { @@ -85,10 +88,18 @@ impl GitResolver { return Ok(None); }; + // Check if we're rate-limited by GitHub, before determining the Git reference + if GITHUB_RATE_LIMIT_STATUS.is_active() { + debug!("Rate-limited by GitHub. Skipping GitHub fast path attempt for: {url}"); + return Ok(None); + } + // Determine the Git reference. let rev = url.reference().as_rev(); - let github_api_url = format!("https://api.github.com/repos/{owner}/{repo}/commits/{rev}"); + let github_api_base_url = std::env::var(EnvVars::UV_GITHUB_FAST_PATH_URL) + .unwrap_or("https://api.github.com/repos".to_owned()); + let github_api_url = format!("{github_api_base_url}/{owner}/{repo}/commits/{rev}"); debug!("Querying GitHub for commit at: {github_api_url}"); let mut request = client.get(&github_api_url); @@ -99,13 +110,20 @@ impl GitResolver { ); let response = request.send().await?; - if !response.status().is_success() { + let status = response.status(); + if !status.is_success() { // Returns a 404 if the repository does not exist, and a 422 if GitHub is unable to // resolve the requested rev. debug!( "GitHub API request failed for: {github_api_url} ({})", response.status() ); + + if is_github_rate_limited(&response) { + // Mark that we are being rate-limited by GitHub + GITHUB_RATE_LIMIT_STATUS.activate(); + } + return Ok(None); } diff --git a/crates/uv-static/src/env_vars.rs b/crates/uv-static/src/env_vars.rs index 8193b6aec..4a44579d7 100644 --- a/crates/uv-static/src/env_vars.rs +++ b/crates/uv-static/src/env_vars.rs @@ -667,6 +667,10 @@ impl EnvVars { #[attr_hidden] pub const UV_TEST_INDEX_URL: &'static str = "UV_TEST_INDEX_URL"; + /// Used to set the GitHub fast-path url for tests. + #[attr_hidden] + pub const UV_GITHUB_FAST_PATH_URL: &'static str = "UV_GITHUB_FAST_PATH_URL"; + /// Hide progress messages with non-deterministic order in tests. #[attr_hidden] pub const UV_TEST_NO_CLI_PROGRESS: &'static str = "UV_TEST_NO_CLI_PROGRESS"; diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index d117b1e8c..6bdaec17b 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -494,6 +494,88 @@ fn add_git_private_raw() -> Result<()> { Ok(()) } +#[tokio::test] +#[cfg(feature = "git")] +async fn add_git_private_rate_limited_by_github_rest_api_403_response() -> Result<()> { + let context = TestContext::new("3.12"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(403)) + .expect(1) + .mount(&server) + .await; + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [] + "#})?; + + uv_snapshot!(context.filters(), context + .add() + .arg(format!("uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage")) + .env("UV_GITHUB_FAST_PATH_URL", server.uri()), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + "); + + Ok(()) +} + +#[tokio::test] +#[cfg(feature = "git")] +async fn add_git_private_rate_limited_by_github_rest_api_429_response() -> Result<()> { + use uv_client::DEFAULT_RETRIES; + + let context = TestContext::new("3.12"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(429)) + .expect(1 + u64::from(DEFAULT_RETRIES)) // Middleware retries on 429 by default + .mount(&server) + .await; + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [] + "#})?; + + uv_snapshot!(context.filters(), context + .add() + .arg(format!("uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage")) + .env("UV_GITHUB_FAST_PATH_URL", server.uri()), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + "); + + Ok(()) +} + #[test] #[cfg(feature = "git")] fn add_git_error() -> Result<()> { diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index e0876b23c..604b8db15 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -2066,6 +2066,64 @@ fn install_git_public_https_missing_branch_or_tag() { "###); } +#[tokio::test] +#[cfg(feature = "git")] +async fn install_git_public_rate_limited_by_github_rest_api_403_response() { + let context = TestContext::new("3.12"); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(403)) + .expect(1) + .mount(&server) + .await; + + uv_snapshot!(context.filters(), context + .pip_install() + .arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage") + .env("UV_GITHUB_FAST_PATH_URL", server.uri()), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389) + "); +} + +#[tokio::test] +#[cfg(feature = "git")] +async fn install_git_public_rate_limited_by_github_rest_api_429_response() { + use uv_client::DEFAULT_RETRIES; + + let context = TestContext::new("3.12"); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(429)) + .expect(1 + u64::from(DEFAULT_RETRIES)) // Middleware retries on 429 by default + .mount(&server) + .await; + + uv_snapshot!(context.filters(), context + .pip_install() + .arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage") + .env("UV_GITHUB_FAST_PATH_URL", server.uri()), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389) + "); +} + /// Install a package from a public GitHub repository at a ref that does not exist #[test] #[cfg(feature = "git")]