mirror of
https://github.com/TheFunny/TelegramTwitterMediaBot.git
synced 2026-09-23 23:32:05 +00:00
perf: retry the bsky HLS request, not the whole fetch
A failed bsky video remux was reported as `FetchError::Transient`, so the fetch loop retried the *whole* adapter — master playlist, variant playlist and up to 500 segments again (256 MiB of cap each time), for a failure that happened near the end of the work the retry was about to redo. The message had to stay honest (returning `Ok` with no media reads as "this post has no media"), so it needed a class of its own. `FetchError::MediaPrep` is that class: post fetched, media could not be prepared locally, not retryable (the per-site `is_retryable` whitelist excludes it by construction), with its own user-facing text — the generic "failed to fetch" would have hidden that the download or encode was what broke. The retry itself is not lost, it moved: `fetch_hls` retries the request that actually failed, once, for the classes a retry can change (transport, 429/5xx). Checked the sibling sites before widening the change: pixiv already degrades to no media when the ugoira encode fails (`Ok(None)`), and its frame-zip download failure is the last step so a re-fetch re-does only that; twitter's auth leg replays two cheap metadata GETs and a GraphQL 5xx *is* worth retrying. Neither needed the new class. Verified with a throwaway proxy harness: a 503 answered twice-in-a-row path costs 2 requests and succeeds, a 404 costs exactly 1 and fails (no pointless retry). `site::bsky::interface::tests::media_prep_failure_is_not_retried` pins the classification. `cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D warnings` and `cargo test --workspace --locked` clean.
This commit is contained in:
@@ -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<Bot>`, 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.
|
||||
|
||||
@@ -56,8 +56,11 @@ pub async fn fetch_from_url(url: &str) -> Result<Fetched, FetchError> {
|
||||
// `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<String> = 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<Fetched, FetchError> {
|
||||
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<Vec<(&'static str, String)>> {
|
||||
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<bytes::Bytes, String> {
|
||||
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!({
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user