diff --git a/AGENTS.md b/AGENTS.md index c93703d..1d8919d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,7 +66,7 @@ Docker: `docker build -t tgxmb .` then `docker run --rm -d --name tgxmb --env-fi ## Code Conventions & Common Patterns -- **Errors via `thiserror` derive** (no anyhow): the public, stringified errors — `FetchError` (`Http`/`Json`/`Pixiv`/`Site`/`NotFound`/`Blocked`/`Disabled`/`Sensitive`/`TooLarge`/`Transient`/`Io`) and `PixivError` — derive `thiserror::Error` with `#[from]` conversions; `Display`/`source()` come from the derive. The internal control-flow enums — `QueueError` (`Retryable { delay_seconds, payload }` / `Permanent`), `SendError` (Retryable/Permanent), `Classification`, `FallbackError` — carry no `Display` and are handled by direct variant matching. New errors should follow the same split: stringified/public errors derive `thiserror`, internal flow enums stay plain. +- **Errors via `thiserror` derive** (no anyhow): the public, stringified errors — `FetchError` (`Http`/`Json`/`Pixiv`/`Site`/`NotFound`/`Blocked`/`Disabled`/`Sensitive`/`TooLarge`/`MediaPrep`/`Transient`/`Io`) and `PixivError` — derive `thiserror::Error` with `#[from]` conversions; `Display`/`source()` come from the derive. The internal control-flow enums — `QueueError` (`Retryable { delay_seconds, payload }` / `Permanent`), `SendError` (Retryable/Permanent), `Classification`, `FallbackError` — carry no `Display` and are handled by direct variant matching. New errors should follow the same split: stringified/public errors derive `thiserror`, internal flow enums stay plain. - **Global state via `std::sync::LazyLock` statics**, not DI: `CONFIG`, `CHAT_STORE`, `TASK_QUEUE` in `handlers/statics.rs`; shared reqwest `CLIENT` in `x-media/src/site/mod.rs`. `Bot` is passed/cloned into handlers; queue workers share the process-wide `send::BOT` (`LazyLock`, force-initialized in `main` so a missing token fails at startup). - **Async**: tokio multi-thread runtime (`#[tokio::main]` default). All rusqlite I/O inside `tokio::task::spawn_blocking`. Long loops use `tokio::select!` with `tokio::sync::{watch, Notify}` stop/wake channels. No streams. - **Blocking sync primitives**: `parking_lot::Mutex` for hot caches, `tokio::sync::Mutex` for async-shared state (pixiv token cache), `AtomicBool` for feature gates. diff --git a/crates/x-media/src/site/bsky/interface.rs b/crates/x-media/src/site/bsky/interface.rs index 8d60943..6382bcc 100644 --- a/crates/x-media/src/site/bsky/interface.rs +++ b/crates/x-media/src/site/bsky/interface.rs @@ -56,8 +56,11 @@ pub async fn fetch_from_url(url: &str) -> Result { // `warn` is a level operators share. let key = cache_key(url).unwrap_or_else(|| "?".into()); // A failed remux is remembered: if it leaves the post with no media at - // all, returning `Ok` would read as "this post has no media" and skip the - // retry that a transient segment-download failure deserves. + // all, returning `Ok` would read as "this post has no media". It is + // reported as `FetchError::MediaPrep` rather than a transient failure — + // the download legs already got their own retry in place ([`fetch_hls`]), + // and the fetch loop's retry would only download every segment again to + // fail the same way. let mut remux_failure: Option = None; for item in fetched.media { let is_hls = matches!(&item, Media::Video { url, .. } @@ -93,7 +96,7 @@ pub async fn fetch_from_url(url: &str) -> Result { if media.is_empty() && let Some(reason) = remux_failure { - return Err(FetchError::Transient(format!( + return Err(FetchError::MediaPrep(format!( "bsky video remux failed: {reason}" ))); } @@ -120,6 +123,24 @@ pub fn media_headers(_url: &str) -> Option> { None } +/// One HLS fetch (a playlist or a segment) with an in-place retry for a +/// retryable class (transport, 429/5xx). These used to get their retry from the +/// outer fetch loop, which pays for it by replaying the whole post: master +/// playlist, variant playlist and every segment again. A segment failing near +/// 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 { + Err(FetchError::Http(_) | FetchError::Transient(_)) => { + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + crate::site::download_media_limited(url, cap) + .await + .map_err(|e| e.to_string()) + } + other => other.map_err(|e| e.to_string()), + } +} + /// Downloads an HLS playlist (master or media) and remuxes its segments to a /// single MP4 via ffmpeg. Returns the MP4 path plus the temp dir that must /// stay alive until the file is uploaded. `Ok(None)` when ffmpeg is missing. @@ -135,7 +156,7 @@ async fn resolve_bsky_video( crate::site::log_once_ffmpeg_missing(); return Ok(None); } - let master = crate::site::download_media_limited(playlist_url, 1_048_576) + let master = fetch_hls(playlist_url, 1_048_576) .await .map_err(|e| format!("bsky video master playlist: {e}"))?; let master = String::from_utf8_lossy(&master); @@ -170,7 +191,7 @@ async fn resolve_bsky_video( playlist_url.to_string() }; - let variant = crate::site::download_media_limited(&playlist_url, 1_048_576) + let variant = fetch_hls(&playlist_url, 1_048_576) .await .map_err(|e| format!("bsky video media playlist: {e}"))?; let variant = String::from_utf8_lossy(&variant); @@ -201,7 +222,7 @@ async fn resolve_bsky_video( let mut total: u64 = 0; let mut list = String::new(); for (i, seg) in segments.iter().enumerate() { - let bytes = crate::site::download_media_limited(seg, 20 * 1024 * 1024) + let bytes = fetch_hls(seg, 20 * 1024 * 1024) .await .map_err(|e| format!("bsky segment {i}: {e}"))?; total += bytes.len() as u64; @@ -422,6 +443,18 @@ mod tests { } } + /// A remux failure is a `MediaPrep`, which the fetch loop does not retry: + /// replaying the post means downloading every HLS segment again, when the + /// request that failed already got its second attempt in place + /// ([`fetch_hls`]). The classes below are the ones still retried there. + #[test] + fn media_prep_failure_is_not_retried() { + assert!(!is_retryable(&FetchError::MediaPrep( + "bsky video remux failed: segment 400: 503".into() + ))); + assert!(is_retryable(&FetchError::Transient("429".into()))); + } + #[test] fn from_json_images_with_missing_defaults() { let raw = thread_json(serde_json::json!({ diff --git a/crates/x-media/src/site/mod.rs b/crates/x-media/src/site/mod.rs index 39f688e..cb4ca8a 100644 --- a/crates/x-media/src/site/mod.rs +++ b/crates/x-media/src/site/mod.rs @@ -260,6 +260,13 @@ pub enum FetchError { /// A download exceeded the caller's size cap (see [`download_media_limited`]). #[error("media too large")] TooLarge, + /// The post was fetched, but its media could not be prepared locally — a + /// download or encode step that runs *after* the site's own response + /// (bsky's HLS remux, say). Deliberately not retryable: the retry would + /// replay the whole fetch, redoing the download work that just failed + /// instead of the request that failed. + #[error("media could not be prepared: {0}")] + MediaPrep(String), /// A transient server-side failure (429 / 5xx); [`fetch`] retries these. #[error("transient: {0}")] Transient(String), diff --git a/crates/xmedia-bot/src/handlers/urls.rs b/crates/xmedia-bot/src/handlers/urls.rs index b253514..201a9c0 100644 --- a/crates/xmedia-bot/src/handlers/urls.rs +++ b/crates/xmedia-bot/src/handlers/urls.rs @@ -459,6 +459,11 @@ fn fetch_error_message(err: &x_media::site::FetchError) -> String { FetchError::Transient(_) | FetchError::Http(_) => { "The source site is unavailable right now (tried 3 times). Try again later.".to_string() } + FetchError::MediaPrep(_) => concat!( + "Could not prepare this post's media (its download or encode failed). ", + "Try again later." + ) + .to_string(), // Parse/shape surprises, pixiv auth details, oversized media: nothing // actionable for the user beyond "this did not work". _ => "Failed to fetch media from this link.".to_string(),