nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-31 07:30:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(artwork): treat a 404/410 image URL as not-found, not a transient fault

Browse Source 
An agent (notably Last.fm's album.getInfo) can advertise a cover URL that is
itself dead — a 404. sources.go's fromURL returned a generic error for any
non-200, so a dead URL was treated as a transient failure: it churned in
backoff and counted toward the circuit breaker, stalling valid lookups.

Map 404/410 to model.ErrNotFound in fromURL so a dead URL settles absent, and
collapse the near-identical fetchPlaylistImageURL (which already did this for
M3U covers) into it.
This commit is contained in:
Deluan 2026-07-24 18:18:53 -04:00
parent 3588f8f391
commit f7261b00ef
2 changed files with 8 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
core/artwork/resolve.go
View File

@ -10,15 +10,12 @@ import (
"image/png"
"io"
"io/fs"
"net/http"
"net/url"
"os"
"strings"
"time"
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/core/ffmpeg"
"github.com/navidrome/navidrome/model"
@ -216,7 +213,7 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents,
return resolution{}, nil
}
if remoteImg != nil && conf.Server.EnableM3UExternalAlbumArt {
sf := func() (io.ReadCloser, string, error) { return fetchPlaylistImageURL(ctx, remoteImg) }
sf := func() (io.ReadCloser, string, error) { return fromURL(ctx, remoteImg) }
if res, ok, isErr := resolveExternalStep(gate, "m3u", sf); ok {
return res, nil
} else if isErr {
@ -335,27 +332,6 @@ func classifyPlaylistImage(imageURL string) (localPath string, remote *url.URL)
}
}
// Like sources.go's fromURL but maps 404/410 to ErrNotFound (definitive), so a stale M3U
// cover URL falls through to the grid instead of retrying forever and tripping the breaker.
func fetchPlaylistImageURL(ctx context.Context, imageURL *url.URL) (io.ReadCloser, string, error) {
hc := http.Client{Timeout: 5 * time.Second}
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageURL.String(), nil)
req.Header.Set("User-Agent", consts.HTTPUserAgent)
resp, err := hc.Do(req) //nolint:gosec
if err != nil {
return nil, "", err
}
if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone {
resp.Body.Close()
return nil, "", model.ErrNotFound
}
if resp.StatusCode != http.StatusOK {
resp.Body.Close()
return nil, "", fmt.Errorf("error retrieving artwork from %s: %s", imageURL, resp.Status)
}
return resp.Body, imageURL.String(), nil
}
func resolveEmbedded(ctx context.Context, lib libraryView, ffm ffmpeg.FFmpeg, embedRel string) (resolution, bool) {
if embedRel == "" {
return resolution{}, false

7
core/artwork/sources.go
View File

@ -177,6 +177,13 @@ func fromURL(ctx context.Context, imageUrl *url.URL) (io.ReadCloser, string, err
if err != nil {
return nil, "", err
}
// A dead image URL is a definitive miss, not a transient fault: agents (e.g. Last.fm) can
// advertise an image URL that 404s. Map it to ErrNotFound so it settles absent instead of
// retrying forever and tripping the artwork breaker.
if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone {
resp.Body.Close()
return nil, "", model.ErrNotFound
}
if resp.StatusCode != http.StatusOK {
resp.Body.Close()
return nil, "", fmt.Errorf("error retrieving artwork from %s: %s", imageUrl, resp.Status)