mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 10:27:08 +02:00
fix(lastfm): report a failure when the artist page has no image (#6198)
GetArtistImages scrapes the og:image tag off the Last.fm artist page, because the API only ever returns the placeholder image. Last.fm now answers non-browser clients with a Fastly bot challenge, served as a 200 with valid HTML, so the query found no og:image and the agent returned an empty list with no error. The artwork worker read that as a definitive "this artist has no image" and settled the state as absent, silently and with nothing in the log. A real artist page always carries an og:image, so its absence now returns an error instead. The worker keeps the previous state, other agents still get their turn, and its per-agent circuit breaker bounds the retries. The error is deliberately not a RetryLaterError: that would park the whole Last.fm agent, including the API-backed biography, similar-artists and top-songs calls, which the page block does not affect. The new fixture is the real 3038-byte challenge page. Fixes #6192
This commit is contained in:
parent
cb7b042e36
commit
285dc4f391
3 changed files with 120 additions and 3 deletions
|
|
@ -241,6 +241,10 @@ func (l *lastfmAgent) GetSimilarSongsByTrack(ctx context.Context, id, name, arti
|
|||
var (
|
||||
artistOpenGraphQuery = cascadia.MustCompile(`html > head > meta[property="og:image"]`)
|
||||
artistIgnoredImage = "2a96cbd8b46e442fc41c2b86b821562f" // Last.fm artist placeholder image name
|
||||
|
||||
// Not a RetryLaterError on purpose: parking the agent would also stall its API-backed
|
||||
// methods, which the page block does not affect.
|
||||
errNoArtistPage = errors.New("no artist image in Last.fm page")
|
||||
)
|
||||
|
||||
func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string) ([]agents.ExternalImage, error) {
|
||||
|
|
@ -267,7 +271,9 @@ func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string)
|
|||
var res []agents.ExternalImage
|
||||
n := cascadia.Query(node, artistOpenGraphQuery)
|
||||
if n == nil {
|
||||
return res, nil
|
||||
// A real artist page always has og:image; its absence means a bot challenge or a redesign.
|
||||
log.Warn(ctx, "Last.fm did not return a usable artist page", "name", name, "url", a.URL)
|
||||
return nil, errNoArtistPage
|
||||
}
|
||||
for _, attr := range n.Attr {
|
||||
if attr.Key != "content" {
|
||||
|
|
|
|||
|
|
@ -648,18 +648,41 @@ var _ = Describe("lastfmAgent", func() {
|
|||
Expect(images).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("returns empty list if page has no meta tags", func() {
|
||||
It("errors when the page has no meta tags", func() {
|
||||
fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json")
|
||||
apiClient.Res = http.Response{Body: fApi, StatusCode: 200}
|
||||
|
||||
fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.no_meta.html")
|
||||
httpClient.Res = http.Response{Body: fScraper, StatusCode: 200}
|
||||
|
||||
_, err := agent.GetArtistImages(ctx, "123", "U2", "")
|
||||
Expect(err).To(MatchError(errNoArtistPage))
|
||||
})
|
||||
|
||||
It("errors when Last.fm serves a bot challenge page", func() {
|
||||
fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json")
|
||||
apiClient.Res = http.Response{Body: fApi, StatusCode: 200}
|
||||
|
||||
fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.challenge.html")
|
||||
httpClient.Res = http.Response{Body: fScraper, StatusCode: 200}
|
||||
|
||||
images, err := agent.GetArtistImages(ctx, "123", "U2", "")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(err).To(MatchError(errNoArtistPage))
|
||||
Expect(images).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("does not park the agent: the failure is not a retry-later", func() {
|
||||
// A RetryLaterError would cool down the agent's API-backed methods too.
|
||||
fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json")
|
||||
apiClient.Res = http.Response{Body: fApi, StatusCode: 200}
|
||||
|
||||
fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.challenge.html")
|
||||
httpClient.Res = http.Response{Body: fScraper, StatusCode: 200}
|
||||
|
||||
_, err := agent.GetArtistImages(ctx, "123", "U2", "")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns error if API call fails", func() {
|
||||
apiClient.Err = errors.New("api error")
|
||||
_, err := agent.GetArtistImages(ctx, "123", "U2", "")
|
||||
|
|
|
|||
88
tests/fixtures/lastfm.artist.page.challenge.html
vendored
Normal file
88
tests/fixtures/lastfm.artist.page.challenge.html
vendored
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
<!DOCTYPE html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta
|
||||
http-equiv="Content-Security-Policy"
|
||||
content="default-src 'self'; img-src 'self' data:; media-src 'self' data:; object-src 'none'; style-src 'self' 'sha256-o4vzfmmUENEg4chMjjRP9EuW9ucGnGIGVdbl8d0SHQQ='; script-src 'self' 'sha256-a9bHdQGvRzDwDVzx8m+Rzw+0FHZad8L0zjtBwkxOIz4=';"
|
||||
/>
|
||||
<link
|
||||
href="/_fs-ch-1T1wmsGaOgGaSxcX/assets/inter-var.woff2"
|
||||
rel="preload"
|
||||
as="font"
|
||||
type="font/woff2"
|
||||
crossorigin
|
||||
/>
|
||||
<link href="/_fs-ch-1T1wmsGaOgGaSxcX/assets/styles.css" rel="stylesheet" />
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1" />
|
||||
<title>Client Challenge</title>
|
||||
<style>
|
||||
#loading-error {
|
||||
font-size: 16px;
|
||||
font-family: 'Inter', sans-serif;
|
||||
margin-top: 10px;
|
||||
margin-left: 10px;
|
||||
display: none;
|
||||
}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<noscript>
|
||||
<div class="noscript-container">
|
||||
<div class="noscript-content">
|
||||
<img
|
||||
src="/_fs-ch-1T1wmsGaOgGaSxcX/assets/errorIcon.svg"
|
||||
alt=""
|
||||
role="presentation"
|
||||
class="error-icon"
|
||||
/>
|
||||
<span class="noscript-span"
|
||||
>JavaScript is disabled in your browser.</span
|
||||
>
|
||||
<p>Please enable JavaScript to proceed.</p>
|
||||
</div>
|
||||
</div>
|
||||
</noscript>
|
||||
<div id="loading-error" role="alert" aria-live="polite">
|
||||
A required part of this site couldn’t load. This may be due to a browser
|
||||
extension, network issues, or browser settings. Please check your
|
||||
connection, disable any ad blockers, or try using a different browser.
|
||||
</div>
|
||||
<script>
|
||||
function loadScript(src) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const script = document.createElement('script');
|
||||
script.onload = resolve;
|
||||
script.onerror = (event) => {
|
||||
console.error('Script load error event:', event);
|
||||
document.getElementById('loading-error').style.display = 'block';
|
||||
reject(
|
||||
new Error(
|
||||
`Failed to load script: ${src}, Please contact the service administrator.`
|
||||
)
|
||||
);
|
||||
};
|
||||
script.src = src;
|
||||
document.body.appendChild(script);
|
||||
});
|
||||
}
|
||||
|
||||
loadScript('/_fs-ch-1T1wmsGaOgGaSxcX/errors.js')
|
||||
.then(() => {
|
||||
const script = document.createElement('script');
|
||||
script.src = '/_fs-ch-1T1wmsGaOgGaSxcX/script.js?reload=true';
|
||||
script.onerror = (event) => {
|
||||
console.error('Script load error event:', event);
|
||||
const errorMsg = new Error(
|
||||
`Failed to load script: ${script.src}. Please contact the service administrator.`
|
||||
);
|
||||
console.error(errorMsg);
|
||||
handleScriptError();
|
||||
};
|
||||
document.body.appendChild(script);
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error(error);
|
||||
});
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
||||
Loading…
Add table
Add a link
Reference in a new issue