fix(retry): stop losing posts to transient failures and broken promises

P0 of the retry audit. The main finding: a Telegram 5xx was classified
Permanent, so one Telegram-side blip dead-lettered the post.

- `classify_request_error`: a server error is retryable again. teloxide sleeps
  10s on a 5xx and then parses the body, so the HTTP status is gone by the
  time the error arrives; it is recognised by shape instead — a JSON
  server-error description, or an `InvalidJson` whose raw body is not JSON
  (a proxy/error page). A JSON body of the wrong shape stays permanent, since
  retrying a type mismatch cannot help. Reproduced end to end: with the old
  classification a fake 502 (HTML body) logged "failed permanently" and
  dead-lettered; now it logs "queued for retry" and the retry delivers.
- The same class of mistake elsewhere: `is_media_fetch_failure` was missing
  `failed to get HTTP url content`, the description single-media URL sends
  answer with, so hotlink-rejected media failed permanently instead of going
  through the reupload fallback.
- `enqueue_retry` now reports whether the row was written, and the callers
  only promise a retry when it was — a failed enqueue (DB write) used to tell
  the user "retrying in Ns" and then deliver nothing, ever.
- A forward that fails retryably now settles the prompt instead of leaving it
  live: the queued row carries the message ids itself, and a live prompt let
  a second Confirm copy the same messages to the channel twice and let Skip
  answer "nothing was forwarded" while the row still delivered.
- A prompt that could not be sent no longer swallows the gated forward
  silently: the chat is told, since nothing would ever forward.
- `scaled_retry_delay` only scales up, so a server-asked `retry_after` above
  the 300s cap is honoured instead of retried early (which earned another 429
  and then dead-lettered the post).
- Download classification: a 4xx media download is permanent (the media is
  gone or refused) while transport errors and 429/5xx retry — previously every
  download error counted as retryable and burned the whole budget. A temp-file
  *write* failure retries too (resource exhaustion clears; a temp dir that
  cannot be created stays permanent).
- Site status mapping: 401/403 are `Blocked` (permanent) rather than
  `Transient`, so a refusal is reported at once instead of after three
  wasted attempts; and a twitter 200 that is not a tweet is no longer
  reported as withheld content (the empty `{}` withheld shape keeps
  `Sensitive`, which is what triggers the auth fallback).
This commit is contained in:
2026-09-20 20:46:18 +08:00
parent 36e5e8afe6
commit 4cf793cd7e
13 changed files with 312 additions and 60 deletions
@@ -214,6 +214,9 @@ pub async fn fetch(dynamic_id: &str) -> Result<model::Item, FetchError> {
if !status.is_success() {
return Err(match status.as_u16() {
412 => risk_control("412"),
// A refusal or an auth demand is not a bad moment (412 above is
// bilibili's risk control, which does clear on its own).
401 | 403 => FetchError::Blocked,
_ => FetchError::Transient(format!("bilibili status {status}")),
});
}
@@ -239,6 +239,9 @@ pub async fn fetch(handle: &str, rkey: &str) -> Result<Post, FetchError> {
if !status.is_success() {
return match status.as_u16() {
404 | 410 => Err(FetchError::NotFound),
// A refusal or an auth demand is not a bad moment: retrying it
// three times only delays an error the user has to see.
401 | 403 => Err(FetchError::Blocked),
_ => Err(FetchError::Transient(format!("bsky status {status}"))),
};
}
@@ -77,6 +77,8 @@ pub async fn fetch(note_id: &str) -> Result<model::Note, FetchError> {
if !status.is_success() {
return Err(match status.as_u16() {
400 => not_found_or_invalid(response).await,
// A refusal or an auth demand is not a bad moment.
401 | 403 => FetchError::Blocked,
_ => FetchError::Transient(format!("misskey status {status}")),
});
}
+25 -8
View File
@@ -525,15 +525,30 @@ pub async fn media_size(url: &str) -> Result<Option<u64>, FetchError> {
Ok(response.content_length())
}
/// Maps a media download's HTTP status onto the same classes the site
/// adapters use, so callers can tell "try again" from "this URL is dead":
/// 4xx is a property of the media (gone, refused by the host), while 429/5xx
/// is a property of the moment. A transport error never reaches this — it
/// fails in `send()` and stays [`FetchError::Http`].
fn download_status_error(status: reqwest::StatusCode) -> FetchError {
match status.as_u16() {
401 | 403 => FetchError::Blocked,
404 | 410 => FetchError::NotFound,
_ => FetchError::Transient(format!("media status {status}")),
}
}
/// Downloads a media file with a hard size cap: the body is streamed and the
/// download aborts with [`FetchError::TooLarge`] the moment the cap is
/// 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?
.error_for_status()?;
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()));
};
if let Some(len) = response.content_length()
&& len > max_bytes
{
@@ -566,10 +581,12 @@ 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?
.error_for_status()?;
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()));
};
if let Some(len) = response.content_length()
&& len > max_bytes
{
+3
View File
@@ -132,6 +132,9 @@ pub async fn fetch(id: &str) -> Result<Tweet, FetchError> {
log::warn!("twitter auth fetch {id}: HTTP {status}");
return match status.as_u16() {
404 | 410 => Err(FetchError::NotFound),
// A stale/refused `auth_token` is not a bad moment: retrying it
// three times only delays the report.
401 | 403 => Err(FetchError::Blocked),
_ => Err(FetchError::Transient(format!(
"twitter auth status {status}"
))),
+26 -1
View File
@@ -105,6 +105,9 @@ pub async fn fetch(id: &str) -> Result<Tweet, FetchError> {
if !status.is_success() {
return match status.as_u16() {
404 | 410 => Err(FetchError::NotFound),
// A refusal or an auth demand is not a bad moment: retrying it
// three times only delays an error the user has to see.
401 | 403 => Err(FetchError::Blocked),
_ => Err(FetchError::Transient(format!("twitter status {status}"))),
};
}
@@ -146,7 +149,18 @@ fn parse_syndication_body(text: &str) -> Result<serde_json::Value, FetchError> {
return Err(FetchError::NotFound);
}
if body.get("id_str").is_none() {
return Err(FetchError::Sensitive);
// Syndication answers an empty `{}` for withheld (NSFW /
// age-restricted) tweets: the documented case, kept as `Sensitive`
// because it is what triggers the logged-in auth fallback.
if body.as_object().is_some_and(|object| object.is_empty()) {
return Err(FetchError::Sensitive);
}
// Any other shape is not a tweet: an interstitial, a truncated body,
// a change on their side. Reporting that as withheld content told the
// user to set TWITTER_AUTH_TOKEN for something auth cannot fix.
return Err(FetchError::Transient(
"unexpected syndication body".to_string(),
));
}
Ok(body)
}
@@ -762,6 +776,17 @@ mod tests {
));
}
#[test]
fn syndication_unexpected_shape_is_transient_not_withheld() {
// A 200 that is not a tweet at all (an interstitial, a truncated
// body) must not be reported as withheld content: that message tells
// the user to set TWITTER_AUTH_TOKEN, which cannot fix it.
match parse_syndication_body("{\"foo\":1}") {
Err(FetchError::Transient(_)) => {}
other => panic!("expected Transient, got {other:?}"),
}
}
#[test]
fn syndication_tweet_body_passes() {
let raw = fixture(serde_json::json!([]));