Refactor: Move HasReleaseArtifacts failure variants into GhApiError

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
This commit is contained in:
Jiahao XU 2024-05-09 23:25:22 +10:00
parent 5417475542
commit 7bd8468c35
No known key found for this signature in database
GPG key ID: 76D1E687CA3C4928
8 changed files with 129 additions and 257 deletions

View file

@ -8,7 +8,7 @@ pub(super) use binstalk_downloader::{
remote::{Client, Url},
};
pub(super) use binstalk_git_repo_api::gh_api_client::GhApiClient;
use binstalk_git_repo_api::gh_api_client::{GhReleaseArtifact, HasReleaseArtifact};
use binstalk_git_repo_api::gh_api_client::{GhApiError, GhReleaseArtifact};
pub(super) use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta};
pub(super) use compact_str::CompactString;
pub(super) use tokio::task::JoinHandle;
@ -34,22 +34,24 @@ pub(super) async fn does_url_exist(
debug!("Using GitHub API to check for existence of artifact, which will also cache the API response");
// The future returned has the same size as a pointer
match gh_api_client.has_release_artifact(artifact).await? {
HasReleaseArtifact::Yes { .. } => return Ok(true),
HasReleaseArtifact::No | HasReleaseArtifact::NoSuchRelease => return Ok(false),
match gh_api_client.has_release_artifact(artifact).await {
Ok(Some(_)) => return Ok(true),
Ok(None) | Err(GhApiError::NotFound) => return Ok(false),
HasReleaseArtifact::RateLimit { retry_after } => {
Err(GhApiError::RateLimit { retry_after }) => {
WARN_RATE_LIMIT_ONCE.call_once(|| {
warn!("Your GitHub API token (if any) has reached its rate limit and cannot be used again until {retry_after:?}, so we will fallback to HEAD/GET on the url.");
warn!("If you did not supply a github token, consider doing so: GitHub limits unauthorized users to 60 requests per hour per origin IP address.");
});
}
HasReleaseArtifact::Unauthorized => {
Err(GhApiError::Unauthorized) => {
WARN_UNAUTHORIZED_ONCE.call_once(|| {
warn!("GitHub API somehow requires a token for the API access, so we will fallback to HEAD/GET on the url.");
warn!("Please consider supplying a token to cargo-binstall to speedup resolution.");
});
}
Err(err) => return Err(err.into()),
}
GH_API_CLIENT_FAILED.store(true, Relaxed);

View file

@ -130,40 +130,22 @@ impl GhApiClient {
}
}
enum FetchReleaseArtifactError {
Error(GhApiError),
RateLimit { retry_after: Instant },
Unauthorized,
}
impl GhApiClient {
async fn do_fetch_release_artifacts(
&self,
release: &GhRelease,
auth_token: Option<&str>,
) -> Result<Option<release_artifacts::Artifacts>, FetchReleaseArtifactError> {
use common::GhApiRet::*;
use FetchReleaseArtifactError as Error;
) -> Result<Option<release_artifacts::Artifacts>, GhApiError> {
match release_artifacts::fetch_release_artifacts(&self.0.client, release, auth_token).await
{
Ok(NotFound) => Ok(None),
Ok(Success(artifacts)) => Ok(Some(artifacts)),
Ok(ReachedRateLimit { retry_after }) => {
let retry_after = retry_after.unwrap_or(DEFAULT_RETRY_DURATION);
let now = Instant::now();
let retry_after = now
.checked_add(retry_after)
.unwrap_or_else(|| now + DEFAULT_RETRY_DURATION);
Err(Error::RateLimit { retry_after })
}
Ok(Unauthorized) => Err(Error::Unauthorized),
Err(err) => Err(Error::Error(err)),
Ok(artifacts) => Ok(Some(artifacts)),
Err(GhApiError::NotFound) => Ok(None),
Err(err) => Err(err),
}
}
/// Return `Ok(Some(api_artifact_url))` if exists.
///
/// The returned future is guaranteed to be pointer size.
pub async fn has_release_artifact(
&self,
@ -171,9 +153,7 @@ impl GhApiClient {
release,
artifact_name,
}: GhReleaseArtifact,
) -> Result<HasReleaseArtifact, GhApiError> {
use FetchReleaseArtifactError as Error;
) -> Result<Option<CompactString>, GhApiError> {
let once_cell = self.0.release_artifacts.get(release.clone());
let res = once_cell
.get_or_try_init(|| {
@ -183,7 +163,9 @@ impl GhApiClient {
if let Some(retry_after) = *guard {
if retry_after.elapsed().is_zero() {
return Err(Error::RateLimit { retry_after });
return Err(GhApiError::RateLimit {
retry_after: Some(retry_after - Instant::now()),
});
} else {
// Instant retry_after is already reached.
*guard = None;
@ -196,7 +178,7 @@ impl GhApiClient {
.do_fetch_release_artifacts(&release, self.0.auth_token.as_deref())
.await
{
Err(Error::Unauthorized) => {
Err(GhApiError::Unauthorized) => {
self.0.is_auth_token_valid.store(false, Relaxed);
}
res => return res,
@ -209,52 +191,19 @@ impl GhApiClient {
.await;
match res {
Ok(Some(artifacts)) => Ok(artifacts
.get_artifact_url(&artifact_name)
.map(|url| HasReleaseArtifact::Yes { url })
.unwrap_or(HasReleaseArtifact::No)),
Ok(None) => Ok(HasReleaseArtifact::NoSuchRelease),
Err(Error::Unauthorized) => Ok(HasReleaseArtifact::Unauthorized),
Err(Error::RateLimit { retry_after }) => {
*self.0.retry_after.lock().unwrap() = Some(retry_after);
Ok(Some(artifacts)) => Ok(artifacts.get_artifact_url(&artifact_name)),
Ok(None) => Ok(None),
Err(GhApiError::RateLimit { retry_after }) => {
*self.0.retry_after.lock().unwrap() =
Some(Instant::now() + retry_after.unwrap_or(DEFAULT_RETRY_DURATION));
Ok(HasReleaseArtifact::RateLimit { retry_after })
Err(GhApiError::RateLimit { retry_after })
}
Err(Error::Error(err)) => Err(err),
Err(err) => Err(err),
}
}
}
#[derive(Eq, PartialEq, Clone, Debug)]
pub enum HasReleaseArtifact {
Yes {
/// get url for downloading the artifact using GitHub API (for private repository).
url: CompactString,
},
No,
NoSuchRelease,
/// GitHub returns 401 requiring a token.
/// In this case, it makes sense to fallback to HEAD/GET.
Unauthorized,
/// GitHub rate limit is applied per hour, so in case of reaching the rate
/// limit, [`GhApiClient`] will return this variant and let the user decide
/// what to do.
///
/// Usually it is more sensible to fallback to directly HEAD/GET the
/// artifact url than waiting until `retry_after`.
///
/// If you encounter this frequently, then you should consider getting an
/// authentication token (can be personal access or oath access token),
/// which should give you 5000 requests per hour per user.
///
/// Rate limit for unauthorized user is 60 requests per hour per originating
/// IP address, so it is very easy to be rate limited.
RateLimit {
retry_after: Instant,
},
}
#[cfg(test)]
mod test {
use super::*;
@ -373,39 +322,31 @@ mod test {
eprintln!("In client {client:?}");
for artifact_name in artifacts {
let ret = client
let res = client
.has_release_artifact(GhReleaseArtifact {
release: release.clone(),
artifact_name: artifact_name.to_compact_string(),
})
.await
.unwrap();
.await;
assert!(
matches!(
ret,
HasReleaseArtifact::Yes { .. } | HasReleaseArtifact::RateLimit { .. }
),
matches!(res, Ok(Some(_)) | Err(GhApiError::RateLimit { .. })),
"for '{artifact_name}': answer is {:#?}",
ret
res
);
}
let ret = client
let res = client
.has_release_artifact(GhReleaseArtifact {
release: release.clone(),
artifact_name: "123z".to_compact_string(),
})
.await
.unwrap();
.await;
assert!(
matches!(
ret,
HasReleaseArtifact::No | HasReleaseArtifact::RateLimit { .. }
),
"ret = {:#?}",
ret
matches!(res, Ok(None) | Err(GhApiError::RateLimit { .. })),
"res = {:#?}",
res
);
}
}
@ -430,21 +371,18 @@ mod test {
tag: "v0.18.2".to_compact_string(),
};
let ret = client
let err = client
.has_release_artifact(GhReleaseArtifact {
release,
artifact_name: "1234".to_compact_string(),
})
.await
.unwrap();
.unwrap_err();
assert!(
matches!(
ret,
HasReleaseArtifact::NoSuchRelease | HasReleaseArtifact::RateLimit { .. }
),
"ret = {:#?}",
ret
matches!(err, GhApiError::NotFound | GhApiError::RateLimit { .. }),
"err = {:#?}",
err
);
}
}

View file

@ -38,14 +38,7 @@ pub(super) fn percent_decode_http_url_path(input: &str) -> CompactString {
}
}
pub(super) enum GhApiRet<T> {
ReachedRateLimit { retry_after: Option<Duration> },
NotFound,
Success(T),
Unauthorized,
}
pub(super) fn check_for_status<T>(status: StatusCode, headers: &HeaderMap) -> Option<GhApiRet<T>> {
fn check_http_status_and_header(status: StatusCode, headers: &HeaderMap) -> Result<(), GhApiError> {
match status {
remote::StatusCode::FORBIDDEN
if headers
@ -53,7 +46,7 @@ pub(super) fn check_for_status<T>(status: StatusCode, headers: &HeaderMap) -> Op
.map(|val| val == "0")
.unwrap_or(false) =>
{
Some(GhApiRet::ReachedRateLimit {
Err(GhApiError::RateLimit {
retry_after: headers.get("x-ratelimit-reset").and_then(|value| {
let secs = value.to_str().ok()?.parse().ok()?;
Some(Duration::from_secs(secs))
@ -61,13 +54,37 @@ pub(super) fn check_for_status<T>(status: StatusCode, headers: &HeaderMap) -> Op
})
}
remote::StatusCode::UNAUTHORIZED => Some(GhApiRet::Unauthorized),
remote::StatusCode::NOT_FOUND => Some(GhApiRet::NotFound),
remote::StatusCode::UNAUTHORIZED => Err(GhApiError::Unauthorized),
remote::StatusCode::NOT_FOUND => Err(GhApiError::NotFound),
_ => None,
_ => Ok(()),
}
}
pub(super) async fn issue_restful_api<T>(
client: &remote::Client,
path: String,
auth_token: Option<&str>,
) -> Result<T, GhApiError>
where
T: DeserializeOwned,
{
let mut request_builder = client
.get(Url::parse(&format!("https://api.github.com/{path}"))?)
.header("Accept", "application/vnd.github+json")
.header("X-GitHub-Api-Version", "2022-11-28");
if let Some(auth_token) = auth_token {
request_builder = request_builder.bearer_auth(&auth_token);
}
let response = request_builder.send(false).await?;
check_http_status_and_header(response.status(), response.headers())?;
Ok(response.json().await?)
}
#[derive(Deserialize)]
enum GraphQLResponse<T> {
#[serde(rename = "data")]
@ -90,16 +107,11 @@ fn get_graphql_endpoint() -> &'static Url {
})
}
pub(super) enum GraphQLResult<T, U> {
Data(T),
Else(GhApiRet<U>),
}
pub(super) async fn issue_graphql_query<T, U>(
pub(super) async fn issue_graphql_query<T>(
client: &remote::Client,
query: String,
auth_token: &str,
) -> Result<GraphQLResult<T, U>, GhApiError>
) -> Result<T, GhApiError>
where
T: DeserializeOwned,
{
@ -115,19 +127,14 @@ where
.bearer_auth(&auth_token);
let response = request_builder.send(false).await?;
if let Some(ret) = check_for_status(response.status(), response.headers()) {
return Ok(GraphQLResult::Else(ret));
}
check_http_status_and_header(response.status(), response.headers())?;
let response: GraphQLResponse<T> = response.json().await?;
match response {
GraphQLResponse::Data(data) => Ok(GraphQLResult::Data(data)),
GraphQLResponse::Data(data) => Ok(data),
GraphQLResponse::Errors(errors) if errors.is_rate_limited() => {
Ok(GraphQLResult::Else(GhApiRet::ReachedRateLimit {
retry_after: None,
}))
Err(GhApiError::RateLimit { retry_after: None })
}
GraphQLResponse::Errors(errors) => Err(errors.into()),
}

View file

@ -1,4 +1,4 @@
use std::{error, fmt, io};
use std::{error, fmt, io, time::Duration};
use binstalk_downloader::remote;
use compact_str::{CompactString, ToCompactString};
@ -31,16 +31,31 @@ pub enum GhApiError {
#[error("Remote failed to process GraphQL query: {0}")]
GraphQLErrors(#[from] GhGraphQLErrors),
#[error("Hit rate-limit, retry after {retry_after:?}")]
RateLimit { retry_after: Option<Duration> },
#[error("Corresponding resource is not found")]
NotFound,
#[error("Does not have permission to access the API")]
Unauthorized,
}
impl GhApiError {
/// Attach context to [`GhApiError`]
pub fn context(self, context: impl fmt::Display) -> Self {
use GhApiError::*;
if matches!(self, RateLimit { .. } | NotFound | Unauthorized) {
self
} else {
Self::Context(Box::new(GhApiContextError {
context: context.to_compact_string(),
err: self,
}))
}
}
}
#[derive(Debug, Deserialize)]

View file

@ -5,12 +5,12 @@ use std::{
hash::{Hash, Hasher},
};
use binstalk_downloader::remote::{self, header::HeaderMap, StatusCode, Url};
use binstalk_downloader::remote::{self};
use compact_str::CompactString;
use serde::Deserialize;
use super::{
common::{self, issue_graphql_query, percent_encode_http_url_path, GraphQLResult},
common::{issue_graphql_query, issue_restful_api, percent_encode_http_url_path},
GhApiError, GhRelease,
};
@ -65,38 +65,22 @@ impl Artifacts {
}
}
pub(super) type FetchReleaseRet = common::GhApiRet<Artifacts>;
fn check_for_status(status: StatusCode, headers: &HeaderMap) -> Option<FetchReleaseRet> {
common::check_for_status(status, headers)
}
async fn fetch_release_artifacts_restful_api(
client: &remote::Client,
GhRelease { owner, repo, tag }: &GhRelease,
auth_token: Option<&str>,
) -> Result<FetchReleaseRet, GhApiError> {
let mut request_builder = client
.get(Url::parse(&format!(
"https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}",
) -> Result<Artifacts, GhApiError> {
issue_restful_api(
client,
format!(
"repos/{owner}/{repo}/releases/tags/{tag}",
owner = percent_encode_http_url_path(owner),
repo = percent_encode_http_url_path(repo),
tag = percent_encode_http_url_path(tag),
))?)
.header("Accept", "application/vnd.github+json")
.header("X-GitHub-Api-Version", "2022-11-28");
if let Some(auth_token) = auth_token {
request_builder = request_builder.bearer_auth(&auth_token);
}
let response = request_builder.send(false).await?;
if let Some(ret) = check_for_status(response.status(), response.headers()) {
Ok(ret)
} else {
Ok(FetchReleaseRet::Success(response.json().await?))
}
),
auth_token,
)
.await
}
#[derive(Deserialize)]
@ -149,7 +133,7 @@ async fn fetch_release_artifacts_graphql_api(
client: &remote::Client,
GhRelease { owner, repo, tag }: &GhRelease,
auth_token: &str,
) -> Result<FetchReleaseRet, GhApiError> {
) -> Result<Artifacts, GhApiError> {
let mut artifacts = Artifacts::default();
let mut cond = FilterCondition::Init;
@ -171,10 +155,7 @@ query {{
}}"#
);
let data: GraphQLData = match issue_graphql_query(client, query, auth_token).await? {
GraphQLResult::Data(data) => data,
GraphQLResult::Else(ret) => return Ok(ret),
};
let data: GraphQLData = issue_graphql_query(client, query, auth_token).await?;
let assets = data
.repository
@ -191,10 +172,10 @@ query {{
} => {
cond = FilterCondition::After(end_cursor);
}
_ => break Ok(FetchReleaseRet::Success(artifacts)),
_ => break Ok(artifacts),
}
} else {
break Ok(FetchReleaseRet::NotFound);
break Err(GhApiError::NotFound);
}
}
}
@ -203,7 +184,7 @@ pub(super) async fn fetch_release_artifacts(
client: &remote::Client,
release: &GhRelease,
auth_token: Option<&str>,
) -> Result<FetchReleaseRet, GhApiError> {
) -> Result<Artifacts, GhApiError> {
if let Some(auth_token) = auth_token {
let res = fetch_release_artifacts_graphql_api(client, release, auth_token)
.await
@ -211,7 +192,7 @@ pub(super) async fn fetch_release_artifacts(
match res {
// Fallback to Restful API
Ok(FetchReleaseRet::Unauthorized) => (),
Err(GhApiError::Unauthorized) => (),
res => return res,
}
}

View file

@ -1,9 +1,8 @@
use binstalk_downloader::remote::{header::HeaderMap, StatusCode, Url};
use compact_str::CompactString;
use serde::Deserialize;
use super::{
common::{self, issue_graphql_query, percent_encode_http_url_path, GraphQLResult},
common::{issue_graphql_query, issue_restful_api, percent_encode_http_url_path},
remote, GhApiError, GhRepo,
};
@ -32,37 +31,21 @@ impl RepoInfo {
}
}
pub(super) type FetchRepoInfoRet = common::GhApiRet<RepoInfo>;
fn check_for_status(status: StatusCode, headers: &HeaderMap) -> Option<FetchRepoInfoRet> {
common::check_for_status(status, headers)
}
async fn fetch_repo_info_restful_api(
client: &remote::Client,
GhRepo { owner, repo }: &GhRepo,
auth_token: Option<&str>,
) -> Result<FetchRepoInfoRet, GhApiError> {
let mut request_builder = client
.get(Url::parse(&format!(
"https://api.github.com/repos/{owner}/{repo}",
) -> Result<RepoInfo, GhApiError> {
issue_restful_api(
client,
format!(
"repos/{owner}/{repo}",
owner = percent_encode_http_url_path(owner),
repo = percent_encode_http_url_path(repo),
))?)
.header("Accept", "application/vnd.github+json")
.header("X-GitHub-Api-Version", "2022-11-28");
if let Some(auth_token) = auth_token {
request_builder = request_builder.bearer_auth(&auth_token);
}
let response = request_builder.send(false).await?;
if let Some(ret) = check_for_status(response.status(), response.headers()) {
Ok(ret)
} else {
Ok(FetchRepoInfoRet::Success(response.json().await?))
}
),
auth_token,
)
.await
}
#[derive(Deserialize)]
@ -74,7 +57,7 @@ async fn fetch_repo_info_graphql_api(
client: &remote::Client,
GhRepo { owner, repo }: &GhRepo,
auth_token: &str,
) -> Result<FetchRepoInfoRet, GhApiError> {
) -> Result<RepoInfo, GhApiError> {
let query = format!(
r#"
query {{
@ -88,17 +71,14 @@ query {{
}}"#
);
match issue_graphql_query(client, query, auth_token).await? {
GraphQLResult::Data(repo_info) => Ok(common::GhApiRet::Success(repo_info)),
GraphQLResult::Else(ret) => Ok(ret),
}
issue_graphql_query(client, query, auth_token).await
}
pub(super) async fn fetch_repo_info(
client: &remote::Client,
repo: &GhRepo,
auth_token: Option<&str>,
) -> Result<FetchRepoInfoRet, GhApiError> {
) -> Result<RepoInfo, GhApiError> {
if let Some(auth_token) = auth_token {
let res = fetch_repo_info_graphql_api(client, repo, auth_token)
.await
@ -106,7 +86,7 @@ pub(super) async fn fetch_repo_info(
match res {
// Fallback to Restful API
Ok(FetchRepoInfoRet::Unauthorized) => (),
Err(GhApiError::Unauthorized) => (),
res => return res,
}
}

View file

@ -1,5 +1,8 @@
pub mod jobserver_client;
pub mod remote;
pub mod remote {
pub use binstalk_downloader::remote::*;
pub use url::ParseError as UrlParseError;
}
pub(crate) mod target_triple;
pub mod tasks;

View file

@ -1,54 +0,0 @@
pub use binstalk_downloader::remote::*;
pub use url::ParseError as UrlParseError;
use std::sync::{
atomic::{AtomicBool, Ordering::Relaxed},
Once,
};
use tracing::{debug, warn};
use super::gh_api_client::{GhApiClient, GhReleaseArtifact, HasReleaseArtifact};
use crate::errors::BinstallError;
/// This function returns a future where its size should be at most size of
/// 2 pointers.
pub async fn does_url_exist(
client: Client,
gh_api_client: GhApiClient,
url: &Url,
) -> Result<bool, BinstallError> {
static GH_API_CLIENT_FAILED: AtomicBool = AtomicBool::new(false);
static WARN_RATE_LIMIT_ONCE: Once = Once::new();
static WARN_UNAUTHORIZED_ONCE: Once = Once::new();
debug!("Checking for package at: '{url}'");
if !GH_API_CLIENT_FAILED.load(Relaxed) {
if let Some(artifact) = GhReleaseArtifact::try_extract_from_url(url) {
debug!("Using GitHub API to check for existence of artifact, which will also cache the API response");
// The future returned has the same size as a pointer
match gh_api_client.has_release_artifact(artifact).await? {
HasReleaseArtifact::Yes { .. } => return Ok(true),
HasReleaseArtifact::No | HasReleaseArtifact::NoSuchRelease => return Ok(false),
HasReleaseArtifact::RateLimit { retry_after } => {
WARN_RATE_LIMIT_ONCE.call_once(|| {
warn!("Your GitHub API token (if any) has reached its rate limit and cannot be used again until {retry_after:?}, so we will fallback to HEAD/GET on the url.");
warn!("If you did not supply a github token, consider doing so: GitHub limits unauthorized users to 60 requests per hour per origin IP address.");
});
}
HasReleaseArtifact::Unauthorized => {
WARN_UNAUTHORIZED_ONCE.call_once(|| {
warn!("GitHub API somehow requires a token for the API access, so we will fallback to HEAD/GET on the url.");
warn!("Please consider supplying a token to cargo-binstall to speedup resolution.");
});
}
}
GH_API_CLIENT_FAILED.store(true, Relaxed);
}
}
Ok(Box::pin(client.remote_gettable(url.clone())).await?)
}