fix(fetch): one status table — a persistent 4xx is permanent everywhere

status_error's catch-all called every unlisted status Transient, so a 400/405/418/451 got three retries per link before the same answer (twitter syndication's broken-token 400 being the live example), and download_status_error plus misskey's and bilibili's local fallbacks each carried their own copy of the table — bilibili and misskey classifying a 404 as Transient while the center classified it NotFound. The center now makes any client error except 408/429 a refusal (permanent), the media path delegates to it as status_error("media", ...) and its duplicated fn is deleted, and misskey/bilibili fall through to the center after their own special statuses (misskey's 400 body, bilibili's 412). A table test pins every class.
This commit is contained in:
2026-09-24 02:52:10 +08:00
parent e6800fd27b
commit 3fb6b3b4da
4 changed files with 64 additions and 31 deletions
@@ -213,10 +213,11 @@ pub async fn fetch(dynamic_id: &str) -> Result<model::Item, FetchError> {
if !status.is_success() { if !status.is_success() {
return Err(match status.as_u16() { return Err(match status.as_u16() {
412 => risk_control("412"), 412 => risk_control("412"),
// A refusal or an auth demand is not a bad moment (412 above is // Everything else shares the central classes (refusals and gone
// bilibili's risk control, which does clear on its own). // posts permanent, 429/5xx retried). The local fallback used to
401 | 403 => FetchError::Blocked, // disagree: a bilibili 404 came back Transient here. 412 above is
_ => FetchError::Transient(format!("bilibili status {status}")), // bilibili's risk control, which does clear on its own.
_ => crate::site::status_error("bilibili", status),
}); });
} }
let detail: model::Detail = response.json().await.map_err(|e| FetchError::Site { let detail: model::Detail = response.json().await.map_err(|e| FetchError::Site {
+6 -15
View File
@@ -105,7 +105,11 @@ fn download_stalled() -> FetchError {
} }
/// Sends a media-download request: the response head must arrive within the /// 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`]. /// idle window, and a non-2xx status is classified by
/// [`super::status_error`] with `"media"` as the name — the same table the
/// site adapters use, so a dead URL and a bad moment read the same everywhere.
/// A transport error never reaches that table — it fails in `send()` and
/// stays [`FetchError::Http`].
async fn send_download(request: reqwest::RequestBuilder) -> Result<reqwest::Response, FetchError> { async fn send_download(request: reqwest::RequestBuilder) -> Result<reqwest::Response, FetchError> {
let response = match tokio::time::timeout(DOWNLOAD_IDLE_TIMEOUT, request.send()).await { let response = match tokio::time::timeout(DOWNLOAD_IDLE_TIMEOUT, request.send()).await {
Ok(Ok(response)) => response, Ok(Ok(response)) => response,
@@ -115,7 +119,7 @@ async fn send_download(request: reqwest::RequestBuilder) -> Result<reqwest::Resp
if response.status().is_success() { if response.status().is_success() {
Ok(response) Ok(response)
} else { } else {
Err(download_status_error(response.status())) Err(super::status_error("media", response.status()))
} }
} }
@@ -221,19 +225,6 @@ fn apply_media_headers(mut request: reqwest::RequestBuilder, url: &str) -> reqwe
request request
} }
/// 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 /// 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 /// download aborts with [`FetchError::TooLarge`] the moment the cap is
/// crossed (or when a declared Content-Length already exceeds it). Keeps the /// crossed (or when a declared Content-Length already exceeds it). Keeps the
+6 -4
View File
@@ -51,7 +51,8 @@ pub fn cache_key(url: &str) -> Option<String> {
/// Fetches a note from misskey.io by id. The API answers client failures /// Fetches a note from misskey.io by id. The API answers client failures
/// with HTTP 400 + `{"error":{"code":...}}` (NO_SUCH_NOTE → NotFound); /// with HTTP 400 + `{"error":{"code":...}}` (NO_SUCH_NOTE → NotFound);
/// everything else non-success is transient and retried by [`crate::site::fetch`]. /// every other non-success status falls through to the shared classes in
/// [`crate::site::status_error`] — persistent 4xx permanent, 429/5xx retried.
pub async fn fetch(note_id: &str) -> Result<model::Note, FetchError> { pub async fn fetch(note_id: &str) -> Result<model::Note, FetchError> {
let response = crate::site::CLIENT let response = crate::site::CLIENT
.post(API_URL) .post(API_URL)
@@ -62,9 +63,10 @@ pub async fn fetch(note_id: &str) -> Result<model::Note, FetchError> {
if !status.is_success() { if !status.is_success() {
return Err(match status.as_u16() { return Err(match status.as_u16() {
400 => not_found_or_invalid(response).await, 400 => not_found_or_invalid(response).await,
// A refusal or an auth demand is not a bad moment. // The local fallback used to disagree with the center: a misskey
401 | 403 => FetchError::Blocked, // 404 came back Transient here and was fetched three more times
_ => FetchError::Transient(format!("misskey status {status}")), // for a note that is simply gone.
_ => crate::site::status_error("misskey", status),
}); });
} }
response.json().await.map_err(|e| FetchError::Site { response.json().await.map_err(|e| FetchError::Site {
+47 -8
View File
@@ -296,17 +296,18 @@ pub enum FetchError {
Io(std::io::Error), Io(std::io::Error),
} }
/// The error class for a non-success HTTP status, as the site adapters that /// The error class for a non-success HTTP status, shared by the site
/// share this mapping classify it: 404/410 mean the post is gone and 401/403 a /// adapters, the media downloads and twitter's auth fallback: 404/410 mean
/// refusal or an auth demand — both permanent, since retrying cannot change /// the post is gone (permanent), any other client error the source answers
/// either — while everything else (429, 5xx) is transient and retried by /// on sight is a refusal (permanent too — three retries only delay the same
/// [`fetch`]. `site` only names the adapter in the transient message; a site /// answer), and only 408/429/5xx are a bad moment, retried by [`fetch`].
/// whose statuses mean something else (bilibili's 412 risk control, misskey's /// `site` only names the adapter in the message (`"media"` for downloads);
/// 400 with `NO_SUCH_NOTE`) maps those before falling back here. /// a site whose statuses mean something else (bilibili's 412 risk control,
/// misskey's 400 with `NO_SUCH_NOTE`) maps those before falling back here.
pub fn status_error(site: &'static str, status: reqwest::StatusCode) -> FetchError { pub fn status_error(site: &'static str, status: reqwest::StatusCode) -> FetchError {
match status.as_u16() { match status.as_u16() {
404 | 410 => FetchError::NotFound, 404 | 410 => FetchError::NotFound,
401 | 403 => FetchError::Blocked, code if status.is_client_error() && !matches!(code, 408 | 429) => FetchError::Blocked,
_ => FetchError::Transient(format!("{site} status {status}")), _ => FetchError::Transient(format!("{site} status {status}")),
} }
} }
@@ -733,6 +734,44 @@ mod tests {
} }
} }
#[test]
fn persistent_client_statuses_are_permanent() {
use reqwest::StatusCode;
// The one table every caller shares now: only 408, 429 and 5xx can
// answer differently on a retry. A 400 used to be Transient here and
// in two local fallbacks — twitter syndication's broken-token 400, for
// one, burned three retries per link before saying the same thing.
assert!(matches!(
status_error("x", StatusCode::NOT_FOUND),
FetchError::NotFound
));
assert!(matches!(
status_error("x", StatusCode::BAD_REQUEST),
FetchError::Blocked
));
assert!(matches!(
status_error("x", StatusCode::PAYLOAD_TOO_LARGE),
FetchError::Blocked
));
assert!(matches!(
status_error("x", StatusCode::REQUEST_TIMEOUT),
FetchError::Transient(_)
));
assert!(matches!(
status_error("x", StatusCode::TOO_MANY_REQUESTS),
FetchError::Transient(_)
));
assert!(matches!(
status_error("x", StatusCode::INTERNAL_SERVER_ERROR),
FetchError::Transient(_)
));
// The download path delegates under its own name, same classes.
assert!(matches!(
status_error("media", StatusCode::BAD_REQUEST),
FetchError::Blocked
));
}
#[tokio::test] #[tokio::test]
async fn disabled_site_is_reported_not_ignored() { async fn disabled_site_is_reported_not_ignored() {
// pixiv is the only token-gated site; with PIXIV_REFRESH_TOKEN set it // pixiv is the only token-gated site; with PIXIV_REFRESH_TOKEN set it