mirror of
https://github.com/TheFunny/TelegramTwitterMediaBot.git
synced 2026-09-23 23:32:05 +00:00
fix(retry): let slow downloads finish, and never re-run a finished task
P1 of the retry audit, from the report's "reliability and diagnosis" batch. - Media downloads no longer share the 30s *total* timeout of metadata fetches. The size caps allowed 10 MiB (reupload fallback) and 512 MiB (ugoira frame zip) while the clock allowed 30s, so a slow link made those posts impossible: `MEDIA_CLIENT` has no total timeout and instead bounds the response head and every chunk with a 30s *idle* window, which keeps the stalled-connection protection. Verified against a local probe: the old policy aborts a 40s download at 30.0s, the new one completes it (2 MiB, 40.1s), and a body that stops delivering still fails after exactly 30s. - A finished row's write-back is no longer best-effort. `delete_row` failing left the row `in_progress` with a live lease, so the next sweep flipped it back to `pending` and re-ran a completed task — a second album, a second prompt, a second channel copy. Both terminal writes are now retried, and a delete that still fails falls back to a `done` tombstone that neither the lease query nor the sweep looks at; reschedule (no safe tombstone: marking it done would drop the retry silently) logs what the sweep will do. - bsky and pixiv no longer present a *failed* video conversion as a post with no media: the remux/ugoira error propagates (pixiv keeps its retry class, bsky reports Transient), so the user sees the real cause and `fetch` gets its retries. bsky's "no ffmpeg" case stays a degradation — retrying a deployment gap cannot help. - pixiv's token exchange checks the HTTP status before parsing the body, so a 429/5xx from the OAuth endpoint stays retryable instead of becoming a permanent Api/Json error (via the shared `pixiv_error_is_retryable`), and startup validation only disables pixiv for a rejected credential — one 503 while the container came up used to turn every later pixiv link into "pixiv support is disabled".
This commit is contained in:
@@ -55,6 +55,10 @@ pub async fn fetch_from_url(url: &str) -> Result<Fetched, FetchError> {
|
||||
// working on: the media URL is derived from what the user pasted, and
|
||||
// `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.
|
||||
let mut remux_failure: Option<String> = None;
|
||||
for item in fetched.media {
|
||||
let is_hls = matches!(&item, Media::Video { url, .. }
|
||||
if url.contains("playlist") || url.ends_with(".m3u8"));
|
||||
@@ -76,10 +80,23 @@ pub async fn fetch_from_url(url: &str) -> Result<Fetched, FetchError> {
|
||||
});
|
||||
fetched._keep_alive = Some(keep_alive);
|
||||
}
|
||||
// No ffmpeg: a deployment gap, not a bad moment — retrying it
|
||||
// would only waste the fetch budget, so the post degrades (and an
|
||||
// all-video post reports the media type as unsupported).
|
||||
Ok(None) => log::warn!("bsky video remux unavailable for [key={key}]"),
|
||||
Err(e) => log::warn!("bsky video remux failed for [key={key}]: {e}"),
|
||||
Err(e) => {
|
||||
log::warn!("bsky video remux failed for [key={key}]: {e}");
|
||||
remux_failure = Some(e);
|
||||
}
|
||||
}
|
||||
}
|
||||
if media.is_empty()
|
||||
&& let Some(reason) = remux_failure
|
||||
{
|
||||
return Err(FetchError::Transient(format!(
|
||||
"bsky video remux failed: {reason}"
|
||||
)));
|
||||
}
|
||||
fetched.media = media;
|
||||
Ok(fetched)
|
||||
}
|
||||
|
||||
@@ -269,15 +269,23 @@ pub enum FetchError {
|
||||
Io(std::io::Error),
|
||||
}
|
||||
|
||||
/// Shared HTTP client (browser User-Agent) for twitter/bsky fetches and
|
||||
/// [`download_media`].
|
||||
pub(crate) static CLIENT: LazyLock<reqwest::Client> = LazyLock::new(|| {
|
||||
/// How long a download may make no progress: the response head, and then each
|
||||
/// individual chunk, must arrive within this window. Deliberately *not* a
|
||||
/// total timeout — see [`MEDIA_CLIENT`].
|
||||
const DOWNLOAD_IDLE_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Builds a client with the shared configuration (browser User-Agent, the
|
||||
/// Bot API's proxy, per-runtime pools under test). `total_timeout` is what
|
||||
/// differs between the two clients below.
|
||||
fn build_client(total_timeout: Option<Duration>) -> reqwest::Client {
|
||||
let mut builder = reqwest::Client::builder()
|
||||
.user_agent("Mozilla/5.0")
|
||||
.connect_timeout(Duration::from_secs(10));
|
||||
if let Some(total) = total_timeout {
|
||||
// reqwest has no total timeout by default; a stalled connection
|
||||
// would otherwise pin a fetch/handler forever.
|
||||
.timeout(Duration::from_secs(30))
|
||||
.connect_timeout(Duration::from_secs(10));
|
||||
builder = builder.timeout(total);
|
||||
}
|
||||
// Route site fetches through the same proxy the Bot API uses, so a
|
||||
// network that needs TELOXIDE_PROXY (e.g. behind the GFW) does not
|
||||
// leave site fetches dead while the bot itself works.
|
||||
@@ -295,7 +303,53 @@ pub(crate) static CLIENT: LazyLock<reqwest::Client> = LazyLock::new(|| {
|
||||
#[cfg(test)]
|
||||
let builder = builder.pool_max_idle_per_host(0);
|
||||
builder.build().expect("failed to build HTTP client")
|
||||
});
|
||||
}
|
||||
|
||||
/// Shared HTTP client (browser User-Agent) for the site fetches — metadata
|
||||
/// requests, where 30s is generous.
|
||||
pub(crate) static CLIENT: LazyLock<reqwest::Client> =
|
||||
LazyLock::new(|| build_client(Some(Duration::from_secs(30))));
|
||||
|
||||
/// Client for media *downloads*, with no total timeout: a 10 MiB fallback
|
||||
/// download, or an ugoira frame zip that may be hundreds of MB, legitimately
|
||||
/// takes minutes on a slow link — a 30s total cap made those posts 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`] instead (see [`next_chunk`]).
|
||||
static MEDIA_CLIENT: LazyLock<reqwest::Client> = LazyLock::new(|| build_client(None));
|
||||
|
||||
/// The error a download reports when it stops making progress.
|
||||
fn download_stalled() -> FetchError {
|
||||
FetchError::Transient(format!(
|
||||
"download stalled for {}s",
|
||||
DOWNLOAD_IDLE_TIMEOUT.as_secs()
|
||||
))
|
||||
}
|
||||
|
||||
/// Sends a media-download request: the response head must arrive within the
|
||||
/// idle window, and a non-2xx status is classified by [`download_status_error`].
|
||||
async fn send_download(request: reqwest::RequestBuilder) -> Result<reqwest::Response, FetchError> {
|
||||
let response = match tokio::time::timeout(DOWNLOAD_IDLE_TIMEOUT, request.send()).await {
|
||||
Ok(Ok(response)) => response,
|
||||
Ok(Err(e)) => return Err(e.into()),
|
||||
Err(_) => return Err(download_stalled()),
|
||||
};
|
||||
if response.status().is_success() {
|
||||
Ok(response)
|
||||
} else {
|
||||
Err(download_status_error(response.status()))
|
||||
}
|
||||
}
|
||||
|
||||
/// One body chunk, or `None` at the end. A body that stops delivering is a
|
||||
/// transient download error rather than a hang.
|
||||
async fn next_chunk(response: &mut reqwest::Response) -> Result<Option<bytes::Bytes>, FetchError> {
|
||||
match tokio::time::timeout(DOWNLOAD_IDLE_TIMEOUT, response.chunk()).await {
|
||||
Ok(Ok(chunk)) => Ok(chunk),
|
||||
Ok(Err(e)) => Err(e.into()),
|
||||
Err(_) => Err(download_stalled()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a usable `ffmpeg` binary is on PATH (probed once). Shared by the
|
||||
/// pixiv ugoira encoder and the bsky HLS remuxer.
|
||||
@@ -543,12 +597,7 @@ fn download_status_error(status: reqwest::StatusCode) -> FetchError {
|
||||
/// crossed (or when a declared Content-Length already exceeds it). Keeps the
|
||||
/// bot from buffering arbitrarily large bodies into memory.
|
||||
pub async fn download_media_limited(url: &str, max_bytes: u64) -> Result<bytes::Bytes, FetchError> {
|
||||
let response = apply_media_headers(CLIENT.get(url), url).send().await?;
|
||||
let response = if response.status().is_success() {
|
||||
response
|
||||
} else {
|
||||
return Err(download_status_error(response.status()));
|
||||
};
|
||||
let response = send_download(apply_media_headers(MEDIA_CLIENT.get(url), url)).await?;
|
||||
if let Some(len) = response.content_length()
|
||||
&& len > max_bytes
|
||||
{
|
||||
@@ -556,7 +605,7 @@ pub async fn download_media_limited(url: &str, max_bytes: u64) -> Result<bytes::
|
||||
}
|
||||
let mut response = response;
|
||||
let mut buf = Vec::new();
|
||||
while let Some(chunk) = response.chunk().await? {
|
||||
while let Some(chunk) = next_chunk(&mut response).await? {
|
||||
buf.extend_from_slice(&chunk);
|
||||
if buf.len() as u64 > max_bytes {
|
||||
return Err(FetchError::TooLarge);
|
||||
@@ -581,12 +630,7 @@ pub async fn download_media_to_file(
|
||||
out: &mut std::fs::File,
|
||||
) -> Result<u64, FetchError> {
|
||||
use std::io::Write;
|
||||
let response = apply_media_headers(CLIENT.get(url), url).send().await?;
|
||||
let response = if response.status().is_success() {
|
||||
response
|
||||
} else {
|
||||
return Err(download_status_error(response.status()));
|
||||
};
|
||||
let response = send_download(apply_media_headers(MEDIA_CLIENT.get(url), url)).await?;
|
||||
if let Some(len) = response.content_length()
|
||||
&& len > max_bytes
|
||||
{
|
||||
@@ -594,7 +638,7 @@ pub async fn download_media_to_file(
|
||||
}
|
||||
let mut response = response;
|
||||
let mut total: u64 = 0;
|
||||
while let Some(chunk) = response.chunk().await? {
|
||||
while let Some(chunk) = next_chunk(&mut response).await? {
|
||||
total += chunk.len() as u64;
|
||||
if total > max_bytes {
|
||||
return Err(FetchError::TooLarge);
|
||||
|
||||
@@ -76,6 +76,13 @@ impl PixivAPI {
|
||||
.header("User-Agent", AUTH_USER_AGENT)
|
||||
.send()
|
||||
.await?;
|
||||
// Check the status *before* reading the body: a 429/5xx from the
|
||||
// token endpoint is worth retrying (the class comes from
|
||||
// `is_retryable`), while parsing a maintenance page as JSON turned it
|
||||
// into a permanent `Api`/`Json` error with no retry at all.
|
||||
if !response.status().is_success() {
|
||||
return Err(PixivError::Status(response.status().as_u16()));
|
||||
}
|
||||
let json: serde_json::Value = serde_json::from_str(&response.text().await?)?;
|
||||
let access_token = json
|
||||
.get("access_token")
|
||||
@@ -134,8 +141,11 @@ impl PixivAPI {
|
||||
let mut illustration = Illustration::from_model(&model);
|
||||
if matches!(&model.r#type, TypeModel::Ugoira) {
|
||||
// Real ugoira support: download the frame zip and encode an MP4.
|
||||
// Without ffmpeg (or on encode failure) the post stays
|
||||
// unsupported (empty media, like Python).
|
||||
// Without ffmpeg the post stays unsupported (empty media, like
|
||||
// Python) — but a *failed* download/encode is reported instead:
|
||||
// a ugoira post has no static image to fall back to, so
|
||||
// swallowing it would present a transient zip-download error as
|
||||
// "this post has no media", with the retries skipped.
|
||||
match self.ugoira_video(illust_id).await {
|
||||
Ok(Some((mp4_path, _keep_alive))) => {
|
||||
illustration.media.push(Media::Video {
|
||||
@@ -146,7 +156,10 @@ impl PixivAPI {
|
||||
illustration._keep_alive = Some(_keep_alive);
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(e) => log::error!("ugoira encode failed for {illust_id}: {e}"),
|
||||
Err(e) => {
|
||||
log::error!("ugoira encode failed for {illust_id}: {e}");
|
||||
return Err(FetchError::Pixiv(e));
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(illustration)
|
||||
|
||||
@@ -50,8 +50,14 @@ impl Site for PixivSite {
|
||||
match super::api::validate().await {
|
||||
Ok(()) => Ok(()),
|
||||
Err(e) => {
|
||||
// Keep the old behavior: a failed login disables pixiv
|
||||
// for the rest of this process.
|
||||
// A rejected credential disables pixiv for the rest of
|
||||
// this process (it will not fix itself). A bad *moment* —
|
||||
// a 5xx or a network error while the container comes up —
|
||||
// must not: disabling on any error turned every later
|
||||
// pixiv link into "pixiv support is disabled".
|
||||
if pixiv_error_is_retryable(&e) {
|
||||
return Err(format!("{e} (transient — pixiv stays enabled)"));
|
||||
}
|
||||
super::api::disable();
|
||||
Err(format!("{e}"))
|
||||
}
|
||||
@@ -84,18 +90,24 @@ pub fn cache_key(url: &str) -> Option<String> {
|
||||
pub fn is_retryable(err: &FetchError) -> bool {
|
||||
match err {
|
||||
FetchError::Http(_) | FetchError::Transient(_) => true,
|
||||
FetchError::Pixiv(e) => match e {
|
||||
PixivError::Http(_) => true,
|
||||
PixivError::Status(code) if *code == 429 || *code >= 500 => true,
|
||||
PixivError::Status(_)
|
||||
| PixivError::Api(_)
|
||||
| PixivError::Json(_)
|
||||
| PixivError::NoAuth => false,
|
||||
},
|
||||
FetchError::Pixiv(e) => pixiv_error_is_retryable(e),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// The pixiv-specific half of the retry policy, shared with startup
|
||||
/// validation: a bad moment (429/5xx, a network error) is retryable, a
|
||||
/// rejected credential is not.
|
||||
fn pixiv_error_is_retryable(err: &PixivError) -> bool {
|
||||
match err {
|
||||
PixivError::Http(_) => true,
|
||||
PixivError::Status(code) if *code == 429 || *code >= 500 => true,
|
||||
PixivError::Status(_) | PixivError::Api(_) | PixivError::Json(_) | PixivError::NoAuth => {
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// pximg.net is hotlink-protected: downloads must carry the pixiv Referer.
|
||||
/// The match is on the media host, not the site PATTERN — pixiv's PATTERN
|
||||
/// only matches `pixiv.net/artworks/...`, never `i.pximg.net`.
|
||||
@@ -410,6 +422,16 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_validation_only_disables_on_a_definitive_failure() {
|
||||
// A bad moment: the site must stay enabled for later links.
|
||||
assert!(pixiv_error_is_retryable(&PixivError::Status(503)));
|
||||
assert!(pixiv_error_is_retryable(&PixivError::Status(429)));
|
||||
// A rejected credential is what `disable()` is for.
|
||||
assert!(!pixiv_error_is_retryable(&PixivError::Status(403)));
|
||||
assert!(!pixiv_error_is_retryable(&PixivError::NoAuth));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_retryable_classifies_transient_and_permanent() {
|
||||
// Transient: network errors, explicit transient, pixiv 429/5xx.
|
||||
|
||||
Reference in New Issue
Block a user