Show artwork up to 20 MB, and remember and log what failed (#169)

Five Glass Cannon Network feeds name Patreon covers of about 7 MB, over the
5 MB limit on /api/art. Each page load fetched them all again, refused them,
and logged a 502 per cover, about 84 a day, with nothing in the log to say
which image or why: the reason went only to the browser. The limit is now
20 MB, a failure is answered from memory for an hour, and the fetch failure
is logged with its address and reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 13:10:18 +00:00
parent c37bcf816b
commit 1e7a858e9c
3 changed files with 62 additions and 3 deletions

View File

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] ## [Unreleased]
### Fixed
- Cover art up to 20 MB is shown, instead of nothing for anything over 5 MB, as for five of the Glass Cannon Network's feeds.
- Artwork that could not be fetched is not fetched again for an hour, instead of on every page load, and the server's log says which image it was and why.
## [0.10.3] - 2026-10-09 ## [0.10.3] - 2026-10-09
### Changed ### Changed

View File

@@ -2,6 +2,9 @@
//! of each publisher's server for every visit, and still there when a publisher is not. //! of each publisher's server for every visit, and still there when a publisher is not.
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::collections::HashMap;
use std::sync::{LazyLock, Mutex};
use std::time::{Duration, Instant};
pub fn dir() -> PathBuf { pub fn dir() -> PathBuf {
crate::config::data_dir().join("art") crate::config::data_dir().join("art")
@@ -38,6 +41,36 @@ pub fn write(file: &Path, kind: &str, body: &[u8]) -> std::io::Result<()> {
std::fs::rename(&tmp, file) std::fs::rename(&tmp, file)
} }
/// How long an address that could not be fetched is answered from memory before it is tried again.
const RETRY: Duration = Duration::from_secs(60 * 60);
/// Addresses that could not be fetched lately, and why. Without it a page shown again fetched
/// them again: five covers too big to keep were downloaded from Patreon on every load (#169).
// ponytail: in memory, and an entry goes only when it is asked for after RETRY. It holds at most
// one per artwork address a feed or item names; move it to disk or prune it on trim if that grows.
static FAILED: LazyLock<Mutex<HashMap<String, (Instant, String)>>> = LazyLock::new(Default::default);
/// Why `url` could not be fetched, if it failed less than an hour ago.
pub fn failed(url: &str) -> Option<String> {
failed_as_of(url, Instant::now())
}
fn failed_as_of(url: &str, now: Instant) -> Option<String> {
let mut map = FAILED.lock().unwrap_or_else(|e| e.into_inner());
match map.get(url) {
Some((at, why)) if now.duration_since(*at) < RETRY => Some(why.clone()),
Some(_) => {
map.remove(url);
None
}
None => None,
}
}
pub fn fail(url: &str, why: &str) {
FAILED.lock().unwrap_or_else(|e| e.into_inner()).insert(url.to_owned(), (Instant::now(), why.to_owned()));
}
/// Removes the least recently used images until what is kept fits in `limit` bytes; 0 empties it. /// Removes the least recently used images until what is kept fits in `limit` bytes; 0 empties it.
/// Returns how many went. /// Returns how many went.
pub fn trim(dir: &Path, limit: u64) -> usize { pub fn trim(dir: &Path, limit: u64) -> usize {
@@ -89,4 +122,15 @@ mod tests {
assert_eq!(trim(&d, 0), 2); assert_eq!(trim(&d, 0), 2);
let _ = std::fs::remove_dir_all(&d); let _ = std::fs::remove_dir_all(&d);
} }
#[test]
fn a_failure_is_remembered_for_an_hour() {
let u = "https://example.com/a-failure-is-remembered.png";
assert_eq!(failed(u), None);
fail(u, "larger than 5 MB");
assert_eq!(failed(u).as_deref(), Some("larger than 5 MB"));
assert_eq!(failed_as_of(u, Instant::now() + RETRY), None);
// Asked for after the hour, it is forgotten, not just ignored.
assert_eq!(failed(u), None);
}
} }

View File

@@ -1893,10 +1893,11 @@ struct ArtQuery {
/// time it is fetched from the publisher and kept on disk (`art`, up to `art_cache_mb`), so /// time it is fetched from the publisher and kept on disk (`art`, up to `art_cache_mb`), so
/// later it is fast, the publisher is not asked on every visit, and it outlives the publisher's /// later it is fast, the publisher is not asked on every visit, and it outlives the publisher's
/// server. Only an address some feed or item names as its artwork, and only an image up to /// server. Only an address some feed or item names as its artwork, and only an image up to
/// 5 MB, so the route cannot be pointed at anything else. It began as a way round http-only /// 20 MB, so the route cannot be pointed at anything else. It began as a way round http-only
/// artwork on the https page (#90). /// artwork on the https page (#90).
async fn art(State(state): State<WebState>, Query(q): Query<ArtQuery>) -> Response { async fn art(State(state): State<WebState>, Query(q): Query<ArtQuery>) -> Response {
const MAX: usize = 5 << 20; // 5 MB turned away the Glass Cannon Network's covers, 7 MB PNGs (#169).
const MAX: usize = 20 << 20;
let web = q.u.starts_with("http://") || q.u.starts_with("https://"); let web = q.u.starts_with("http://") || q.u.starts_with("https://");
if !web || !state.ctx.db.names_image(&q.u).await.unwrap_or(false) { if !web || !state.ctx.db.names_image(&q.u).await.unwrap_or(false) {
return (StatusCode::NOT_FOUND, "no feed names that artwork").into_response(); return (StatusCode::NOT_FOUND, "no feed names that artwork").into_response();
@@ -1906,6 +1907,9 @@ async fn art(State(state): State<WebState>, Query(q): Query<ArtQuery>) -> Respon
if keep && let Some((kind, body)) = crate::art::read(&file) { if keep && let Some((kind, body)) = crate::art::read(&file) {
return art_response(kind, body); return art_response(kind, body);
} }
if let Some(why) = crate::art::failed(&q.u) {
return (StatusCode::BAD_GATEWAY, why).into_response();
}
let got = async { let got = async {
let mut r = state.ctx.client.get(&q.u).timeout(crate::feed::FEED_TIMEOUT).send().await?.error_for_status()?; let mut r = state.ctx.client.get(&q.u).timeout(crate::feed::FEED_TIMEOUT).send().await?.error_for_status()?;
let kind = r.headers().get(header::CONTENT_TYPE).and_then(|v| v.to_str().ok()).unwrap_or("").to_owned(); let kind = r.headers().get(header::CONTENT_TYPE).and_then(|v| v.to_str().ok()).unwrap_or("").to_owned();
@@ -1924,7 +1928,13 @@ async fn art(State(state): State<WebState>, Query(q): Query<ArtQuery>) -> Respon
} }
art_response(kind, body) art_response(kind, body)
} }
Err(e) => (StatusCode::BAD_GATEWAY, format!("{e:#}")).into_response(), Err(e) => {
let why = format!("{e:#}");
// The access line says only that it failed, not which image or why (#169).
tracing::warn!(url = %q.u, error = %why, "could not fetch artwork");
crate::art::fail(&q.u, &why);
(StatusCode::BAD_GATEWAY, why).into_response()
}
} }
} }