shifathosn
/
craft-launcher
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Show GitHub's limit notice only while it actually holds details back (#53) 

Issue #44: the footer kept saying "GitHub's hourly limit was reached"
when it hadn't been, or long after it stopped mattering.

A 403 or 429 from the API counted as rate limited until
x-ratelimit-reset. GitHub sends that header with every answer, so a
secondary limit (quota left, Retry-After: 60) became up to an hour of
"hourly limit reached". The reset now only counts when
x-ratelimit-remaining is 0; otherwise Retry-After decides, or a minute
on a bare 429, as GitHub documents.

The notice also stayed up after the ArtCraft feed had answered every
app. A check now reports the limit only when some app had to come from
the release pages or the saved answers.

Co-authored-by: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Fabio Lussi 2026-10-11 08:24:30 +02:00 committed by GitHub
parent 28ef3ad924
commit 136c4ade3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 100 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
crates/native/src/backend.rs
View File

@ -295,7 +295,8 @@ impl NativeBackend {
let at = crate::now();
*shared.last_check.lock().unwrap_or_else(PoisonError::into_inner) = Some(at);
shared.checking.store(false, Ordering::SeqCst);
shared.emit(Event::CheckFinished { at, source: source_used, rate_limited_until: source.rate_limited_until() });
let rate_limited_until = limit_notice(source_used, source.rate_limited_until());
shared.emit(Event::CheckFinished { at, source: source_used, rate_limited_until });
});
}
@ -690,6 +691,13 @@ fn latest_for(
}
}
/// Until when the footer says GitHub's limit holds details back: only while it does, that is
/// when this check had to settle for the release pages or saved answers. When the feed (or the
/// API itself) answered everything, nothing is missing, however recently GitHub said no.
fn limit_notice(source_used: Source, limited_until: Option<i64>) -> Option<i64> {
limited_until.filter(|_| matches!(source_used, Source::GitHubPages | Source::Cache))
}
/// The repo to ask GitHub about, following the feed's repo renames.
fn feed_repo(feed: Option<&Feed>, app: &CraftApp) -> String {
feed.map_or_else(|| app.repo.to_string(), |f| f.repo_for(app))
@ -1005,6 +1013,18 @@ mod tests {
.unwrap()
}
#[test]
fn the_limit_notice_shows_only_while_details_are_missing() {
let until = Some(NOW + 31 * 60);
// GitHub said no earlier, but the feed has since answered every app: nothing to warn about.
assert_eq!(limit_notice(Source::Feed, until), None);
assert_eq!(limit_notice(Source::GitHubApi, until), None);
// Some apps came from the release pages or saved answers: say when the rest returns.
assert_eq!(limit_notice(Source::GitHubPages, until), until);
assert_eq!(limit_notice(Source::Cache, until), until);
assert_eq!(limit_notice(Source::Cache, None), None);
}
#[test]
fn the_first_poll_tries_once_and_falls_back_at_once() {
// Nothing has ever loaded and the feed didn't answer: GitHub, freely.

85
crates/native/src/http.rs
View File

@ -120,12 +120,11 @@ impl Http {
404 => Ok(Fetched::NotFound),
403 | 429 => {
let remaining = header("x-ratelimit-remaining");
let reset = header("x-ratelimit-reset").and_then(|r| r.parse::<i64>().ok());
let retry_after = header("retry-after").and_then(|r| r.parse::<i64>().ok()).map(|s| crate::now() + s);
if status == 429 || remaining.as_deref() == Some("0") || retry_after.is_some() {
Ok(Fetched::RateLimited { reset: reset.or(retry_after) })
} else {
Err(format!("GitHub refused the request (HTTP {status})"))
let reset = header("x-ratelimit-reset").and_then(|r| r.trim().parse::<i64>().ok());
let retry_after = header("retry-after").and_then(|r| r.trim().parse::<i64>().ok());
match refused_until(status, remaining.as_deref(), reset, retry_after, crate::now()) {
Some(reset) => Ok(Fetched::RateLimited { reset }),
None => Err(format!("GitHub refused the request (HTTP {status})")),
}
}
s => Err(format!("GitHub answered HTTP {s}")),
@ -496,6 +495,25 @@ pub fn sha256_file(path: &Path) -> Result<String, String> {
Ok(hex(&hasher.finalize()))
}
/// Whether a 403 or 429 from GitHub's API means "slow down", and until when (Unix seconds, when
/// it says). `None`: refused for some other reason.
///
/// GitHub sends `x-ratelimit-reset` with every answer: it's when the hourly quota refills, which
/// only matters once `x-ratelimit-remaining` is 0. A secondary limit says how long to wait in
/// `Retry-After` (seconds) and leaves the hourly quota alone; without either, GitHub's advice is
/// to wait a minute.
fn refused_until(status: u16, remaining: Option<&str>, reset: Option<i64>, retry_after: Option<i64>, now: i64) -> Option<Option<i64>> {
let retry = retry_after.map(|secs| now.saturating_add(secs.max(0)));
if remaining.map(str::trim) == Some("0") {
// `None` sorts first: whichever of the two is known, and the later if both are.
return Some(reset.max(retry));
}
if retry.is_some() {
return Some(retry);
}
(status == 429).then_some(Some(now.saturating_add(60)))
}
fn host(url: &str) -> &str {
url.split("://").nth(1).and_then(|r| r.split('/').next()).unwrap_or(url)
}
@ -805,6 +823,61 @@ pub(crate) mod tests {
assert_eq!(std::fs::metadata(&dest).unwrap().len(), asset.size);
}
/// Answers one request with `head` (status line and headers) and an empty body.
fn answer_once(head: String) -> String {
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let url = format!("http://{}/repos/storytold/photocraft/releases/latest", listener.local_addr().unwrap());
std::thread::spawn(move || {
let Ok((mut stream, _)) = listener.accept() else { return };
let mut reader = BufReader::new(stream.try_clone().unwrap());
loop {
let mut line = String::new();
if reader.read_line(&mut line).unwrap_or(0) == 0 || line == "\r\n" {
break;
}
}
let _ = write!(stream, "{head}Content-Length: 0\r\nConnection: close\r\n\r\n");
});
url
}
#[test]
fn a_secondary_limit_waits_for_retry_after_not_the_hourly_reset() {
// GitHub's secondary limit: quota left, an hourly reset 50 minutes away, and "try again
// in a minute". That's a minute, not 50 minutes of "the hourly limit was reached".
let now = crate::now();
let url = answer_once(format!(
"HTTP/1.1 403 Forbidden\r\nx-ratelimit-remaining: 41\r\nx-ratelimit-reset: {}\r\nretry-after: 60\r\n",
now + 50 * 60
));
let Fetched::RateLimited { reset: Some(until) } = Http::new().unwrap().get_api(&url, None).unwrap() else { panic!("not rate limited") };
assert!((now + 60..now + 120).contains(&until), "limited for {} s", until - now);
}
#[test]
fn an_empty_hourly_quota_waits_for_the_reset() {
let now = crate::now();
let reset = now + 31 * 60;
let url = answer_once(format!("HTTP/1.1 403 Forbidden\r\nx-ratelimit-remaining: 0\r\nx-ratelimit-reset: {reset}\r\n"));
assert_eq!(Http::new().unwrap().get_api(&url, None).unwrap(), Fetched::RateLimited { reset: Some(reset) });
}
#[test]
fn github_refusals_are_read_the_way_github_documents_them() {
const NOW: i64 = 1_791_500_000;
// Out of quota: until the reset, or Retry-After if that's later.
assert_eq!(refused_until(403, Some("0"), Some(NOW + 1800), None, NOW), Some(Some(NOW + 1800)));
assert_eq!(refused_until(403, Some("0"), Some(NOW + 30), Some(90), NOW), Some(Some(NOW + 90)));
assert_eq!(refused_until(403, Some("0"), None, None, NOW), Some(None));
// A secondary limit: Retry-After, never the hourly reset.
assert_eq!(refused_until(429, Some("12"), Some(NOW + 1800), Some(30), NOW), Some(Some(NOW + 30)));
// A 429 that doesn't say: a minute.
assert_eq!(refused_until(429, Some("12"), Some(NOW + 1800), None, NOW), Some(Some(NOW + 60)));
// A plain 403 (blocked, private, …) isn't a rate limit.
assert_eq!(refused_until(403, Some("12"), Some(NOW + 1800), None, NOW), None);
assert_eq!(refused_until(403, None, None, None, NOW), None);
}
#[test]
fn partial_files_are_named_after_the_file_and_size() {
assert_eq!(part_path(Path::new("/d/photocraft.dmg"), 9), Path::new("/d/photocraft.dmg.9.part"));