From 6d6968e9b1e9c3ab2218499bf1a6e18ac35af5a8 Mon Sep 17 00:00:00 2001 From: KeelrLeek <156356974+KeelrLeek@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:39:14 -0500 Subject: [PATCH] artist art: a provider's API being up is no promise its CDN is On 2026-07-28 `webservice.fanart.tv` answered every request normally while `assets.fanart.tv` refused TCP on both :443 and :80. The two are different machines, and the hero chain only ever asked the first one anything. Every gate in that chain tests whether a provider RETURNED URLS, never whether those URLs load. So Fanart.tv handed back its usual eight banners and four logos, `banners.is_empty()` was false, and the two fallbacks that exist for exactly this situation -- TheAudioDB, then Bing -- were both skipped. The page then fell back to a blown-up square portrait. The tell that this was a provider outage rather than a regression in the hero code is that it was artist-shaped: every artist Fanart.tv covers lost its hero, and the ones it does not cover (Tame Impala) were untouched. Worth recording that browsing fanart.tv proves nothing here -- that is a third host, on Cloudflare, and it was up throughout. The check is a one-byte ranged GET, so it is proof the host serves image bytes without pulling a 4K background to find out. Any HTTP answer counts as alive: a 404 on one image is not an outage, so only a transport failure or a 5xx marks a host dead. Cached per host for 5 minutes, which is long enough that a dead CDN costs one probe rather than one per artist, and short enough that the hero comes back within minutes of the host recovering -- no restart, no cache clearing. The probe races the page, hence the 3s ceiling. It has to be applied in three places, and the third is the one that is easy to miss: - Discovery, after Fanart and again after TheAudioDB, so an unreachable CDN reads downstream as "this provider had nothing" -- precisely the state the fallback chain already knows how to handle. - The persisted manifest, because a manifest outlives the CDN behind it. A stored answer short-circuits discovery entirely, so replaying one produces a hero that never loads AND no path to the fallback that would have worked. - The "keep previous banners as a floor" clause. Restoring unreachable banners would undo the whole point of falling through: discovery correctly dropped a dead CDN, and the floor would put it straight back. Note for the next time this looks like a renderer bug: a 1x1 hero canvas is a SYMPTOM, not the cause. It means no image ever decoded, so the GPU renderer never sized itself. --- crates/metadata/src/lib.rs | 101 +++++++++++++++++++++++++++++++++++++ src-tauri/src/commands.rs | 40 +++++++++++---- 2 files changed, 131 insertions(+), 10 deletions(-) diff --git a/crates/metadata/src/lib.rs b/crates/metadata/src/lib.rs index cb36958..1695b69 100644 --- a/crates/metadata/src/lib.rs +++ b/crates/metadata/src/lib.rs @@ -205,6 +205,15 @@ fn stage_trace(stage: &str, started: Instant, detail: &str) { } } +/// How long a provider image host keeps its last liveness verdict. +/// +/// Long enough that a dead CDN costs one probe rather than one per artist, +/// short enough that the hero comes back on its own within minutes of the host +/// recovering — no restart, no cache clearing. +const IMAGE_HOST_PROBE_TTL: Duration = Duration::from_secs(5 * 60); +/// A liveness probe races the page, so it must never become the page's latency. +const IMAGE_HOST_PROBE_TIMEOUT: Duration = Duration::from_secs(3); + /// Cached catalog responses expire after this long. const CATALOG_TTL: Duration = Duration::from_secs(15 * 60); /// Deezer publishes a budget of 50 requests per 5 seconds per IP and answers an @@ -1572,6 +1581,12 @@ impl MetadataClient { }; let (mut tadb, (linked_mbid, (mut banners, mut logos))) = tokio::join!(self.audiodb_artist(artist), linked_fanart); + // Fanart.tv's API host and its asset host are different machines, and + // a live API is no promise of a live CDN. Verify before these URLs win, + // or a dead CDN satisfies every `is_empty()` guard below and silently + // locks out both TheAudioDB and Bing. See `image_host_alive`. + self.drop_if_host_is_down(&mut banners).await; + self.drop_if_host_is_down(&mut logos).await; // A same-name TheAudioDB record may be a completely different act. // Keep it only when one of its provider ids independently closes the // loop back to the requested Deezer identity. In particular, never use @@ -1611,6 +1626,13 @@ impl MetadataClient { } } + // Same check for whatever TheAudioDB just supplied. Cached per host, so + // when Fanart was the dead one this costs nothing, and when TheAudioDB + // is the dead one it leaves `banners` empty for the Bing fallback in + // `resolve_art` rather than handing the page more unloadable URLs. + self.drop_if_host_is_down(&mut banners).await; + self.drop_if_host_is_down(&mut logos).await; + ArtistArt { banners, logos } } @@ -2857,6 +2879,85 @@ impl MetadataClient { select_audiodb_artist(&body, artist).cloned() } + /// Is a provider's IMAGE host actually serving bytes right now? + /// + /// A provider's API and its CDN are different machines, and the API being + /// up is no promise the images are. Measured 2026-07-28: Fanart.tv's + /// `webservice.fanart.tv` answered normally while `assets.fanart.tv` + /// refused every TCP connection. Because the whole hero chain gates on + /// "did the provider return URLs" and never on "do those URLs load", every + /// artist Fanart.tv covers got eight dead banners and four dead logos — + /// and the `banners.is_empty()` guards that pick TheAudioDB and then Bing + /// were both satisfied, so neither fallback ever ran. Pages fell back to a + /// blown-up square artist portrait; only artists Fanart.tv did NOT cover + /// (Tame Impala) still had a working hero. + /// + /// One probe per host per [`IMAGE_HOST_PROBE_TTL`] turns "this provider's + /// CDN is down" into "this provider returned nothing", which is precisely + /// the state the existing fallback chain already knows how to handle. + pub async fn image_host_alive(&self, url: &str) -> bool { + static HEALTH: std::sync::OnceLock>> = + std::sync::OnceLock::new(); + let Some(host) = reqwest::Url::parse(url) + .ok() + .and_then(|parsed| parsed.host_str().map(String::from)) + else { + // Not a URL this can reason about; never block art on that. + return true; + }; + let health = HEALTH.get_or_init(|| Mutex::new(HashMap::new())); + if let Some((alive, checked)) = health.lock().await.get(&host).copied() { + if checked.elapsed() < IMAGE_HOST_PROBE_TTL { + return alive; + } + } + // A one-byte ranged GET: proof the host serves image bytes without + // pulling a 4K background to find out. Any HTTP answer means the host + // is up — a 404 for one image is not an outage — so only a transport + // failure or a server error counts as dead. + let alive = tokio::time::timeout( + IMAGE_HOST_PROBE_TIMEOUT, + self.http + .get(url) + .header(reqwest::header::RANGE, "bytes=0-0") + .send(), + ) + .await + .ok() + .and_then(|sent| sent.ok()) + .is_some_and(|resp| !resp.status().is_server_error()); + if !alive { + log::warn!("image host {host} is not serving images; falling through to next provider"); + } + health.lock().await.insert(host, (alive, Instant::now())); + alive + } + + /// Can this already-chosen artwork actually be shown right now? + /// + /// A persisted manifest is a list of URLs, and the CDN behind them can go + /// down long after it was written. Serving one then produces a hero that + /// never loads AND no path to the fallback that would have worked, because + /// the stored answer short-circuits discovery entirely. A manifest with no + /// banners is not a liveness question, and a custom data: URL has no host. + pub async fn art_is_servable(&self, art: &CatalogArtistArt) -> bool { + let Some(url) = art.banners.first().or(art.banner.as_ref()) else { + return true; + }; + self.image_host_alive(url).await + } + + /// Drop a candidate list whose host cannot currently serve it, so an + /// unreachable CDN reads downstream as "this provider had nothing". + async fn drop_if_host_is_down(&self, urls: &mut Vec) { + let Some(probe) = urls.first().cloned() else { + return; + }; + if !self.image_host_alive(&probe).await { + urls.clear(); + } + } + /// Fanart.tv's most-liked 1920×1080 backgrounds and HD ClearLOGO candidates /// for a MusicBrainz id — one call, both best-effort. Returns /// `(banners, logos)`; either list is empty when the artist has none. diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 9f392ee..1e1d351 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -1443,8 +1443,19 @@ pub async fn catalog_artist_art( // address the already-known hero immediately instead of rediscovering Bing. let stored = read_artist_art_manifest(&app, artist_id).await; let mut art = match stored { - // Fresh, current recipe: serve it, no network. - Some(stored) if stored.current => stored.art, + // Fresh, current recipe, and the host is still serving the images: + // serve it, no network. The liveness check matters because a manifest + // outlives the CDN behind it -- measured 2026-07-28, Fanart.tv's API + // stayed up while `assets.fanart.tv` refused every connection, so every + // stored manifest naming it replayed a hero that could never load and + // short-circuited the discovery that would have found a working one. + // One probe per host per 5 minutes, so the hero returns on its own once + // the CDN does. + Some(stored) + if stored.current && state.metadata.art_is_servable(&stored.art).await => + { + stored.art + } // Either nothing on disk, or something stale/older-recipe. Try to do // better, but keep what we have as a floor: the discovery tier fails // per-artist-name (Bing collapses rare names onto popular entities and @@ -1452,14 +1463,23 @@ pub async fn catalog_artist_art( // a normal event, not a reason to show an artist with no hero. other => { let resolved = state.metadata.catalog_artist_art(artist_id).await; - // Only banners we already had count as a floor. Note that - // `catalog_artist_art` returns Some as soon as the Deezer *profile* - // resolves, so "found nothing" arrives as Some with an empty banner - // list -- an emptiness check, not a None check, is what detects a - // failed hero discovery. - let previous = other - .map(|stored| stored.art) - .filter(|art| !art.banners.is_empty()); + // Only banners we already had AND can still show count as a floor. + // Restoring unreachable ones would undo the whole point of falling + // through: discovery correctly dropped a dead CDN, and the floor + // would put it straight back. Note that `catalog_artist_art` + // returns Some as soon as the Deezer *profile* resolves, so "found + // nothing" arrives as Some with an empty banner list -- an + // emptiness check, not a None check, is what detects a failed hero + // discovery. + let previous = match other.map(|stored| stored.art) { + Some(art) + if !art.banners.is_empty() + && state.metadata.art_is_servable(&art).await => + { + Some(art) + } + _ => None, + }; match (resolved, previous) { (Some(mut resolved), Some(previous)) if resolved.banners.is_empty() => { log::info!(