diff --git a/crates/metadata/src/lib.rs b/crates/metadata/src/lib.rs index b4850d9..84d1b81 100644 --- a/crates/metadata/src/lib.rs +++ b/crates/metadata/src/lib.rs @@ -355,6 +355,18 @@ const POPULAR_KEEP: usize = 10; /// mandatory 1 req/s gate. const MB_RELEASE_GROUP_PAGE: usize = 100; const MB_RELEASE_GROUP_PAGES: usize = 3; +/// How many times the FIRST release-group page is retried before the supplement +/// declares MusicBrainz unavailable. +/// +/// The failures this exists for are transient by nature — a 503 out of the +/// 1 req/s limiter, or this machine having throttled itself with probes — and +/// the cost of giving up is the whole date + attribution pass for that artist. +/// Two retries behind a widening backoff is enough to ride out a blip without +/// turning a genuine outage into a slow page: the request rides the same +/// 1 req/s gate as everything else, and the identity pass is deferred, so +/// nothing on screen is waiting on it. +const MB_RETRY_ATTEMPTS: u32 = 2; +const MB_RETRY_BACKOFF: Duration = Duration::from_millis(1200); /// A resolved artist banner rarely changes and costs two external calls, so it /// is cached far longer than ordinary catalog pages. @@ -534,6 +546,20 @@ struct ArtistSupplements { listeners: HashMap, /// MusicBrainz first-release facts, keyed by [`release_date_key`]. first_released: HashMap, + /// MusicBrainz never answered, as distinct from having nothing to say. + /// + /// The two used to be the same value — a failed page `break`s and leaves an + /// empty map, which reads exactly like "this artist has no release groups". + /// One transient blip therefore cost a page its true album dates AND its + /// original-album attribution, silently, with nothing in the UI or the + /// traces to say so. It also produces false bug reports: My Bloody + /// Valentine was recorded as "the supplement is failing to inject Loveless" + /// when the supplement was fine and MusicBrainz had simply not replied that + /// once. + /// + /// Callers must not treat this as evidence of anything. See + /// [`MetadataClient::musicbrainz_first_release_dates`]. + musicbrainz_unavailable: bool, } #[derive(Debug, Clone, Default, serde::Serialize)] @@ -776,6 +802,16 @@ const HERO_SCORE_RECIPE: &str = "bing-identity-visual-v1"; #[derive(Debug, Clone)] struct MbCatalogAlbum { release_group_id: String, + /// The ARTIST's MusicBrainz id, not the release group's. + /// + /// Kept because the Last.fm scrobble table is anchored on exactly this, and + /// a supplemented album used to have no way back to it: the play-count pass + /// resolved the mbid from a DEEZER artist id, which a record Deezer never + /// carried does not have. So the records the supplement exists to rescue — + /// Loveless, Isn't Anything, Exmilitary — opened with no Plays column at + /// all. The id was in hand when the group was registered; it just was not + /// kept. + artist_mbid: String, artist: String, title: String, release_date: Option, @@ -916,6 +952,14 @@ struct DeezerReleaseEvidence { #[derive(Debug, Clone)] struct AppleCatalogAlbum { collection_id: u64, + /// The DEEZER artist id this supplement was fetched for. + /// + /// Same reason [`MbCatalogAlbum::artist_mbid`] exists: the play-count pass + /// needs a way back to the artist, and an Apple-supplemented row has no + /// Deezer ALBUM to read one from. Weezer's "Hurley (Deluxe Edition)" is an + /// Apple row, and it opened with no Plays column while the plain Deezer + /// edition beside it had one. + artist_deezer_id: u64, artist: String, title: String, cover: Option, @@ -4135,7 +4179,20 @@ impl MetadataClient { stage_trace( "page.original_dates", dates_started, - &format!("{corrected}/{} corrected, {inherited} inherited", albums.len()), + &format!( + "{corrected}/{} corrected, {inherited} inherited{}", + albums.len(), + // Without this the trace reads "0/47 corrected" whether + // MusicBrainz said the dates are already right or never + // answered at all — which is precisely the confusion that sent + // an hour chasing a regression in Eagles' dates that turned out + // to be this machine having throttled itself with probes. + if supplements.musicbrainz_unavailable { + " — MUSICBRAINZ UNAVAILABLE, dates are unverified" + } else { + "" + } + ), ); artist.album_count = Some(albums.len() as u32); albums.sort_by(|a, b| b.release_date.cmp(&a.release_date)); @@ -4158,7 +4215,18 @@ impl MetadataClient { stage_trace( "page.attribution", attribution_started, - &format!("{reattributed}/{} re-attributed", popular_tracks.len()), + &format!( + "{reattributed}/{} re-attributed{}", + popular_tracks.len(), + // Attribution reads the same facts, so it fails the same way: + // with no secondary types the rule is a no-op and every popular + // row keeps whatever pressing Deezer licensed. + if supplements.musicbrainz_unavailable { + " — MUSICBRAINZ UNAVAILABLE, rows keep Deezer's attribution" + } else { + "" + } + ), ); stage_trace( "page.total", @@ -4948,7 +5016,7 @@ impl MetadataClient { // "Lover The Secret Studio Sessions". Verified live: with the filter // Queen's supplement is exactly its 14 studio albums. let query = format!("arid:{mbid} AND primarytype:album AND -secondarytype:* AND status:official"); - self.musicbrainz_release_groups(artist, &query, false).await + self.musicbrainz_release_groups(artist, mbid, &query, false).await } /// Release groups MusicBrainz types `Mixtape/Street`, for the same @@ -4969,7 +5037,7 @@ impl MetadataClient { let query = format!( "arid:{mbid} AND primarytype:album AND secondarytype:\"Mixtape/Street\" AND status:official" ); - self.musicbrainz_release_groups(artist, &query, true).await + self.musicbrainz_release_groups(artist, mbid, &query, true).await } /// Shared body of the two supplements above: run a release-group search, @@ -4978,6 +5046,7 @@ impl MetadataClient { async fn musicbrainz_release_groups( &self, artist: &str, + artist_mbid: &str, query: &str, is_mixtape: bool, ) -> Vec { @@ -5030,6 +5099,7 @@ impl MetadataClient { id, MbCatalogAlbum { release_group_id: release_group_id.to_string(), + artist_mbid: artist_mbid.to_string(), artist: artist.to_string(), title: title.clone(), release_date: release_date.clone(), @@ -5270,13 +5340,21 @@ impl MetadataClient { /// Album details with the full tracklist (and preview URLs). pub async fn catalog_album(&self, album_id: u64) -> Option { - // Only the Deezer branch knows which artist this record belongs to, and - // the scrobble table is anchored on that id. A MusicBrainz/Apple - // supplement album simply carries no play counts. + // The scrobble table is anchored on an artist MBID. Two ways to reach + // one: a Deezer album resolves its artist id and looks the link up, and + // a MusicBrainz-supplemented album already knows it outright — it was + // fetched by that id. The second path is why Loveless, which Deezer + // never carried, now has play counts at all. let mut deezer_artist_id: Option = None; + let mut artist_mbid: Option = None; let mut page = if let Some(album) = self.mb_catalog.lock().await.get(&album_id).cloned() { + artist_mbid = Some(album.artist_mbid.clone()); self.musicbrainz_album(album_id, album).await? } else if let Some(album) = self.apple_catalog.lock().await.get(&album_id).cloned() { + // Apple has no MusicBrainz id of its own, but it does know whose + // page it was supplementing, so it reaches the scrobble table by + // the ordinary Deezer-id route. + deezer_artist_id = Some(album.artist_deezer_id); self.apple_album(album_id, album).await? } else { let body = self @@ -5337,7 +5415,8 @@ impl MetadataClient { if is_future_or_unresolved_date(page.album.release_date.as_deref()) { self.mark_unreleased_tracks(&mut page).await; } - self.apply_track_stats(&mut page, deezer_artist_id).await; + self.apply_track_stats(&mut page, deezer_artist_id, artist_mbid) + .await; Some(page) } @@ -5348,12 +5427,26 @@ impl MetadataClient { /// [`LASTFM_TRACK_LIMIT`] it reaches an album's deep cuts — measured 56 of /// 57 tracks across five 150k-390k-track catalogues. A track Last.fm has /// never seen keeps `None` and renders as a dash: unknown is not zero. - async fn apply_track_stats(&self, page: &mut CatalogAlbumPage, deezer_artist_id: Option) { - let Some(artist_id) = deezer_artist_id else { - return; - }; - let Some(mbid) = self.deezer_linked_mbid(artist_id).await else { - return; + async fn apply_track_stats( + &self, + page: &mut CatalogAlbumPage, + deezer_artist_id: Option, + artist_mbid: Option, + ) { + // A supplemented album carries the MBID already; a Deezer one has to + // resolve it from the artist id. Either way the scrobble lookup below + // is identical, and both share one cached Last.fm body per artist. + let mbid = match artist_mbid { + Some(mbid) => mbid, + None => { + let Some(artist_id) = deezer_artist_id else { + return; + }; + let Some(mbid) = self.deezer_linked_mbid(artist_id).await else { + return; + }; + mbid + } }; let Some(ranks) = self.lastfm_listener_ranks(&mbid).await else { return; @@ -5608,13 +5701,14 @@ impl MetadataClient { let Some(mbid) = self.deezer_linked_mbid(artist_id).await else { return ArtistSupplements::default(); }; - let (listeners, first_released) = tokio::join!( + let (listeners, (first_released, musicbrainz_unavailable)) = tokio::join!( self.lastfm_listener_ranks(&mbid), self.musicbrainz_first_release_dates(&mbid), ); ArtistSupplements { listeners: listeners.unwrap_or_default(), first_released, + musicbrainz_unavailable, } } @@ -5627,7 +5721,28 @@ impl MetadataClient { /// records and soundtracks are all fetched too — they are real releases /// whose dates are just as wrong on Deezer, and excluding them here would /// leave a compilation dated by its reissue. - async fn musicbrainz_first_release_dates(&self, mbid: &str) -> HashMap { + /// + /// Returns `(facts, unavailable)`. The flag is the THIRD state, and it + /// exists because the first two are indistinguishable without it: an + /// artist MusicBrainz has nothing for answers with an empty + /// `release-groups` ARRAY, while a failed request answers with nothing at + /// all. Collapsing both into an empty map is what let one blip silently + /// strip a page of its dates and its attribution. + /// + /// A `None` here is always a failure, never absence — the mbid was + /// resolved FROM MusicBrainz moments earlier (see + /// [`Self::deezer_linked_mbid`]), so the artist provably exists there. + /// + /// The first page is retried before that verdict is reached, because the + /// failure this guards against is overwhelmingly transient: a 503 from the + /// 1 req/s limiter, or our own probes having throttled the IP. A later page + /// failing is reported as partial rather than unavailable — the dates + /// already collected are real, and the discography is date-sorted so the + /// tail matters least. + async fn musicbrainz_first_release_dates( + &self, + mbid: &str, + ) -> (HashMap, bool) { let started = Instant::now(); let mut dates: HashMap = HashMap::new(); for page in 0..MB_RELEASE_GROUP_PAGES { @@ -5638,10 +5753,35 @@ impl MetadataClient { let url = format!( "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit=100&offset={offset}&fmt=json" ); - let Some(body) = self.musicbrainz_json(&url).await else { + let mut answer = self.musicbrainz_json(&url).await; + if answer.is_none() && page == 0 { + for attempt in 0..MB_RETRY_ATTEMPTS { + tokio::time::sleep(MB_RETRY_BACKOFF * (attempt + 1)).await; + net_trace(&url, &format!("mb-retry {}", attempt + 1)); + answer = self.musicbrainz_json(&url).await; + if answer.is_some() { + break; + } + } + } + let Some(body) = answer else { + if page == 0 { + log::warn!( + "musicbrainz did not answer for {mbid}; \ + album dates and original-album attribution are unavailable, not absent" + ); + stage_trace("musicbrainz.first_release_dates", started, "UNAVAILABLE"); + return (HashMap::new(), true); + } break; }; let Some(groups) = body.get("release-groups").and_then(|v| v.as_array()) else { + // A 200 whose shape we do not recognise is also not an answer + // about the artist. Same reasoning as above on the first page. + if page == 0 { + stage_trace("musicbrainz.first_release_dates", started, "MALFORMED"); + return (HashMap::new(), true); + } break; }; let returned = groups.len(); @@ -5661,7 +5801,7 @@ impl MetadataClient { started, &format!("{} groups", dates.len()), ); - dates + (dates, false) } /// All-time Last.fm listener counts for one artist's tracks, keyed by @@ -6426,6 +6566,7 @@ impl MetadataClient { id, AppleCatalogAlbum { collection_id, + artist_deezer_id: deezer_id, artist: artist.to_string(), title: title.to_string(), cover: cover.clone(),