diff --git a/crates/x-media/src/site/mod.rs b/crates/x-media/src/site/mod.rs index 1e327be..d2bd96d 100644 --- a/crates/x-media/src/site/mod.rs +++ b/crates/x-media/src/site/mod.rs @@ -299,10 +299,32 @@ pub(crate) fn log_once_ffmpeg_missing() { /// Fetches a post from its URL. Returns `Ok(None)` when no site pattern /// matches (unsupported links are silently ignored by the bot). /// -/// Transient network failures are retried: 3 total attempts with 1s then 2s -/// delays. Retried classes: bare HTTP errors, [`FetchError::Transient`] -/// (429/5xx from any site), and pixiv errors (its network failures arrive -/// wrapped as `PixivError`). Non-retried: Json/NotFound/Blocked/Sensitive. +/// Transient failures are retried: 3 total attempts with 1s then 2s delays. +/// Retried classes: bare HTTP errors, [`FetchError::Transient`] (429/5xx +/// from any site), pixiv network errors, and pixiv HTTP statuses that are +/// actually transient (429 / 5xx). Permanent classes are returned +/// immediately: Json, NotFound, Blocked, Sensitive, pixiv 4xx statuses +/// (bad/expired token, forbidden, not found) and pixiv API/auth errors. +/// Whether [`fetch`] should retry `err` (3 total attempts, 1s then 2s +/// backoff). Permanent classes — 4xx statuses, invalid tokens, unparseable +/// bodies, not-found/blocked/sensitive — are returned immediately; retrying +/// them only wastes attempts against the source site. +fn fetch_error_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, + // 4xx, invalid token, unparseable body: retrying cannot help. + PixivError::Status(_) + | PixivError::Api(_) + | PixivError::Json(_) + | PixivError::NoAuth => false, + }, + _ => false, + } +} + pub async fn fetch(url: &str) -> Result, FetchError> { for attempt in 0..3u32 { match fetch_once(url).await { @@ -315,14 +337,13 @@ pub async fn fetch(url: &str) -> Result, FetchError> { return Ok(Some(fetched)); } Ok(None) => return Ok(None), - Err(e @ (FetchError::Http(_) | FetchError::Transient(_) | FetchError::Pixiv(_))) => { - if attempt < 2 { + Err(err) => { + if fetch_error_is_retryable(&err) && attempt < 2 { tokio::time::sleep(Duration::from_secs(1 << attempt)).await; } else { - return Err(e); + return Err(err); } } - Err(other) => return Err(other), } } unreachable!("retry loop always returns") @@ -453,6 +474,51 @@ mod tests { assert_eq!(cache_key("https://example.com/not-a-post"), None); } + #[test] + fn fetch_error_retryability_classification() { + // Transient: network errors, explicit transient, pixiv 429/5xx. + assert!(fetch_error_is_retryable(&FetchError::Transient( + "429".into() + ))); + assert!(fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(429) + ))); + assert!(fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(500) + ))); + assert!(fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(503) + ))); + // Permanent: pixiv 4xx (bad/expired token, forbidden, not found), + // api/auth errors, unparseable bodies, not-found/blocked/sensitive. + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(400) + ))); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(401) + ))); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(403) + ))); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Status(404) + ))); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Api("invalid_grant".into()) + ))); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::NoAuth + ))); + let json_err = serde_json::from_str::("x").unwrap_err(); + assert!(!fetch_error_is_retryable(&FetchError::Pixiv( + PixivError::Json(json_err) + ))); + assert!(!fetch_error_is_retryable(&FetchError::NotFound)); + assert!(!fetch_error_is_retryable(&FetchError::Blocked)); + assert!(!fetch_error_is_retryable(&FetchError::Sensitive)); + assert!(!fetch_error_is_retryable(&FetchError::TooLarge)); + } + #[test] fn caption_from_fields_substitutes_and_escapes() { // The format string is escaped, the field values are substituted diff --git a/crates/x-media/src/site/pixiv/api.rs b/crates/x-media/src/site/pixiv/api.rs index 5434236..256500e 100644 --- a/crates/x-media/src/site/pixiv/api.rs +++ b/crates/x-media/src/site/pixiv/api.rs @@ -29,6 +29,10 @@ pub enum PixivError { NoAuth, Http(reqwest::Error), Json(serde_json::Error), + /// Non-2xx HTTP status from the app API. The code lets [`crate::site::fetch`] + /// retry only transient classes (429 / 5xx) instead of burning attempts on + /// permanent 4xx (bad token, forbidden, not found). + Status(u16), Api(String), } @@ -38,6 +42,7 @@ impl fmt::Display for PixivError { PixivError::NoAuth => write!(f, "pixiv: no authentication"), PixivError::Http(e) => write!(f, "pixiv http error: {e}"), PixivError::Json(e) => write!(f, "pixiv json error: {e}"), + PixivError::Status(code) => write!(f, "pixiv status {code}"), PixivError::Api(message) => write!(f, "pixiv api error: {message}"), } } @@ -137,7 +142,7 @@ impl PixivAPI { .send() .await?; if !response.status().is_success() { - return Err(PixivError::Api(format!("status {}", response.status()))); + return Err(PixivError::Status(response.status().as_u16())); } let json: serde_json::Value = serde_json::from_str(&response.text().await?)?; if json.get("error").is_some() { @@ -190,7 +195,7 @@ impl PixivAPI { .send() .await?; if !response.status().is_success() { - return Err(PixivError::Api(format!("status {}", response.status()))); + return Err(PixivError::Status(response.status().as_u16())); } let json: serde_json::Value = serde_json::from_str(&response.text().await?)?; if json.get("error").is_some() {