fix(pixiv): stop retrying permanent 4xx API errors

site::fetch retried every PixivError, so a bad/expired token (403) or a
deleted artwork (404) burned all 3 attempts with backoff against pixiv's
API for nothing. Add PixivError::Status(u16) — the app-API calls now
surface the HTTP status — and retry only the transient classes: network
errors, 429 and 5xx. 4xx / Api (token errors) / Json / NoAuth are
returned immediately. The classification is a pure helper
(fetch_error_is_retryable) with unit tests.
This commit is contained in:
2026-08-13 23:17:52 +08:00
parent 6911e9146e
commit 47935dd7c6
2 changed files with 81 additions and 10 deletions
+74 -8
View File
@@ -299,10 +299,32 @@ pub(crate) fn log_once_ffmpeg_missing() {
/// Fetches a post from its URL. Returns `Ok(None)` when no site pattern /// Fetches a post from its URL. Returns `Ok(None)` when no site pattern
/// matches (unsupported links are silently ignored by the bot). /// matches (unsupported links are silently ignored by the bot).
/// ///
/// Transient network failures are retried: 3 total attempts with 1s then 2s /// Transient failures are retried: 3 total attempts with 1s then 2s delays.
/// delays. Retried classes: bare HTTP errors, [`FetchError::Transient`] /// Retried classes: bare HTTP errors, [`FetchError::Transient`] (429/5xx
/// (429/5xx from any site), and pixiv errors (its network failures arrive /// from any site), pixiv network errors, and pixiv HTTP statuses that are
/// wrapped as `PixivError`). Non-retried: Json/NotFound/Blocked/Sensitive. /// 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<Option<Fetched>, FetchError> { pub async fn fetch(url: &str) -> Result<Option<Fetched>, FetchError> {
for attempt in 0..3u32 { for attempt in 0..3u32 {
match fetch_once(url).await { match fetch_once(url).await {
@@ -315,14 +337,13 @@ pub async fn fetch(url: &str) -> Result<Option<Fetched>, FetchError> {
return Ok(Some(fetched)); return Ok(Some(fetched));
} }
Ok(None) => return Ok(None), Ok(None) => return Ok(None),
Err(e @ (FetchError::Http(_) | FetchError::Transient(_) | FetchError::Pixiv(_))) => { Err(err) => {
if attempt < 2 { if fetch_error_is_retryable(&err) && attempt < 2 {
tokio::time::sleep(Duration::from_secs(1 << attempt)).await; tokio::time::sleep(Duration::from_secs(1 << attempt)).await;
} else { } else {
return Err(e); return Err(err);
} }
} }
Err(other) => return Err(other),
} }
} }
unreachable!("retry loop always returns") unreachable!("retry loop always returns")
@@ -453,6 +474,51 @@ mod tests {
assert_eq!(cache_key("https://example.com/not-a-post"), None); 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::<serde_json::Value>("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] #[test]
fn caption_from_fields_substitutes_and_escapes() { fn caption_from_fields_substitutes_and_escapes() {
// The format string is escaped, the field values are substituted // The format string is escaped, the field values are substituted
+7 -2
View File
@@ -29,6 +29,10 @@ pub enum PixivError {
NoAuth, NoAuth,
Http(reqwest::Error), Http(reqwest::Error),
Json(serde_json::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), Api(String),
} }
@@ -38,6 +42,7 @@ impl fmt::Display for PixivError {
PixivError::NoAuth => write!(f, "pixiv: no authentication"), PixivError::NoAuth => write!(f, "pixiv: no authentication"),
PixivError::Http(e) => write!(f, "pixiv http error: {e}"), PixivError::Http(e) => write!(f, "pixiv http error: {e}"),
PixivError::Json(e) => write!(f, "pixiv json 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}"), PixivError::Api(message) => write!(f, "pixiv api error: {message}"),
} }
} }
@@ -137,7 +142,7 @@ impl PixivAPI {
.send() .send()
.await?; .await?;
if !response.status().is_success() { 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?)?; let json: serde_json::Value = serde_json::from_str(&response.text().await?)?;
if json.get("error").is_some() { if json.get("error").is_some() {
@@ -190,7 +195,7 @@ impl PixivAPI {
.send() .send()
.await?; .await?;
if !response.status().is_success() { 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?)?; let json: serde_json::Value = serde_json::from_str(&response.text().await?)?;
if json.get("error").is_some() { if json.get("error").is_some() {