mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-05 17:08:26 +00:00
fix(retry): honor attempt caps per download URL
This commit is contained in:
+38
-24
@@ -429,9 +429,12 @@ async fn download_file(
|
||||
// The legacy `max_tries` payload field is still honored as a cap on
|
||||
// attempts, but transient backoff is additionally bounded by
|
||||
// `retry::MAX_RETRIES` so a single URL cannot spin forever.
|
||||
let mut strike = 0_usize;
|
||||
let max_attempts = payload.max_tries.max(1) as usize;
|
||||
'url: for url in &payload.urls {
|
||||
let mut strike = 0_usize;
|
||||
let mut attempts = 0_usize;
|
||||
loop {
|
||||
attempts += 1;
|
||||
match download_attempt(&events, &client, &payload, url, &mut control_rx).await {
|
||||
Ok(()) => return DownloadOutcome::Completed,
|
||||
Err(AttemptError::Controlled(DownloadControl::Pause)) => {
|
||||
@@ -446,34 +449,45 @@ async fn download_file(
|
||||
}
|
||||
Err(AttemptError::Failed(error)) => {
|
||||
last_error = error.clone();
|
||||
let transient = crate::retry::is_transient_network_error(&error);
|
||||
let strikes_left = strike < crate::retry::MAX_RETRIES;
|
||||
if !(transient && strikes_left) {
|
||||
// Permanent error (e.g. HTTP 404 / disk full) or the
|
||||
// 3-strike budget is exhausted — advance to the next URL.
|
||||
strike = 0;
|
||||
|
||||
if attempts >= max_attempts {
|
||||
continue 'url;
|
||||
}
|
||||
|
||||
// Transient: announce `Retrying`, back off, then retry.
|
||||
// The backoff sleep is itself cancelable so a user
|
||||
// pause/cancel during the wait is honored immediately.
|
||||
events.emit_retrying(payload.id, strike, error);
|
||||
let delay = crate::retry::backoff_for(strike);
|
||||
tokio::select! {
|
||||
_ = tokio::time::sleep(delay) => {}
|
||||
control = control_rx.recv() => {
|
||||
return match control.unwrap_or(DownloadControl::Cancel) {
|
||||
DownloadControl::Pause => DownloadOutcome::Paused,
|
||||
DownloadControl::Cancel => {
|
||||
let _ = fs::remove_file(&payload.output_path).await;
|
||||
DownloadOutcome::Cancelled
|
||||
}
|
||||
DownloadControl::Replace => DownloadOutcome::Cancelled,
|
||||
};
|
||||
let transient = crate::retry::is_transient_network_error(&error);
|
||||
let strikes_left = strike < crate::retry::MAX_RETRIES;
|
||||
|
||||
if transient && strikes_left {
|
||||
// Transient: announce `Retrying`, back off, then retry.
|
||||
// The backoff sleep is itself cancelable so a user
|
||||
// pause/cancel during the wait is honored immediately.
|
||||
events.emit_retrying(payload.id, strike, error);
|
||||
let delay = crate::retry::backoff_for(strike);
|
||||
tokio::select! {
|
||||
_ = tokio::time::sleep(delay) => {}
|
||||
control = control_rx.recv() => {
|
||||
return match control.unwrap_or(DownloadControl::Cancel) {
|
||||
DownloadControl::Pause => DownloadOutcome::Paused,
|
||||
DownloadControl::Cancel => {
|
||||
let _ = fs::remove_file(&payload.output_path).await;
|
||||
DownloadOutcome::Cancelled
|
||||
}
|
||||
DownloadControl::Replace => DownloadOutcome::Cancelled,
|
||||
};
|
||||
}
|
||||
}
|
||||
strike += 1;
|
||||
continue;
|
||||
}
|
||||
strike += 1;
|
||||
|
||||
if !transient && !crate::retry::is_permanent_network_error(&error) {
|
||||
// Legacy `max_tries` cap for ambiguous HTTP statuses (e.g.
|
||||
// 500) that are neither clearly transient nor permanent.
|
||||
continue;
|
||||
}
|
||||
|
||||
// Permanent error or transient strike budget exhausted.
|
||||
continue 'url;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+19
-7
@@ -72,12 +72,8 @@ pub fn backoff_for(strike: usize) -> Duration {
|
||||
/// 401/403/404/410/451, "not found", permission denied, out-of-disk. The
|
||||
/// permanent list is checked first so a composite message (e.g. an HTTP 404
|
||||
/// that also mentions "timeout" in a URL) still fails fast.
|
||||
pub fn is_transient_network_error(message: &str) -> bool {
|
||||
pub fn is_permanent_network_error(message: &str) -> bool {
|
||||
let m = message.to_ascii_lowercase();
|
||||
|
||||
// Permanent conditions win — never retry a 404/403/410/401/451, a missing
|
||||
// file, a permission error, or a full disk, even if the message also
|
||||
// contains a transient keyword (e.g. "timeout" inside a URL path).
|
||||
const PERMANENT: [&str; 9] = [
|
||||
"http 401",
|
||||
"http 403",
|
||||
@@ -89,11 +85,17 @@ pub fn is_transient_network_error(message: &str) -> bool {
|
||||
"permission denied",
|
||||
"no space left on device",
|
||||
];
|
||||
if PERMANENT.iter().any(|p| m.contains(p)) {
|
||||
PERMANENT.iter().any(|p| m.contains(p))
|
||||
}
|
||||
|
||||
pub fn is_transient_network_error(message: &str) -> bool {
|
||||
if is_permanent_network_error(message) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const TRANSIENT: [&str; 18] = [
|
||||
let m = message.to_ascii_lowercase();
|
||||
|
||||
const TRANSIENT: [&str; 20] = [
|
||||
// reqwest / hyper / OS socket-layer
|
||||
"timed out",
|
||||
"timeout",
|
||||
@@ -112,6 +114,8 @@ pub fn is_transient_network_error(message: &str) -> bool {
|
||||
// HTTP-level transient
|
||||
"http 408",
|
||||
"request timeout",
|
||||
"http 503",
|
||||
"503 service unavailable",
|
||||
// aria2c log phrasing
|
||||
"connection was closed",
|
||||
"timeout.",
|
||||
@@ -204,6 +208,14 @@ mod tests {
|
||||
assert!(is_transient_network_error("request timeout"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_http_503_as_transient() {
|
||||
assert!(is_transient_network_error("HTTP 503 Service Unavailable"));
|
||||
assert!(is_transient_network_error(
|
||||
"http://127.0.0.1/file returned HTTP 503 Service Unavailable"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_ytdlp_and_aria2_phrasing_as_transient() {
|
||||
assert!(is_transient_network_error(
|
||||
|
||||
Reference in New Issue
Block a user