diff --git a/crates/x-media/src/site/bsky/interface.rs b/crates/x-media/src/site/bsky/interface.rs index 68d6af6..efafe9f 100644 --- a/crates/x-media/src/site/bsky/interface.rs +++ b/crates/x-media/src/site/bsky/interface.rs @@ -131,10 +131,10 @@ fn concat_list(files: &mut [(usize, std::path::PathBuf)]) -> String { /// the end of a 500-segment video meant downloading the entire thing twice /// more, so the second attempt belongs on the request that actually failed. async fn fetch_hls(url: &str, cap: u64) -> Result { - match crate::site::download_media_limited(url, cap).await { + match crate::site::download_media_limited(url, cap, crate::site::DOWNLOAD_TOTAL_TIMEOUT).await { Err(FetchError::Http(_) | FetchError::Transient(_)) => { tokio::time::sleep(std::time::Duration::from_secs(1)).await; - crate::site::download_media_limited(url, cap) + crate::site::download_media_limited(url, cap, crate::site::DOWNLOAD_TOTAL_TIMEOUT) .await .map_err(|e| e.to_string()) } diff --git a/crates/x-media/src/site/download.rs b/crates/x-media/src/site/download.rs index d141739..4f99788 100644 --- a/crates/x-media/src/site/download.rs +++ b/crates/x-media/src/site/download.rs @@ -16,15 +16,18 @@ use std::time::Duration; /// [`DOWNLOAD_TOTAL_TIMEOUT`]. const DOWNLOAD_IDLE_TIMEOUT: Duration = Duration::from_secs(30); -/// Absolute ceiling for one media download, on top of the idle window. A server -/// that drips a byte every 29 s keeps [`next_chunk`] satisfied indefinitely, and -/// on the bot's side each such download holds one of the process-wide upload-prep -/// slots (`send::upload`'s `PREP_SLOTS`) for as long as it lasts. Generous on -/// purpose: the legitimate cases are big — an ugoira frame zip runs to hundreds -/// of MB and an HLS remux pulls a whole video — and a slow link is not an error. -/// Checked between chunks, so a transfer that completes just over the budget is -/// kept rather than thrown away. -const DOWNLOAD_TOTAL_TIMEOUT: Duration = Duration::from_secs(600); +/// Absolute ceiling for one media download, on top of the idle window: a +/// server that drips a byte every 29 s keeps [`next_chunk`] satisfied +/// indefinitely, and a transfer that trickles forever holds whatever the +/// caller pinned to it — a fetch permit for an in-flight post, a prep slot +/// for the bot's upload fallback. Generous on purpose: the legitimate cases +/// are big — an ugoira frame zip runs to hundreds of MB and an HLS remux +/// pulls a whole video — so this is the budget for downloads *inside a +/// fetch*, while the slot-holding fallback passes its own shorter one (see +/// [`download_media_limited`]'s `total`). Checked between chunks, so a +/// transfer that completes just over the budget is kept rather than thrown +/// away. +pub(crate) const DOWNLOAD_TOTAL_TIMEOUT: Duration = Duration::from_secs(600); /// The error a download reports when it spends its whole budget without /// finishing. Retryable: the transfer may simply have been unlucky, and a retry @@ -89,8 +92,8 @@ pub(crate) static CLIENT: LazyLock = /// impossible to deliver at all (the size cap said 512 MiB, the clock said 30s). /// What a stalled connection cannot do is hang a worker: the head and every /// chunk are bounded by [`DOWNLOAD_IDLE_TIMEOUT`] (see [`next_chunk`]), and a -/// transfer that keeps trickling but never finishes is bounded by -/// [`DOWNLOAD_TOTAL_TIMEOUT`]. +/// transfer that keeps trickling but never finishes is bounded by the +/// caller's total budget (see [`download_media_limited`]). static MEDIA_CLIENT: LazyLock = LazyLock::new(|| build_client(None)); /// The error a download reports when it stops making progress. @@ -232,7 +235,16 @@ fn apply_media_headers(mut request: reqwest::RequestBuilder, url: &str) -> reqwe /// cannot fetch a media URL itself (hotlink protection), the bot downloads /// the file and uploads it via multipart. Site-appropriate headers come from /// each site's `media_headers` (pixiv image hosts need `Referer`). -pub async fn download_media_limited(url: &str, max_bytes: u64) -> Result { +/// +/// `total` is this caller's whole-transfer budget. The bot's upload fallback +/// holds a prep slot (and its memory reservation) while this runs, so it +/// passes a shorter one of its own; bsky's in-fetch segments take the +/// generous [`super::DOWNLOAD_TOTAL_TIMEOUT`]. +pub async fn download_media_limited( + url: &str, + max_bytes: u64, + total: Duration, +) -> Result { let response = send_download(media_request(url)?).await?; if let Some(len) = response.content_length() && len > max_bytes @@ -243,8 +255,8 @@ pub async fn download_media_limited(url: &str, max_bytes: u64) -> Result DOWNLOAD_TOTAL_TIMEOUT { - return Err(download_too_slow(DOWNLOAD_TOTAL_TIMEOUT)); + if started.elapsed() > total { + return Err(download_too_slow(total)); } buf.extend_from_slice(&chunk); if buf.len() as u64 > max_bytes { @@ -358,7 +370,10 @@ mod tests { #[ignore = "live network: requires outbound HTTPS to httpbin.org"] async fn live_redirect_into_the_hosts_network_is_refused() { let url = "https://httpbin.org/redirect-to?url=http://169.254.169.254/latest/meta-data/"; - match download_media_limited(url, u64::MAX).await.unwrap_err() { + match download_media_limited(url, u64::MAX, DOWNLOAD_TOTAL_TIMEOUT) + .await + .unwrap_err() + { // A policy refusal reaches the caller wrapped by reqwest. FetchError::Http(e) => assert!(e.is_redirect(), "got {e}"), FetchError::Blocked => {} @@ -375,13 +390,15 @@ mod tests { "http://169.254.169.254/latest/meta-data/", "http://127.0.0.1:9/secret", ] { - let err = download_media_limited(url, u64::MAX).await.unwrap_err(); + let err = download_media_limited(url, u64::MAX, DOWNLOAD_TOTAL_TIMEOUT) + .await + .unwrap_err(); assert!(matches!(err, FetchError::Blocked), "{url}: got {err:?}"); } // A malformed URL is refused the same way instead of becoming a // retryable transport error. assert!(matches!( - download_media_limited("not a url", u64::MAX) + download_media_limited("not a url", u64::MAX, DOWNLOAD_TOTAL_TIMEOUT) .await .unwrap_err(), FetchError::Blocked @@ -409,7 +426,9 @@ mod tests { other => panic!("expected illustration media, got {other:?}"), }; assert!(url.contains("i.pximg.net")); - let bytes = download_media_limited(&url, u64::MAX).await.unwrap(); + let bytes = download_media_limited(&url, u64::MAX, DOWNLOAD_TOTAL_TIMEOUT) + .await + .unwrap(); assert!(!bytes.is_empty()); } } diff --git a/crates/x-media/src/site/mod.rs b/crates/x-media/src/site/mod.rs index e586d68..07d2aaf 100644 --- a/crates/x-media/src/site/mod.rs +++ b/crates/x-media/src/site/mod.rs @@ -22,7 +22,7 @@ pub mod twitter; pub use pixiv::PixivError; -pub(crate) use download::CLIENT; +pub(crate) use download::{CLIENT, DOWNLOAD_TOTAL_TIMEOUT}; pub use download::{download_media_limited, download_media_to_file}; /// The result of fetching a post: canonical URL, HTML caption, the post's diff --git a/crates/xmedia-bot/src/send/upload.rs b/crates/xmedia-bot/src/send/upload.rs index 62dd1ea..041e7a5 100644 --- a/crates/xmedia-bot/src/send/upload.rs +++ b/crates/xmedia-bot/src/send/upload.rs @@ -34,6 +34,17 @@ static PREP_SLOTS: LazyLock = /// post was lost. pub(super) const MAX_MEDIA_UPLOAD_BYTES: u64 = 50 * 1024 * 1024; +/// Whole-transfer budget for one fallback download. The prep slot (and the +/// non-photo memory reservation) is held while this runs, and the idle window +/// alone lets a server drip one byte every 29 s forever — so this path caps +/// its own transfers well below the in-fetch default: 50 MiB in 300 s needs +/// about 1.4 Mbit/s, and a much slower link is better served by the retry +/// path toward the item's smaller fallback URL than by pinning a slot for +/// ten minutes. +/// ponytail: if slow-link reports show up, move the download out of the prep +/// slot (slot = decode/upload only) instead of raising this again. +const FALLBACK_DOWNLOAD_TOTAL: std::time::Duration = std::time::Duration::from_secs(300); + /// Infers a file extension from magic bytes so Telegram detects the mime type /// on multipart uploads. pub(super) fn sniff_ext(bytes: &[u8]) -> &'static str { @@ -111,7 +122,13 @@ async fn download_to_temp( } else { Some(photo::reserve_memory(MAX_MEDIA_UPLOAD_BYTES).await) }; - let bytes = match x_media::site::download_media_limited(media_url, limit).await { + let bytes = match x_media::site::download_media_limited( + media_url, + limit, + FALLBACK_DOWNLOAD_TOTAL, + ) + .await + { Ok(bytes) => bytes, Err(e) => return Err(classify_download_error(e)), };