mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
Merge branch 'master' into go-127
This commit is contained in:
commit
126d488da7
@ -405,7 +405,8 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S
|
||||
log.Warn(ctx, "Last.fm client.scrobble returned error", "track", s.Title, err)
|
||||
return errors.Join(err, scrobbler.ErrRetryLater)
|
||||
}
|
||||
if lfErr.Code == 11 || lfErr.Code == 16 {
|
||||
// 11: service offline; 16: temporarily unavailable. Rate limiting is mapped by the client.
|
||||
if lfErr.Code == 11 || lfErr.Code == 16 || errors.Is(err, scrobbler.ErrRetryLater) {
|
||||
return errors.Join(err, scrobbler.ErrRetryLater)
|
||||
}
|
||||
return errors.Join(err, scrobbler.ErrUnrecoverable)
|
||||
|
||||
@ -100,6 +100,15 @@ var _ = Describe("lastfmAgent", func() {
|
||||
Expect(httpClient.RequestCount).To(Equal(1))
|
||||
Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("U2"))
|
||||
})
|
||||
|
||||
It("returns ErrRetryLater on error 29 (rate limit exceeded)", func() {
|
||||
httpClient.Res = http.Response{
|
||||
Body: io.NopCloser(bytes.NewBufferString(`{"error":29,"message":"Rate limit exceeded"}`)),
|
||||
StatusCode: 200,
|
||||
}
|
||||
_, err := agent.GetArtistBiography(ctx, "123", "U2", "")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Language Fallback", func() {
|
||||
@ -497,6 +506,16 @@ var _ = Describe("lastfmAgent", func() {
|
||||
Expect(err).To(MatchError(scrobbler.ErrRetryLater))
|
||||
})
|
||||
|
||||
It("returns ErrRetryLater on error 29 (rate limit exceeded)", func() {
|
||||
httpClient.Res = http.Response{
|
||||
Body: io.NopCloser(bytes.NewBufferString(`{"error":29,"message":"Rate limit exceeded"}`)),
|
||||
StatusCode: 200,
|
||||
}
|
||||
|
||||
err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()})
|
||||
Expect(errors.Is(err, scrobbler.ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("returns ErrRetryLater on http errors", func() {
|
||||
httpClient.Res = http.Response{
|
||||
Body: io.NopCloser(bytes.NewBufferString(`internal server error`)),
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"crypto/md5"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@ -14,11 +15,15 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
)
|
||||
|
||||
const (
|
||||
apiBaseUrl = "https://ws.audioscrobbler.com/2.0/"
|
||||
// errCodeRateLimit is Last.fm's "rate limit exceeded"; it arrives in the body, with HTTP 200
|
||||
// and no rate-limit headers, so the body code is the only signal.
|
||||
errCodeRateLimit = 29
|
||||
)
|
||||
|
||||
type lastFMError struct {
|
||||
@ -225,7 +230,11 @@ func (c *client) makeRequest(ctx context.Context, method string, params url.Valu
|
||||
return nil, jsonErr
|
||||
}
|
||||
if response.Error != 0 {
|
||||
return &response, &lastFMError{Code: response.Error, Message: response.Message}
|
||||
var err error = &lastFMError{Code: response.Error, Message: response.Message}
|
||||
if response.Error == errCodeRateLimit {
|
||||
err = errors.Join(err, &agents.RetryLaterError{})
|
||||
}
|
||||
return &response, err
|
||||
}
|
||||
|
||||
return &response, nil
|
||||
|
||||
@ -164,6 +164,19 @@ var _ = Describe("listenBrainzAgent", func() {
|
||||
err := agent.Scrobble(ctx, "user-1", sc)
|
||||
Expect(err).To(MatchError(scrobbler.ErrUnrecoverable))
|
||||
})
|
||||
|
||||
It("keeps a 429 scrobble for retry and carries the delay", func() {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 429,
|
||||
Header: http.Header{"X-Ratelimit-Reset-In": []string{"7"}},
|
||||
Body: io.NopCloser(bytes.NewBufferString(`{"code":429,"error":"rate limited"}`)),
|
||||
}
|
||||
err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()})
|
||||
Expect(errors.Is(err, scrobbler.ErrRetryLater)).To(BeTrue())
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(7 * time.Second))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetArtistUrl", func() {
|
||||
|
||||
@ -13,6 +13,7 @@ import (
|
||||
"slices"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
)
|
||||
|
||||
@ -21,6 +22,12 @@ const (
|
||||
labsBase = "https://labs.api.listenbrainz.org/"
|
||||
)
|
||||
|
||||
// retryLaterErr reads the wait ListenBrainz asked for. It sends X-RateLimit-Reset-In
|
||||
// (delta-seconds) on every response, including the 429, and never Retry-After.
|
||||
func retryLaterErr(h http.Header) *agents.RetryLaterError {
|
||||
return &agents.RetryLaterError{RetryIn: agents.ParseRetryIn(h.Get("X-RateLimit-Reset-In"))}
|
||||
}
|
||||
|
||||
var (
|
||||
ErrorNotFound = errors.New("listenbrainz: not found")
|
||||
)
|
||||
@ -174,6 +181,9 @@ func (c *client) makeAuthenticatedRequest(ctx context.Context, method string, en
|
||||
}
|
||||
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode == http.StatusTooManyRequests {
|
||||
return nil, retryLaterErr(resp.Header)
|
||||
}
|
||||
decoder := json.NewDecoder(resp.Body)
|
||||
|
||||
var response listenBrainzResponse
|
||||
@ -185,6 +195,10 @@ func (c *client) makeAuthenticatedRequest(ctx context.Context, method string, en
|
||||
return nil, jsonErr
|
||||
}
|
||||
if response.Code != 0 && response.Code != 200 {
|
||||
// LB also reports rate limiting as a body code, not only as an HTTP status.
|
||||
if response.Code == http.StatusTooManyRequests {
|
||||
return &response, retryLaterErr(resp.Header)
|
||||
}
|
||||
return &response, &listenBrainzError{Code: response.Code, Message: response.Error}
|
||||
}
|
||||
|
||||
@ -211,6 +225,9 @@ func (c *client) makeGenericRequest(ctx context.Context, method string, endpoint
|
||||
// On a 200 code, there is no code. Decode using using error message if it exists
|
||||
if resp.StatusCode != 200 {
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode == http.StatusTooManyRequests {
|
||||
return nil, retryLaterErr(resp.Header)
|
||||
}
|
||||
decoder := json.NewDecoder(resp.Body)
|
||||
|
||||
var lbzError lbzHttpError
|
||||
|
||||
@ -4,13 +4,17 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
@ -461,4 +465,73 @@ var _ = Describe("client", func() {
|
||||
}))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("rate limiting", func() {
|
||||
It("returns RetryLaterError with the header delay on 429", func() {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 429,
|
||||
Header: http.Header{"X-Ratelimit-Reset-In": []string{"3"}},
|
||||
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"You have exceeded your rate limit."}`)),
|
||||
}
|
||||
_, err := client.validateToken(context.Background(), "token")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(3 * time.Second))
|
||||
})
|
||||
|
||||
It("returns RetryLaterError with zero delay when no header is present", func() {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 429,
|
||||
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
|
||||
}
|
||||
_, err := client.validateToken(context.Background(), "token")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
retry, _ := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(retry.RetryIn).To(BeZero())
|
||||
})
|
||||
|
||||
DescribeTable("caps absurd header values at one hour",
|
||||
func(header string) {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 429,
|
||||
Header: http.Header{"X-Ratelimit-Reset-In": []string{header}},
|
||||
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
|
||||
}
|
||||
_, err := client.validateToken(context.Background(), "token")
|
||||
retry, _ := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(retry.RetryIn).To(Equal(time.Hour))
|
||||
},
|
||||
Entry("a large value", "999999"),
|
||||
Entry("a huge value", "99999999999"),
|
||||
// Scaling this to nanoseconds before capping wraps past 2^64, landing on ~0.29s.
|
||||
Entry("a value that overflows int64 nanoseconds", "18446744074"),
|
||||
)
|
||||
|
||||
It("maps a body-level 429 sent with a non-429 status", func() {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 200,
|
||||
Header: http.Header{"X-Ratelimit-Reset-In": []string{"7"}},
|
||||
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"You have exceeded your rate limit."}`)),
|
||||
}
|
||||
_, err := client.validateToken(context.Background(), "token")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(7 * time.Second))
|
||||
})
|
||||
|
||||
It("returns RetryLaterError on a 429 from makeGenericRequest", func() {
|
||||
httpClient.Res = http.Response{
|
||||
StatusCode: 429,
|
||||
Header: http.Header{"X-Ratelimit-Reset-In": []string{"5"}},
|
||||
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
|
||||
}
|
||||
_, err := client.getArtistUrl(context.Background(), "1")
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(5 * time.Second))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -1,9 +1,13 @@
|
||||
package agents
|
||||
|
||||
import (
|
||||
"cmp"
|
||||
"context"
|
||||
"errors"
|
||||
"maps"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
@ -22,11 +26,43 @@ type PluginLoader interface {
|
||||
LoadMediaAgent(name string) (Interface, bool)
|
||||
}
|
||||
|
||||
// agentCooldown is the default cooldown duration for an agent that returns a RetryLaterError without a specific
|
||||
// RetryIn duration.
|
||||
const agentCooldown = time.Minute
|
||||
|
||||
// errUnsupported marks an agent that does not implement the requested method: it never ran,
|
||||
// so it neither answered nor throttled.
|
||||
var errUnsupported = errors.New("agent does not support this method")
|
||||
|
||||
// Agents is a meta-agent that aggregates multiple built-in and plugin agents. It tries each enabled agent in order
|
||||
// until one returns valid data.
|
||||
type Agents struct {
|
||||
ds model.DataStore
|
||||
pluginLoader PluginLoader
|
||||
cooldowns cooldowns
|
||||
}
|
||||
|
||||
// cooldowns remembers, across dispatches, which agents asked to be left alone and until when.
|
||||
type cooldowns struct {
|
||||
mu sync.RWMutex
|
||||
until map[string]time.Time
|
||||
}
|
||||
|
||||
func (c *cooldowns) active(name string) bool {
|
||||
c.mu.RLock()
|
||||
defer c.mu.RUnlock()
|
||||
return time.Now().Before(c.until[name])
|
||||
}
|
||||
|
||||
// park keeps whichever deadline is later, so a call still in flight when a longer cooldown
|
||||
// starts cannot cut it short when it finally answers.
|
||||
func (c *cooldowns) park(name string, d time.Duration) {
|
||||
until := time.Now().Add(d)
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
if until.After(c.until[name]) {
|
||||
c.until[name] = until
|
||||
}
|
||||
}
|
||||
|
||||
// GetAgents returns the singleton instance of Agents
|
||||
@ -41,6 +77,7 @@ func createAgents(ds model.DataStore, pluginLoader PluginLoader) *Agents {
|
||||
return &Agents{
|
||||
ds: ds,
|
||||
pluginLoader: pluginLoader,
|
||||
cooldowns: cooldowns{until: map[string]time.Time{}},
|
||||
}
|
||||
}
|
||||
|
||||
@ -90,12 +127,19 @@ func (a *Agents) getEnabledAgentNames() []enabledAgent {
|
||||
} else if isPlugin {
|
||||
validAgents = append(validAgents, enabledAgent{name: name, isPlugin: true})
|
||||
} else {
|
||||
log.Debug("Unknown agent ignored", "name", name)
|
||||
log.Debug("Unknown agent ignored", "name", name, "available", availableAgentNames(availablePlugins))
|
||||
}
|
||||
}
|
||||
return validAgents
|
||||
}
|
||||
|
||||
// availableAgentNames returns every name accepted by the Agents config option.
|
||||
func availableAgentNames(plugins []string) []string {
|
||||
names := append(slices.Collect(maps.Keys(Map)), plugins...)
|
||||
slices.Sort(names)
|
||||
return names
|
||||
}
|
||||
|
||||
func (a *Agents) getAgent(ea enabledAgent) Interface {
|
||||
if ea.isPlugin {
|
||||
// Try to load WASM plugin agent (if plugin loader is available)
|
||||
@ -171,7 +215,7 @@ func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (str
|
||||
return callAgentMethod(ctx, a, "GetArtistMBID", func(ag Interface) (string, error) {
|
||||
retriever, ok := ag.(ArtistMBIDRetriever)
|
||||
if !ok {
|
||||
return "", ErrNotFound
|
||||
return "", errUnsupported
|
||||
}
|
||||
return retriever.GetArtistMBID(ctx, id, name)
|
||||
})
|
||||
@ -188,7 +232,7 @@ func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (strin
|
||||
return callAgentMethod(ctx, a, "GetArtistURL", func(ag Interface) (string, error) {
|
||||
retriever, ok := ag.(ArtistURLRetriever)
|
||||
if !ok {
|
||||
return "", ErrNotFound
|
||||
return "", errUnsupported
|
||||
}
|
||||
return retriever.GetArtistURL(ctx, id, name, mbid)
|
||||
})
|
||||
@ -205,7 +249,7 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string)
|
||||
return callAgentMethod(ctx, a, "GetArtistBiography", func(ag Interface) (string, error) {
|
||||
retriever, ok := ag.(ArtistBiographyRetriever)
|
||||
if !ok {
|
||||
return "", ErrNotFound
|
||||
return "", errUnsupported
|
||||
}
|
||||
return retriever.GetArtistBiography(ctx, id, name, mbid)
|
||||
})
|
||||
@ -224,7 +268,11 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
|
||||
overLimit := int(float64(limit) * conf.Server.DevExternalArtistFetchMultiplier)
|
||||
|
||||
start := time.Now()
|
||||
attempts := newAttempts(&a.cooldowns)
|
||||
for _, enabledAgent := range a.getEnabledAgentNames() {
|
||||
if attempts.skip(enabledAgent.name) {
|
||||
continue
|
||||
}
|
||||
ag := a.getAgent(enabledAgent)
|
||||
if ag == nil {
|
||||
continue
|
||||
@ -237,6 +285,7 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
|
||||
continue
|
||||
}
|
||||
similar, err := retriever.GetSimilarArtists(ctx, id, name, mbid, overLimit)
|
||||
attempts.record(enabledAgent.name, err)
|
||||
if len(similar) > 0 && err == nil {
|
||||
if log.IsGreaterOrEqualTo(log.LevelTrace) {
|
||||
log.Debug(ctx, "Got Similar Artists", "agent", ag.AgentName(), "artist", name, "similar", similar, "elapsed", time.Since(start))
|
||||
@ -246,7 +295,7 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
|
||||
return similar, err
|
||||
}
|
||||
}
|
||||
return nil, ErrNotFound
|
||||
return nil, attempts.noResultErr()
|
||||
}
|
||||
|
||||
func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]ExternalImage, error) {
|
||||
@ -260,7 +309,7 @@ func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]
|
||||
return callAgentSliceMethod(ctx, a, "GetArtistImages", func(ag Interface) ([]ExternalImage, error) {
|
||||
retriever, ok := ag.(ArtistImageRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetArtistImages(ctx, id, name, mbid)
|
||||
})
|
||||
@ -281,7 +330,7 @@ func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid str
|
||||
return callAgentSliceMethod(ctx, a, "GetArtistTopSongs", func(ag Interface) ([]Song, error) {
|
||||
retriever, ok := ag.(ArtistTopSongsRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetArtistTopSongs(ctx, id, artistName, mbid, overLimit)
|
||||
})
|
||||
@ -295,7 +344,7 @@ func (a *Agents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*
|
||||
return callAgentMethod(ctx, a, "GetAlbumInfo", func(ag Interface) (*AlbumInfo, error) {
|
||||
retriever, ok := ag.(AlbumInfoRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetAlbumInfo(ctx, name, artist, mbid)
|
||||
})
|
||||
@ -309,7 +358,7 @@ func (a *Agents) GetAlbumImages(ctx context.Context, name, artist, mbid string)
|
||||
return callAgentSliceMethod(ctx, a, "GetAlbumImages", func(ag Interface) ([]ExternalImage, error) {
|
||||
retriever, ok := ag.(AlbumImageRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetAlbumImages(ctx, name, artist, mbid)
|
||||
})
|
||||
@ -320,7 +369,7 @@ func (a *Agents) GetSimilarSongsByTrack(ctx context.Context, id, name, artist, m
|
||||
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByTrack", func(ag Interface) ([]Song, error) {
|
||||
retriever, ok := ag.(SimilarSongsByTrackRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetSimilarSongsByTrack(ctx, id, name, artist, mbid, count)
|
||||
})
|
||||
@ -331,7 +380,7 @@ func (a *Agents) GetSimilarSongsByAlbum(ctx context.Context, id, name, artist, m
|
||||
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByAlbum", func(ag Interface) ([]Song, error) {
|
||||
retriever, ok := ag.(SimilarSongsByAlbumRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetSimilarSongsByAlbum(ctx, id, name, artist, mbid, count)
|
||||
})
|
||||
@ -349,16 +398,61 @@ func (a *Agents) GetSimilarSongsByArtist(ctx context.Context, id, name, mbid str
|
||||
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByArtist", func(ag Interface) ([]Song, error) {
|
||||
retriever, ok := ag.(SimilarSongsByArtistRetriever)
|
||||
if !ok {
|
||||
return nil, ErrNotFound
|
||||
return nil, errUnsupported
|
||||
}
|
||||
return retriever.GetSimilarSongsByArtist(ctx, id, name, mbid, count)
|
||||
})
|
||||
}
|
||||
|
||||
func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error)) (T, error) {
|
||||
// agentAttempts tallies what the enabled agents did in one dispatch.
|
||||
type agentAttempts struct {
|
||||
cooldowns *cooldowns
|
||||
throttled bool
|
||||
answered bool
|
||||
}
|
||||
|
||||
func newAttempts(c *cooldowns) agentAttempts {
|
||||
return agentAttempts{cooldowns: c}
|
||||
}
|
||||
|
||||
// skip reports whether name is still cooling down, counting it as throttled for this dispatch.
|
||||
func (t *agentAttempts) skip(name string) bool {
|
||||
if !t.cooldowns.active(name) {
|
||||
return false
|
||||
}
|
||||
t.throttled = true
|
||||
return true
|
||||
}
|
||||
|
||||
// record files one agent's outcome, parking it when it asked to be retried later.
|
||||
func (t *agentAttempts) record(name string, err error) {
|
||||
switch retry, isRetryLater := errors.AsType[*RetryLaterError](err); {
|
||||
case errors.Is(err, errUnsupported):
|
||||
case isRetryLater:
|
||||
t.cooldowns.park(name, cmp.Or(retry.RetryIn, agentCooldown))
|
||||
t.throttled = true
|
||||
default:
|
||||
t.answered = true
|
||||
}
|
||||
}
|
||||
|
||||
// noResultErr tells a retryable empty dispatch (nobody answered) from a definitive miss.
|
||||
func (t *agentAttempts) noResultErr() error {
|
||||
if t.throttled && !t.answered {
|
||||
return ErrRetryLater
|
||||
}
|
||||
return ErrNotFound
|
||||
}
|
||||
|
||||
// callAgent tries each enabled agent in order until found reports a usable result.
|
||||
func callAgent[T any](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error), found func(T) bool) (T, error) {
|
||||
var zero T
|
||||
start := time.Now()
|
||||
attempts := newAttempts(&agents.cooldowns)
|
||||
for _, enabledAgent := range agents.getEnabledAgentNames() {
|
||||
if attempts.skip(enabledAgent.name) {
|
||||
continue
|
||||
}
|
||||
ag := agents.getAgent(enabledAgent)
|
||||
if ag == nil {
|
||||
continue
|
||||
@ -367,41 +461,29 @@ func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodNa
|
||||
break
|
||||
}
|
||||
result, err := fn(ag)
|
||||
attempts.record(enabledAgent.name, err)
|
||||
if err != nil {
|
||||
log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err)
|
||||
continue
|
||||
}
|
||||
|
||||
if result != zero {
|
||||
if found(result) {
|
||||
log.Debug(ctx, "Got result", "method", methodName, "agent", ag.AgentName(), "elapsed", time.Since(start))
|
||||
return result, nil
|
||||
}
|
||||
}
|
||||
return zero, ErrNotFound
|
||||
return zero, attempts.noResultErr()
|
||||
}
|
||||
|
||||
func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error)) (T, error) {
|
||||
return callAgent(ctx, agents, methodName, fn, func(result T) bool {
|
||||
var zero T
|
||||
return result != zero
|
||||
})
|
||||
}
|
||||
|
||||
func callAgentSliceMethod[T any](ctx context.Context, agents *Agents, methodName string, fn func(Interface) ([]T, error)) ([]T, error) {
|
||||
start := time.Now()
|
||||
for _, enabledAgent := range agents.getEnabledAgentNames() {
|
||||
ag := agents.getAgent(enabledAgent)
|
||||
if ag == nil {
|
||||
continue
|
||||
}
|
||||
if utils.IsCtxDone(ctx) {
|
||||
break
|
||||
}
|
||||
results, err := fn(ag)
|
||||
if err != nil {
|
||||
log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err)
|
||||
continue
|
||||
}
|
||||
|
||||
if len(results) > 0 {
|
||||
log.Debug(ctx, "Got results", "method", methodName, "agent", ag.AgentName(), "count", len(results), "elapsed", time.Since(start))
|
||||
return results, nil
|
||||
}
|
||||
}
|
||||
return nil, ErrNotFound
|
||||
return callAgent(ctx, agents, methodName, fn, func(results []T) bool { return len(results) > 0 })
|
||||
}
|
||||
|
||||
var _ Interface = (*Agents)(nil)
|
||||
|
||||
@ -3,6 +3,8 @@ package agents
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"slices"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
@ -14,6 +16,29 @@ import (
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("cooldowns", func() {
|
||||
// Calls to one agent overlap, so a short cooldown can land after a long one started.
|
||||
It("keeps the longer deadline when a shorter park lands after it", func() {
|
||||
c := cooldowns{until: map[string]time.Time{}}
|
||||
|
||||
c.park("fake", time.Hour)
|
||||
c.park("fake", time.Millisecond)
|
||||
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
Expect(c.active("fake")).To(BeTrue())
|
||||
})
|
||||
|
||||
It("extends the deadline when the later park is longer", func() {
|
||||
c := cooldowns{until: map[string]time.Time{}}
|
||||
|
||||
c.park("fake", time.Millisecond)
|
||||
c.park("fake", time.Hour)
|
||||
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
Expect(c.active("fake")).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("Agents", func() {
|
||||
var ctx context.Context
|
||||
var cancel context.CancelFunc
|
||||
@ -67,6 +92,22 @@ var _ = Describe("Agents", func() {
|
||||
Expect(ags).ToNot(ContainElement("disabled"))
|
||||
})
|
||||
|
||||
Describe("availableAgentNames", func() {
|
||||
It("combines built-in agents with the given plugins", func() {
|
||||
names := availableAgentNames([]string{"apple-music"})
|
||||
Expect(names).To(ContainElements("apple-music", LocalAgentName, "fake", "empty"))
|
||||
})
|
||||
|
||||
It("returns the names sorted", func() {
|
||||
names := availableAgentNames([]string{"zz-plugin", "aa-plugin"})
|
||||
Expect(slices.IsSorted(names)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("works when there are no plugins", func() {
|
||||
Expect(availableAgentNames(nil)).To(ContainElement(LocalAgentName))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetArtistMBID", func() {
|
||||
It("returns on first match", func() {
|
||||
Expect(ag.GetArtistMBID(ctx, "123", "test")).To(Equal("mbid"))
|
||||
@ -160,6 +201,102 @@ var _ = Describe("Agents", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("cooldown", func() {
|
||||
It("skips an agent that returned RetryLaterError until the deadline", func() {
|
||||
mock.Err = &RetryLaterError{RetryIn: time.Hour}
|
||||
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
|
||||
// Immediately after: agent is skipped, not called
|
||||
mock.Err = nil
|
||||
calls := mock.Calls
|
||||
_, err = ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(mock.Calls).To(Equal(calls))
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
|
||||
// Providers that throttle without saying for how long (Last.fm sends no delay at all)
|
||||
// must still be parked, or the aggregate keeps calling them on every request.
|
||||
It("parks an agent that asked to be retried without a delay", func() {
|
||||
mock.Err = ErrRetryLater
|
||||
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
|
||||
mock.Err = nil
|
||||
calls := mock.Calls
|
||||
_, err = ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(mock.Calls).To(Equal(calls), "the default cooldown must outlast the request")
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("calls the agent again once the cooldown expires", func() {
|
||||
mock.Err = &RetryLaterError{RetryIn: 10 * time.Millisecond}
|
||||
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
|
||||
mock.Err = nil
|
||||
Eventually(func() (string, error) {
|
||||
return ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
}, 5*time.Second, 10*time.Millisecond).Should(Equal("bio"))
|
||||
})
|
||||
|
||||
It("returns ErrNotFound, not ErrRetryLater, when agents failed for other reasons", func() {
|
||||
mock.Err = errors.New("boom")
|
||||
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
|
||||
})
|
||||
|
||||
// ErrRetryLater tells the caller "nobody answered, do not cache this". A definitive
|
||||
// answer from any other agent is an answer, throttled peer or not.
|
||||
It("returns ErrNotFound when another agent answered with a definitive miss", func() {
|
||||
other := &mockAgent{Err: ErrNotFound}
|
||||
Register("fake2", func(model.DataStore) Interface { return other })
|
||||
conf.Server.Agents = "fake,fake2"
|
||||
ag = createAgents(ds, nil)
|
||||
mock.Err = &RetryLaterError{RetryIn: time.Hour}
|
||||
|
||||
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
|
||||
|
||||
// The cooldown was still recorded for the throttled agent
|
||||
calls := mock.Calls
|
||||
_, _ = ag.GetArtistBiography(ctx, "id", "name", "mbid")
|
||||
Expect(mock.Calls).To(Equal(calls))
|
||||
})
|
||||
|
||||
It("returns ErrNotFound when another agent answered with an empty slice", func() {
|
||||
empty := &testImageAgent{Name: "emptyImages"}
|
||||
Register("emptyImages", func(model.DataStore) Interface { return empty })
|
||||
conf.Server.Agents = "fake,emptyImages"
|
||||
ag = createAgents(ds, nil)
|
||||
mock.Err = &RetryLaterError{RetryIn: time.Hour}
|
||||
|
||||
_, err := ag.GetArtistImages(ctx, "123", "test", "mb123")
|
||||
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns ErrRetryLater from GetSimilarArtists when only cooling agents remain", func() {
|
||||
mock.Err = &RetryLaterError{RetryIn: time.Hour}
|
||||
_, err := ag.GetSimilarArtists(ctx, "123", "test", "mb123", 2)
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("returns ErrNotFound from GetSimilarArtists when another agent answered", func() {
|
||||
other := &mockAgent{Err: ErrNotFound}
|
||||
Register("fake2", func(model.DataStore) Interface { return other })
|
||||
conf.Server.Agents = "fake,fake2"
|
||||
ag = createAgents(ds, nil)
|
||||
mock.Err = &RetryLaterError{RetryIn: time.Hour}
|
||||
|
||||
_, err := ag.GetSimilarArtists(ctx, "123", "test", "mb123", 2)
|
||||
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
|
||||
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetArtistImages", func() {
|
||||
It("returns on first match", func() {
|
||||
Expect(ag.GetArtistImages(ctx, "123", "test", "mb123")).To(Equal([]ExternalImage{{
|
||||
@ -423,8 +560,9 @@ var _ = Describe("Agents", func() {
|
||||
})
|
||||
|
||||
type mockAgent struct {
|
||||
Args []any
|
||||
Err error
|
||||
Args []any
|
||||
Err error
|
||||
Calls int
|
||||
}
|
||||
|
||||
func (a *mockAgent) AgentName() string {
|
||||
@ -449,6 +587,7 @@ func (a *mockAgent) GetArtistURL(_ context.Context, id, name, mbid string) (stri
|
||||
|
||||
func (a *mockAgent) GetArtistBiography(_ context.Context, id, name, mbid string) (string, error) {
|
||||
a.Args = []any{id, name, mbid}
|
||||
a.Calls++
|
||||
if a.Err != nil {
|
||||
return "", a.Err
|
||||
}
|
||||
|
||||
@ -3,6 +3,9 @@ package agents
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"github.com/gohugoio/hashstructure"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
@ -52,11 +55,49 @@ func (s Song) Equals(other Song) bool {
|
||||
return h1 == h2
|
||||
}
|
||||
|
||||
var (
|
||||
// ErrNotFound means the provider answered and had nothing. Return the underlying error
|
||||
// for a fault instead, or callers that back off on faults will treat it as definitive.
|
||||
ErrNotFound = errors.New("not found")
|
||||
)
|
||||
// ErrNotFound means the provider answered and had nothing. Return the underlying error
|
||||
// for a fault instead, or callers that back off on faults will treat it as definitive.
|
||||
var ErrNotFound = errors.New("not found")
|
||||
|
||||
// ErrRetryLater is the zero-delay RetryLaterError: the provider is temporarily unavailable
|
||||
// or throttling us, but did not say for how long. Both errors.Is(err, ErrRetryLater) and
|
||||
// errors.AsType[*RetryLaterError] match it and every delay-carrying variant.
|
||||
// Treat it as immutable; build a new RetryLaterError to name a delay.
|
||||
var ErrRetryLater = &RetryLaterError{}
|
||||
|
||||
// RetryLaterError asks callers to back off, optionally for the delay the provider requested.
|
||||
type RetryLaterError struct {
|
||||
RetryIn time.Duration
|
||||
}
|
||||
|
||||
func (e *RetryLaterError) Error() string {
|
||||
if e.RetryIn > 0 {
|
||||
return fmt.Sprintf("retry later (in %s)", e.RetryIn)
|
||||
}
|
||||
return "retry later"
|
||||
}
|
||||
|
||||
func (e *RetryLaterError) Is(target error) bool {
|
||||
_, ok := target.(*RetryLaterError)
|
||||
return ok
|
||||
}
|
||||
|
||||
// MaxRetryIn caps a delay parsed from a provider, so a bogus value cannot park it indefinitely.
|
||||
const MaxRetryIn = time.Hour
|
||||
const maxRetryInSeconds = int(MaxRetryIn / time.Second)
|
||||
|
||||
// ParseRetryIn reads a provider's delay given in seconds, from a header or a plugin token.
|
||||
// Anything unparseable or non-positive means unspecified.
|
||||
func ParseRetryIn(seconds string) time.Duration {
|
||||
// Clamp in seconds: scaling first would wrap a huge value past int64 nanoseconds,
|
||||
// turning "wait an age" into a fraction of a second. Parse at a fixed width so the
|
||||
// cap holds on the 32-bit targets we ship, where a plain Atoi would overflow first.
|
||||
secs, err := strconv.ParseInt(seconds, 10, 64)
|
||||
if err != nil || secs <= 0 {
|
||||
return 0
|
||||
}
|
||||
return time.Duration(min(secs, int64(maxRetryInSeconds))) * time.Second
|
||||
}
|
||||
|
||||
// AlbumInfoRetriever provides album info (no images)
|
||||
type AlbumInfoRetriever interface {
|
||||
|
||||
@ -1,27 +1,42 @@
|
||||
package agents
|
||||
package agents_test
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/core/scrobbler"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("Song.Equals", func() {
|
||||
base := Song{ID: "1", Name: "S", Artists: []Artist{{ID: "x", Name: "A"}}}
|
||||
It("true for identical songs incl Artists", func() {
|
||||
Expect(base.Equals(base)).To(BeTrue())
|
||||
var _ = Describe("RetryLaterError", func() {
|
||||
It("matches the ErrRetryLater sentinel via errors.Is", func() {
|
||||
err := &agents.RetryLaterError{RetryIn: 30 * time.Second}
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
It("false when Artists differ", func() {
|
||||
other := base
|
||||
other.Artists = []Artist{{ID: "y", Name: "B"}}
|
||||
Expect(base.Equals(other)).To(BeFalse())
|
||||
|
||||
It("matches through errors.Join and wrapping", func() {
|
||||
err := fmt.Errorf("calling LB: %w", errors.Join(errors.New("http 429"), &agents.RetryLaterError{}))
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
It("false when a scalar differs", func() {
|
||||
other := base
|
||||
other.Name = "T"
|
||||
Expect(base.Equals(other)).To(BeFalse())
|
||||
|
||||
It("exposes the delay through the wrapped error", func() {
|
||||
err := errors.Join(errors.New("http 429"), &agents.RetryLaterError{RetryIn: 42 * time.Second})
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(42 * time.Second))
|
||||
})
|
||||
It("true when both have empty Artists and equal scalars", func() {
|
||||
a := Song{ID: "1", Name: "S"}
|
||||
Expect(a.Equals(a)).To(BeTrue())
|
||||
|
||||
It("matches the sentinel too, reporting no delay", func() {
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](agents.ErrRetryLater)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(BeZero())
|
||||
})
|
||||
|
||||
It("is the same sentinel as scrobbler.ErrRetryLater", func() {
|
||||
Expect(errors.Is(scrobbler.ErrRetryLater, agents.ErrRetryLater)).To(BeTrue())
|
||||
Expect(errors.Is(&agents.RetryLaterError{}, scrobbler.ErrRetryLater)).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
27
core/agents/song_test.go
Normal file
27
core/agents/song_test.go
Normal file
@ -0,0 +1,27 @@
|
||||
package agents
|
||||
|
||||
import (
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("Song.Equals", func() {
|
||||
base := Song{ID: "1", Name: "S", Artists: []Artist{{ID: "x", Name: "A"}}}
|
||||
It("true for identical songs incl Artists", func() {
|
||||
Expect(base.Equals(base)).To(BeTrue())
|
||||
})
|
||||
It("false when Artists differ", func() {
|
||||
other := base
|
||||
other.Artists = []Artist{{ID: "y", Name: "B"}}
|
||||
Expect(base.Equals(other)).To(BeFalse())
|
||||
})
|
||||
It("false when a scalar differs", func() {
|
||||
other := base
|
||||
other.Name = "T"
|
||||
Expect(base.Equals(other)).To(BeFalse())
|
||||
})
|
||||
It("true when both have empty Artists and equal scalars", func() {
|
||||
a := Song{ID: "1", Name: "S"}
|
||||
Expect(a.Equals(a)).To(BeTrue())
|
||||
})
|
||||
})
|
||||
@ -2,6 +2,7 @@ package artwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/url"
|
||||
|
||||
@ -41,22 +42,36 @@ func bestImageURL(imgs []agents.ExternalImage) *url.URL {
|
||||
return best
|
||||
}
|
||||
|
||||
// fetchArtistImage tries each enabled artist-image agent in order. extErr is true only when no
|
||||
// longerRetry keeps whichever external failure asks for the longer wait, so one provider's
|
||||
// short delay cannot shorten another's.
|
||||
func longerRetry(a, b error) error {
|
||||
if a == nil {
|
||||
return b
|
||||
}
|
||||
var ra, rb *agents.RetryLaterError
|
||||
if errors.As(b, &rb) && (!errors.As(a, &ra) || rb.RetryIn > ra.RetryIn) {
|
||||
return b
|
||||
}
|
||||
return a
|
||||
}
|
||||
|
||||
// fetchArtistImage tries each enabled artist-image agent in order. The error is non-nil only when no
|
||||
// agent succeeded and at least one failed transiently.
|
||||
func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar model.Artist) (r io.ReadCloser, agentName string, extErr bool) {
|
||||
func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar model.Artist) (io.ReadCloser, string, error) {
|
||||
// Synthetic artists would otherwise get an unrelated agent result assigned to them.
|
||||
switch ar.ID {
|
||||
case consts.UnknownArtistID, consts.VariousArtistsID:
|
||||
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"})
|
||||
return nil, "", false
|
||||
return nil, "", nil
|
||||
}
|
||||
name := externalName(ar.Name)
|
||||
imageAgents := ag.ArtistImageAgents()
|
||||
if len(imageAgents) == 0 {
|
||||
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped,
|
||||
Detail: "no enabled agent provides artist images"})
|
||||
return nil, "", false
|
||||
return nil, "", nil
|
||||
}
|
||||
var extErr error
|
||||
for _, a := range imageAgents {
|
||||
reader, path, err := gate(a.Name, func() (io.ReadCloser, string, error) {
|
||||
imgs, err := a.Retriever.GetArtistImages(ctx, ar.ID, name, ar.MbzArtistID)
|
||||
@ -71,10 +86,10 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
|
||||
})
|
||||
recordAgent(ctx, a.Name, reader, path, err)
|
||||
if reader != nil {
|
||||
return reader, a.Name, false
|
||||
return reader, a.Name, nil
|
||||
}
|
||||
if isTransientExternal(err) {
|
||||
extErr = true
|
||||
extErr = longerRetry(extErr, err)
|
||||
log.Debug(ctx, "Artwork: External artist-image lookup failed", "agent", a.Name, "artist", ar.Name, err)
|
||||
}
|
||||
}
|
||||
@ -82,14 +97,15 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
|
||||
}
|
||||
|
||||
// fetchAlbumImage is the album counterpart of fetchArtistImage.
|
||||
func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al model.Album) (r io.ReadCloser, agentName string, extErr bool) {
|
||||
func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al model.Album) (io.ReadCloser, string, error) {
|
||||
name, artist := externalName(al.Name), externalName(al.AlbumArtist)
|
||||
imageAgents := ag.AlbumImageAgents()
|
||||
if len(imageAgents) == 0 {
|
||||
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped,
|
||||
Detail: "no enabled agent provides album images"})
|
||||
return nil, "", false
|
||||
return nil, "", nil
|
||||
}
|
||||
var extErr error
|
||||
for _, a := range imageAgents {
|
||||
reader, path, err := gate(a.Name, func() (io.ReadCloser, string, error) {
|
||||
imgs, err := a.Retriever.GetAlbumImages(ctx, name, artist, al.MbzAlbumID)
|
||||
@ -104,10 +120,10 @@ func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al m
|
||||
})
|
||||
recordAgent(ctx, a.Name, reader, path, err)
|
||||
if reader != nil {
|
||||
return reader, a.Name, false
|
||||
return reader, a.Name, nil
|
||||
}
|
||||
if isTransientExternal(err) {
|
||||
extErr = true
|
||||
extErr = longerRetry(extErr, err)
|
||||
log.Debug(ctx, "Artwork: External album-image lookup failed", "agent", a.Name, "album", al.Name, err)
|
||||
}
|
||||
}
|
||||
|
||||
@ -2,11 +2,13 @@ package artwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
@ -153,11 +155,11 @@ var _ = Describe("agent images", func() {
|
||||
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
|
||||
ag := imageAgents(a)
|
||||
|
||||
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1", Name: "Artist"})
|
||||
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1", Name: "Artist"})
|
||||
Expect(r).ToNot(BeNil())
|
||||
defer r.Close()
|
||||
Expect(name).To(Equal("agentA"))
|
||||
Expect(extErr).To(BeFalse())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("skips the external lookup for synthetic artists", func() {
|
||||
@ -165,10 +167,10 @@ var _ = Describe("agent images", func() {
|
||||
ag := imageAgents(a)
|
||||
|
||||
for _, id := range []string{consts.UnknownArtistID, consts.VariousArtistsID} {
|
||||
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: id, Name: "Various Artists"})
|
||||
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: id, Name: "Various Artists"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(name).To(BeEmpty())
|
||||
Expect(extErr).To(BeFalse())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
}
|
||||
Expect(a.artistCalls).To(Equal(0), "synthetic artists never reach the agents")
|
||||
})
|
||||
@ -177,9 +179,9 @@ var _ = Describe("agent images", func() {
|
||||
ag := imageAgents()
|
||||
t := &ChainTrace{}
|
||||
|
||||
r, _, extErr := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
r, _, err := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(extErr).To(BeFalse())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
|
||||
Detail: "no enabled agent provides artist images"}}),
|
||||
"a configured external token must never be silently absent from the chain")
|
||||
@ -211,11 +213,11 @@ var _ = Describe("agent images", func() {
|
||||
b := &fakeImageAgent{name: "agentB", imgs: []agents.ExternalImage{img("/b", 50)}}
|
||||
ag := imageAgents(a, b)
|
||||
|
||||
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(r).ToNot(BeNil())
|
||||
defer r.Close()
|
||||
Expect(name).To(Equal("agentB"))
|
||||
Expect(extErr).To(BeFalse(), "a later hit clears an earlier agent's error")
|
||||
Expect(err).ToNot(HaveOccurred(), "a later hit clears an earlier agent's error")
|
||||
Expect(a.artistCalls).To(Equal(1))
|
||||
Expect(b.artistCalls).To(Equal(1))
|
||||
})
|
||||
@ -225,20 +227,43 @@ var _ = Describe("agent images", func() {
|
||||
b := &fakeImageAgent{name: "agentB", err: agents.ErrNotFound}
|
||||
ag := imageAgents(a, b)
|
||||
|
||||
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(name).To(BeEmpty())
|
||||
Expect(extErr).To(BeFalse(), "not-found is definitive, never a transient failure")
|
||||
Expect(err).ToNot(HaveOccurred(), "not-found is definitive, never a transient failure")
|
||||
})
|
||||
|
||||
It("reports extErr when one agent fails transiently and the rest find nothing", func() {
|
||||
It("reports an error when one agent fails transiently and the rest find nothing", func() {
|
||||
a := &fakeImageAgent{name: "agentA", err: agents.ErrNotFound}
|
||||
b := &fakeImageAgent{name: "agentB", err: context.DeadlineExceeded}
|
||||
ag := imageAgents(a, b)
|
||||
|
||||
r, _, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
r, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(extErr).To(BeTrue())
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
|
||||
// The worker reschedules on this delay, so it is only honored if the agent loop
|
||||
// returns it. Two throttled agents: the longest wait is the one that must survive.
|
||||
It("returns the longest retry delay the providers asked for", func() {
|
||||
a := &fakeImageAgent{name: "agentA", err: &agents.RetryLaterError{RetryIn: 10 * time.Second}}
|
||||
b := &fakeImageAgent{name: "agentB", err: &agents.RetryLaterError{RetryIn: 5 * time.Second}}
|
||||
ag := imageAgents(a, b)
|
||||
|
||||
r, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(r).To(BeNil())
|
||||
retry, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(retry.RetryIn).To(Equal(10 * time.Second))
|
||||
})
|
||||
|
||||
It("returns no delay when the provider did not ask for one", func() {
|
||||
ag := imageAgents(&fakeImageAgent{name: "agentA", err: errors.New("boom")})
|
||||
|
||||
_, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
|
||||
Expect(err).To(HaveOccurred())
|
||||
_, ok := errors.AsType[*agents.RetryLaterError](err)
|
||||
Expect(ok).To(BeFalse(), "a plain failure must not look like a throttle")
|
||||
})
|
||||
})
|
||||
|
||||
@ -247,11 +272,11 @@ var _ = Describe("agent images", func() {
|
||||
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
|
||||
ag := imageAgents(a)
|
||||
|
||||
r, name, extErr := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album", AlbumArtist: "Artist"})
|
||||
r, name, err := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album", AlbumArtist: "Artist"})
|
||||
Expect(r).ToNot(BeNil())
|
||||
defer r.Close()
|
||||
Expect(name).To(Equal("agentA"))
|
||||
Expect(extErr).To(BeFalse())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(a.albumCalls).To(Equal(1))
|
||||
})
|
||||
|
||||
@ -259,21 +284,21 @@ var _ = Describe("agent images", func() {
|
||||
ag := imageAgents()
|
||||
t := &ChainTrace{}
|
||||
|
||||
r, _, extErr := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"})
|
||||
r, _, err := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(extErr).To(BeFalse())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
|
||||
Detail: "no enabled agent provides album images"}}),
|
||||
"a configured external token must never be silently absent from the chain")
|
||||
})
|
||||
|
||||
It("reports extErr when the only agent fails transiently", func() {
|
||||
It("reports an error when the only agent fails transiently", func() {
|
||||
a := &fakeImageAgent{name: "agentA", err: context.DeadlineExceeded}
|
||||
ag := imageAgents(a)
|
||||
|
||||
r, _, extErr := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album"})
|
||||
r, _, err := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(extErr).To(BeTrue())
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@ -16,6 +16,7 @@ import (
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/core/artwork/blurhash"
|
||||
"github.com/navidrome/navidrome/core/artwork/dominant"
|
||||
"github.com/navidrome/navidrome/core/artwork/thumbhash"
|
||||
@ -80,7 +81,7 @@ type processor struct {
|
||||
|
||||
// acquire resolves one queue item end to end: find an image, hash/decode/
|
||||
// blurhash it, place its bytes, and persist the resulting state.
|
||||
func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired) {
|
||||
func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired, retryIn time.Duration) {
|
||||
repo := p.ds.Artwork(ctx)
|
||||
start := time.Now()
|
||||
defer func() {
|
||||
@ -92,10 +93,13 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
if err != nil {
|
||||
traceStage(ctx, "resolve", err)
|
||||
log.Warn(ctx, "Artwork: Could not resolve item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, 0
|
||||
}
|
||||
if retry, ok := errors.AsType[*agents.RetryLaterError](res.extErr); ok {
|
||||
retryIn = retry.RetryIn
|
||||
}
|
||||
if res.reader == nil {
|
||||
if res.extError || res.localError {
|
||||
if res.extErr != nil || res.localError {
|
||||
// A fault is not a definitive "no image": never settle absent, keep serving old state.
|
||||
// A chainless resolver (playlist/radio) records no step, so leave a fallback or explain is blank.
|
||||
if t := traceFrom(ctx); len(t.Steps()) == 0 {
|
||||
@ -106,10 +110,10 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
t.add(TraceStep{Candidate: cmp.Or(res.source, "source"), Outcome: outcome})
|
||||
}
|
||||
log.Debug(ctx, "Artwork: No image, but a source faulted; keeping previous state",
|
||||
"kind", item.ItemKind, "id", item.ItemID, "extError", res.extError, "localError", res.localError)
|
||||
return outcomeFailed, nil
|
||||
"kind", item.ItemKind, "id", item.ItemID, "extErr", res.extErr, "localError", res.localError)
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
return writeAbsent(ctx, repo, item), nil
|
||||
return writeAbsent(ctx, repo, item), nil, 0
|
||||
}
|
||||
defer res.reader.Close()
|
||||
|
||||
@ -118,7 +122,7 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
if err != nil {
|
||||
traceStage(ctx, "read", err)
|
||||
log.Warn(ctx, "Artwork: Failed to read resolved image", "kind", item.ItemKind, "id", item.ItemID, "source", res.source, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
log.Debug(ctx, "Artwork: Read resolved image", "kind", item.ItemKind, "id", item.ItemID,
|
||||
"source", res.source, "bytes", len(data), "elapsed", time.Since(readStart))
|
||||
@ -128,7 +132,7 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
if err != nil {
|
||||
traceStage(ctx, "hash", err)
|
||||
log.Warn(ctx, "Artwork: Failed to hash image", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
log.Trace(ctx, "Artwork: Hashed image", "kind", item.ItemKind, "id", item.ItemID,
|
||||
"hash", hash, "bytes", len(data), "elapsed", time.Since(hashStart))
|
||||
@ -152,14 +156,14 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
if err != nil {
|
||||
traceStage(ctx, "decode", err)
|
||||
log.Warn(ctx, "Artwork: Failed to decode resolved image", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
log.Debug(ctx, "Artwork: Decoded new image", "kind", item.ItemKind, "id", item.ItemID, "hash", hash,
|
||||
"width", art.Width, "height", art.Height, "mime", art.Mime, "elapsed", time.Since(decodeStart))
|
||||
default:
|
||||
traceStage(ctx, "lookup", err)
|
||||
log.Warn(ctx, "Artwork: Failed to look up image hash", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
art.SizeBytes = int64(len(data))
|
||||
|
||||
@ -167,15 +171,15 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
|
||||
if err != nil {
|
||||
traceStage(ctx, "store", err)
|
||||
log.Warn(ctx, "Artwork: Failed to persist resolved image", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
return outcomeFailed, nil
|
||||
return outcomeFailed, nil, retryIn
|
||||
}
|
||||
got = &acquired{ia: ia, mime: art.Mime, data: data}
|
||||
if res.extError {
|
||||
if res.extErr != nil {
|
||||
log.Debug(ctx, "Artwork: Serving a lower-priority source after an external failure",
|
||||
"kind", item.ItemKind, "id", item.ItemID, "source", res.source)
|
||||
return outcomeFoundStale, got
|
||||
return outcomeFoundStale, got, retryIn
|
||||
}
|
||||
return outcomeFound, got
|
||||
return outcomeFound, got, retryIn
|
||||
}
|
||||
|
||||
// persist places the bytes and commits the rows referencing them, excluding Prune for that
|
||||
|
||||
@ -90,7 +90,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "al1", Name: "Album", FolderIDs: []string{"f1"}},
|
||||
})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
|
||||
@ -127,7 +127,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "alL1", Name: "Album", FolderIDs: []string{"f1"}},
|
||||
})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL1"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL1"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
Expect(lock.locks).To(BeNumerically(">", 0), "the write window must exclude prune")
|
||||
Expect(lock.held()).To(BeFalse(), "the window must close before acquire returns")
|
||||
@ -141,7 +141,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "alL2", Name: "Album", FolderIDs: []string{"f1"}},
|
||||
})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL2"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL2"})
|
||||
Expect(out).To(Equal(outcomeAbsent))
|
||||
Expect(lock.locks).To(BeZero())
|
||||
})
|
||||
@ -153,7 +153,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
})
|
||||
folderRepo.result = nil
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al2", model.ImageTypePrimary)
|
||||
@ -176,7 +176,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "al3", Name: "Album"},
|
||||
})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
|
||||
Expect(out).To(Equal(outcomeAbsent))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al3", model.ImageTypePrimary)
|
||||
@ -197,7 +197,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "al-io", Name: "Album", FolderIDs: []string{"f1"}},
|
||||
})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al-io"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al-io"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al-io", model.ImageTypePrimary)
|
||||
@ -222,7 +222,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
DeferCleanup(func() { _ = os.Chmod(upload, 0o600) })
|
||||
radioRepo.Data["ra-io"] = &model.Radio{ID: "ra-io", Name: "Station", UploadedImage: "ra-io.jpg"}
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-io"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-io"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra-io", model.ImageTypePrimary)
|
||||
@ -249,7 +249,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
radioRepo.Data["ra-tr"] = &model.Radio{ID: "ra-tr", Name: "Station", UploadedImage: "ra-tr.jpg"}
|
||||
|
||||
trace := &ChainTrace{}
|
||||
out, _ := proc.acquire(withTrace(ctx, trace), model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-tr"})
|
||||
out, _, _ := proc.acquire(withTrace(ctx, trace), model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-tr"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
steps := trace.Steps()
|
||||
@ -265,13 +265,26 @@ var _ = Describe("processor.acquire", func() {
|
||||
})
|
||||
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
|
||||
out, _, retryIn := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
Expect(retryIn).To(BeZero(), "a plain failure asks for no particular delay")
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al4", model.ImageTypePrimary)
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
})
|
||||
|
||||
It("failed-on-extError: reports the delay a throttled provider asked for", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
|
||||
{ID: "al4r", Name: "Album"},
|
||||
})
|
||||
imageAgents(&fakeImageAgent{name: "throttled", err: &agents.RetryLaterError{RetryIn: 42 * time.Second}})
|
||||
|
||||
out, _, retryIn := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4r"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
Expect(retryIn).To(Equal(42 * time.Second))
|
||||
})
|
||||
|
||||
It("found-stale: a fallback hit after a transient external failure persists state and returns outcomeFoundStale", func() {
|
||||
conf.Server.CoverArtPriority = "external, cover.jpg"
|
||||
folderRepo.result = []model.Folder{{
|
||||
@ -283,7 +296,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
})
|
||||
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alstale"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alstale"})
|
||||
Expect(out).To(Equal(outcomeFoundStale))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alstale", model.ImageTypePrimary)
|
||||
@ -300,7 +313,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alU", Name: "Album", FolderIDs: []string{"f1"}}})
|
||||
folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}}
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alU"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alU"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alU", model.ImageTypePrimary)
|
||||
@ -320,7 +333,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alE", Name: "Album", FolderIDs: []string{"f1"}}})
|
||||
folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}}
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alE"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alE"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alE", model.ImageTypePrimary)
|
||||
@ -338,7 +351,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alX", Name: "Album"}})
|
||||
imageAgents(&fakeImageAgent{name: "deezerFake", imgs: []agents.ExternalImage{{URL: srv.URL, Size: 500}}})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alX"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alX"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alX", model.ImageTypePrimary)
|
||||
@ -357,7 +370,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alext", Name: "Album"}})
|
||||
imageAgents(&fakeImageAgent{name: "deezerFake", imgs: []agents.ExternalImage{{URL: srv.URL, Size: 500}}})
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alext"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alext"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alext", model.ImageTypePrimary)
|
||||
@ -382,7 +395,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
{ID: "al6", Name: "Album B", FolderIDs: []string{"f1"}},
|
||||
})
|
||||
|
||||
out1, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
|
||||
out1, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
|
||||
Expect(out1).To(Equal(outcomeFound))
|
||||
ia1, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al5", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@ -392,7 +405,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
poisoned.BlurHash = "SENTINEL"
|
||||
artRepo.Data[ia1.Hash] = poisoned
|
||||
|
||||
out2, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"})
|
||||
out2, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"})
|
||||
Expect(out2).To(Equal(outcomeFound))
|
||||
ia2, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al6", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@ -422,7 +435,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
})
|
||||
|
||||
folderRepo.result = []model.Folder{{Path: "album-a", ImageFiles: []string{"cover.jpg"}}}
|
||||
outN, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alA"})
|
||||
outN, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alA"})
|
||||
Expect(outN).To(Equal(outcomeFound))
|
||||
iaA, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alA", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@ -436,7 +449,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
artRepo.Data[iaA.Hash] = poisoned
|
||||
|
||||
folderRepo.result = []model.Folder{{Path: "album-b", ImageFiles: []string{"cover.jpg"}}}
|
||||
outN, _ = proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alB"})
|
||||
outN, _, _ = proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alB"})
|
||||
Expect(outN).To(Equal(outcomeFound))
|
||||
iaB, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alB", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@ -467,7 +480,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
radioRepo.Data = map[string]*model.Radio{"ra1": {ID: "ra1", Name: "Radio", UploadedImage: "ra1_test.jpg"}}
|
||||
ds.MockedRadio = radioRepo
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
|
||||
@ -488,7 +501,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
radioRepo.Data = map[string]*model.Radio{"big": {ID: "big", Name: "Radio", UploadedImage: "big_test.jpg"}}
|
||||
ds.MockedRadio = radioRepo
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "big"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "big"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err = artRepo.GetItemArtwork(model.KindRadioArtwork, "big", model.ImageTypePrimary)
|
||||
@ -554,7 +567,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(artRepo.PutImage(&model.Artwork{Hash: hash, Mime: "application/octet-stream"})).To(Succeed())
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alM"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alM"})
|
||||
Expect(out).To(Equal(outcomeFound))
|
||||
|
||||
upgraded, err := artRepo.GetImage(hash)
|
||||
@ -574,7 +587,7 @@ var _ = Describe("processor.acquire", func() {
|
||||
Expect(os.WriteFile(blockedRoot, []byte("x"), 0600)).To(Succeed())
|
||||
proc.store = NewImageStore(blockedRoot)
|
||||
|
||||
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"})
|
||||
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"})
|
||||
Expect(out).To(Equal(outcomeFailed))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al7", model.ImageTypePrimary)
|
||||
|
||||
@ -25,9 +25,9 @@ type resolution struct {
|
||||
source string // model.ItemArtwork.Source value: "folder", "embedded", "external", "upload", "generated"
|
||||
sourcePath string // backing library/upload file (folder/upload: the image; embedded: the audio file); "" otherwise
|
||||
refMtime int64 // sourcePath mtime (unix-nanoseconds) at resolution; 0 when no sourcePath
|
||||
// external source errored/timed out. With no reader it forces failed (never absent);
|
||||
// on a hit a higher-priority external step failed—serve this, but retry later.
|
||||
extError bool
|
||||
// a faulted external source, carrying the provider's requested delay when it named one.
|
||||
// With no reader it forces failed (never absent); on a hit, serve this but retry later.
|
||||
extErr error
|
||||
// a local source that should have been readable wasn't. With no reader it forces failed,
|
||||
// so a transient I/O fault never records absent.
|
||||
localError bool
|
||||
@ -36,14 +36,15 @@ type resolution struct {
|
||||
// chainState carries what a priority walk has seen so far. A hit takes extErr with it so a
|
||||
// transient external failure still retries; localErr is dropped, as the scanner re-lists changes.
|
||||
type chainState struct {
|
||||
extErr, localErr bool
|
||||
trace *ChainTrace // nil only where no caller attached one
|
||||
extErr error
|
||||
localErr bool
|
||||
trace *ChainTrace // nil only where no caller attached one
|
||||
}
|
||||
|
||||
// try stamps the accumulated external failure onto a hit, and records the miss otherwise.
|
||||
func (c *chainState) try(candidate string, res resolution, ok bool) (resolution, bool) {
|
||||
if ok {
|
||||
res.extError = c.extErr
|
||||
res.extErr = c.extErr
|
||||
c.record(candidate, OutcomeHit, res.sourcePath)
|
||||
return res, true
|
||||
}
|
||||
@ -62,7 +63,7 @@ func (c *chainState) record(candidate string, out Outcome, detail string) {
|
||||
|
||||
// exhausted is the outcome when no source in the chain yielded an image.
|
||||
func (c *chainState) exhausted() resolution {
|
||||
return resolution{extError: c.extErr, localError: c.localErr}
|
||||
return resolution{extErr: c.extErr, localError: c.localErr}
|
||||
}
|
||||
|
||||
// externalSource holds the agents to ask and the rate limiter/circuit breaker to ask them through.
|
||||
@ -181,16 +182,16 @@ func chainFetchesExternal(priority string) bool {
|
||||
|
||||
// Album and artist fetches stop here when the resolver is local-only, rather than at each point in
|
||||
// the chain walk; resolvePlaylist gates the third network path, the m3u image URL, itself.
|
||||
func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, bool) {
|
||||
func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, error) {
|
||||
if r.ext == nil {
|
||||
return nil, "", false
|
||||
return nil, "", nil
|
||||
}
|
||||
return fetchAlbumImage(ctx, r.ext.agents, r.ext.gate, al)
|
||||
}
|
||||
|
||||
func (r *resolver) fetchExternalArtist(ctx context.Context, ar model.Artist) (io.ReadCloser, string, bool) {
|
||||
func (r *resolver) fetchExternalArtist(ctx context.Context, ar model.Artist) (io.ReadCloser, string, error) {
|
||||
if r.ext == nil {
|
||||
return nil, "", false
|
||||
return nil, "", nil
|
||||
}
|
||||
return fetchArtistImage(ctx, r.ext.agents, r.ext.gate, ar)
|
||||
}
|
||||
@ -223,10 +224,10 @@ func (r *resolver) resolveAlbum(ctx context.Context, albumID string) (resolution
|
||||
return res, nil
|
||||
}
|
||||
case pattern == externalCandidate:
|
||||
if rd, name, isErr := r.fetchExternalAlbum(ctx, *al); rd != nil {
|
||||
if rd, name, err := r.fetchExternalAlbum(ctx, *al); rd != nil {
|
||||
return resolution{reader: rd, source: ExternalPrefix + name}, nil
|
||||
} else if isErr {
|
||||
chain.extErr = true
|
||||
} else if err != nil {
|
||||
chain.extErr = longerRetry(chain.extErr, err)
|
||||
}
|
||||
case len(imgFiles) > 0:
|
||||
res, ok := resolveFolderFile(ctx, lib, imgFiles, pattern)
|
||||
@ -285,10 +286,10 @@ func (r *resolver) resolveArtist(ctx context.Context, artistID string) (resoluti
|
||||
}
|
||||
switch {
|
||||
case pattern == externalCandidate:
|
||||
if rd, name, isErr := r.fetchExternalArtist(ctx, *ar); rd != nil {
|
||||
if rd, name, err := r.fetchExternalArtist(ctx, *ar); rd != nil {
|
||||
return resolution{reader: rd, source: ExternalPrefix + name}, nil
|
||||
} else if isErr {
|
||||
chain.extErr = true
|
||||
} else if err != nil {
|
||||
chain.extErr = longerRetry(chain.extErr, err)
|
||||
}
|
||||
case pattern == "image-folder":
|
||||
res, ok := resolveArtistImageFolder(ar)
|
||||
@ -332,7 +333,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
|
||||
return resolution{}, err
|
||||
}
|
||||
|
||||
var extErr bool
|
||||
var extErr error
|
||||
for _, src := range []struct{ path, source string }{
|
||||
{pl.UploadedImagePath(), "upload"},
|
||||
{findPlaylistSidecarPath(ctx, pl.Path), "folder"},
|
||||
@ -366,7 +367,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
|
||||
if res, ok, err := resolveExternalStep(r.ext.gate, "m3u", sf); ok {
|
||||
return res, nil
|
||||
} else if err != nil {
|
||||
extErr = true
|
||||
extErr = longerRetry(extErr, err)
|
||||
// Record it here with its detail: once album sampling adds its own steps, the processor's
|
||||
// empty-trace fallback no longer fires, and the error that forced the retry would be lost.
|
||||
traceFrom(ctx).add(TraceStep{Candidate: ExternalPrefix + "m3u", Outcome: OutcomeError, Detail: err.Error()})
|
||||
@ -389,8 +390,8 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
|
||||
}
|
||||
continue
|
||||
}
|
||||
if res.extError {
|
||||
extErr = true
|
||||
if res.extErr != nil {
|
||||
extErr = longerRetry(extErr, res.extErr)
|
||||
}
|
||||
if res.reader == nil {
|
||||
continue
|
||||
@ -409,7 +410,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
|
||||
if tileErr != nil {
|
||||
return resolution{}, fmt.Errorf("resolvePlaylist: sampled album art failed: %w", tileErr)
|
||||
}
|
||||
return resolution{extError: extErr}, nil
|
||||
return resolution{extErr: extErr}, nil
|
||||
}
|
||||
// Grow to 4 tiles by repeating what we have.
|
||||
switch len(tiles) {
|
||||
@ -420,9 +421,9 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
|
||||
}
|
||||
grid, err := assembleTiles(tiles)
|
||||
if err != nil {
|
||||
return resolution{extError: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolution error
|
||||
return resolution{extErr: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolution error
|
||||
}
|
||||
return resolution{reader: grid, source: "generated", extError: extErr}, nil
|
||||
return resolution{reader: grid, source: "generated", extErr: extErr}, nil
|
||||
}
|
||||
|
||||
// resolveRadio serves only an uploaded image; there is no fallback.
|
||||
|
||||
@ -100,7 +100,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.source).To(Equal("embedded"))
|
||||
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/test.mp3"))
|
||||
Expect(res.refMtime).To(BeNumerically(">", 0))
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("resolves absent when the track has no cover art", func() {
|
||||
@ -111,7 +111,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf2"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("resolves absent when media file cover art is disabled", func() {
|
||||
@ -154,7 +154,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.source).To(Equal("folder"))
|
||||
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/cover.jpg"))
|
||||
Expect(res.refMtime).To(BeNumerically(">", 0))
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("falls back to embedded art when no folder image matches", func() {
|
||||
@ -172,7 +172,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.refMtime).To(BeNumerically(">", 0))
|
||||
})
|
||||
|
||||
It("sets extError when the external source errors without being not-found", func() {
|
||||
It("sets extErr when the external source errors without being not-found", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
|
||||
{ID: "al3", Name: "Album"},
|
||||
@ -182,10 +182,10 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("does not set extError when the external source reports not-found", func() {
|
||||
It("does not set extErr when the external source reports not-found", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
|
||||
{ID: "al4", Name: "Album"},
|
||||
@ -195,10 +195,10 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("carries extError onto a fallback folder hit after a transient external failure", func() {
|
||||
It("carries extErr onto a fallback folder hit after a transient external failure", func() {
|
||||
conf.Server.CoverArtPriority = "external, cover.jpg"
|
||||
folderRepo.result = []model.Folder{{
|
||||
Path: "tests/fixtures/artist/an-album",
|
||||
@ -214,10 +214,10 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.reader).ToNot(BeNil())
|
||||
defer res.reader.Close()
|
||||
Expect(res.source).To(Equal("folder"))
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("does not carry extError onto a fallback folder hit after a definitive external not-found", func() {
|
||||
It("does not carry extErr onto a fallback folder hit after a definitive external not-found", func() {
|
||||
conf.Server.CoverArtPriority = "external, cover.jpg"
|
||||
folderRepo.result = []model.Folder{{
|
||||
Path: "tests/fixtures/artist/an-album",
|
||||
@ -233,7 +233,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.reader).ToNot(BeNil())
|
||||
defer res.reader.Close()
|
||||
Expect(res.source).To(Equal("folder"))
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("routes the external step through the injected gate, keyed by agent name", func() {
|
||||
@ -250,7 +250,7 @@ var _ = Describe("resolveItem", func() {
|
||||
|
||||
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
Expect(gatedNames).To(Equal([]string{"failAgent"}))
|
||||
})
|
||||
})
|
||||
@ -298,7 +298,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/artist.png"))
|
||||
})
|
||||
|
||||
It("sets extError when the external source errors without being not-found", func() {
|
||||
It("sets extErr when the external source errors without being not-found", func() {
|
||||
conf.Server.ArtistArtPriority = "external"
|
||||
artistRepo := tests.CreateMockArtistRepo()
|
||||
artistRepo.SetData(model.Artists{{ID: "ar3", Name: "Artist"}})
|
||||
@ -308,10 +308,10 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar3"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("does not set extError when the external source reports not-found", func() {
|
||||
It("does not set extErr when the external source reports not-found", func() {
|
||||
conf.Server.ArtistArtPriority = "external"
|
||||
artistRepo := tests.CreateMockArtistRepo()
|
||||
artistRepo.SetData(model.Artists{{ID: "ar4", Name: "Artist"}})
|
||||
@ -321,7 +321,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar4"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("routes the external step through the injected gate, keyed by agent name", func() {
|
||||
@ -338,7 +338,7 @@ var _ = Describe("resolveItem", func() {
|
||||
|
||||
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
Expect(gatedNames).To(Equal([]string{"failAgent"}))
|
||||
})
|
||||
})
|
||||
@ -516,7 +516,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "ple"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
Expect(gatedNames).To(Equal([]string{"m3u"}), "the playlist URL fetch is gated under \"m3u\"")
|
||||
})
|
||||
|
||||
@ -537,7 +537,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, gate).resolve(withTrace(ctx, trace),
|
||||
model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm3u"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
|
||||
steps := trace.Steps()
|
||||
var m3u *TraceStep
|
||||
@ -562,7 +562,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("treats an ExternalImageURL 404 as a definitive miss and falls through to the grid", func() {
|
||||
@ -582,7 +582,7 @@ var _ = Describe("resolveItem", func() {
|
||||
Expect(res.reader).ToNot(BeNil())
|
||||
defer res.reader.Close()
|
||||
Expect(res.source).To(Equal("generated"))
|
||||
Expect(res.extError).To(BeFalse())
|
||||
Expect(res.extErr).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
// A local resolver holds no agents: reaching the external branch would panic, not degrade.
|
||||
@ -594,7 +594,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newLocalResolver(ds, ffm).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alx"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeFalse(), "a skipped step is not a failed one")
|
||||
Expect(res.extErr).ToNot(HaveOccurred(), "a skipped step is not a failed one")
|
||||
})
|
||||
|
||||
// The worker resolving the same playlist is asserted alongside, so this cannot pass vacuously.
|
||||
@ -642,7 +642,7 @@ var _ = Describe("resolveItem", func() {
|
||||
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl500"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res.reader).To(BeNil())
|
||||
Expect(res.extError).To(BeTrue())
|
||||
Expect(res.extErr).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("yields an empty resolution when no album has art", func() {
|
||||
|
||||
@ -241,7 +241,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
|
||||
item.ImageType = cmp.Or(item.ImageType, model.ImageTypePrimary)
|
||||
trace := &ChainTrace{}
|
||||
ctx = withTrace(ctx, trace)
|
||||
out, got := w.proc.acquire(ctx, item)
|
||||
out, got, retryIn := w.proc.acquire(ctx, item)
|
||||
|
||||
queue := w.proc.ds.ArtworkQueue(ctx)
|
||||
switch out {
|
||||
@ -252,7 +252,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
|
||||
log.Warn(ctx, "Artwork: Could not delete processed queue item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
}
|
||||
case outcomeFoundStale, outcomeFailed:
|
||||
retryAt := time.Now().Add(backoff(item.Attempts))
|
||||
retryAt := time.Now().Add(retryDelay(item.Attempts, retryIn))
|
||||
encoded := trace.encode("")
|
||||
if retryAt.Before(item.EnqueuedAt.Add(giveUpAfter)) {
|
||||
// A mid-flight re-enqueue reset retry_at; stale backoff must not stomp its
|
||||
@ -341,3 +341,8 @@ func backoffFor(attempts int, jitter float64) time.Duration {
|
||||
func backoff(attempts int) time.Duration {
|
||||
return backoffFor(attempts, rand.Float64()*0.8-0.4) //nolint:gosec // retry jitter, not security-sensitive
|
||||
}
|
||||
|
||||
// retryDelay is how long a failed item waits: our backoff, unless the provider asked for longer.
|
||||
func retryDelay(attempts int, hint time.Duration) time.Duration {
|
||||
return max(backoff(attempts), hint)
|
||||
}
|
||||
|
||||
@ -95,7 +95,7 @@ var _ = Describe("Worker soak", func() {
|
||||
start := time.Now()
|
||||
for i := range soakCycles {
|
||||
it := items[i%len(items)]
|
||||
out, _ := proc.acquire(context.Background(), it)
|
||||
out, _, _ := proc.acquire(context.Background(), it)
|
||||
|
||||
// Read-back exercises the surfaces a caller would use after acquisition.
|
||||
if out == outcomeFound {
|
||||
|
||||
@ -245,6 +245,23 @@ var _ = Describe("Worker", func() {
|
||||
Expect(err).To(MatchError(model.ErrNotFound), "a timeout must never settle on absent")
|
||||
})
|
||||
|
||||
It("reschedules past the provider's requested delay when it exceeds the backoff", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al9", Name: "Album"}})
|
||||
// Well above backoff(0)'s jittered ceiling, so only the hint can produce this retry_at.
|
||||
const askedFor = 90 * time.Minute
|
||||
imageAgents(&fakeImageAgent{name: "throttledAgent", err: &agents.RetryLaterError{RetryIn: askedFor}})
|
||||
Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al9"})).To(Succeed())
|
||||
|
||||
n, err := w.drain(ctx, 2)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(n).To(Equal(1))
|
||||
|
||||
it := findQueued(queueRepo, "al", "al9")
|
||||
Expect(it).ToNot(BeNil())
|
||||
Expect(it.RetryAt).To(BeTemporally("~", time.Now().Add(askedFor), time.Minute))
|
||||
})
|
||||
|
||||
It("reschedules a found-stale item via MarkFailed while keeping its served state", func() {
|
||||
conf.Server.CoverArtPriority = "external, cover.jpg"
|
||||
folderRepo.result = []model.Folder{{
|
||||
@ -911,3 +928,19 @@ var _ = Describe("backoff", func() {
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("retryDelay", func() {
|
||||
It("uses the backoff schedule when the provider asked for nothing", func() {
|
||||
d := retryDelay(0, 0)
|
||||
Expect(d).To(BeNumerically(">=", 3*time.Second))
|
||||
Expect(d).To(BeNumerically("<=", 7*time.Second))
|
||||
})
|
||||
|
||||
It("waits the provider's delay when it is longer than the backoff", func() {
|
||||
Expect(retryDelay(0, time.Hour)).To(Equal(time.Hour))
|
||||
})
|
||||
|
||||
It("keeps the backoff when it is longer than the provider's delay", func() {
|
||||
Expect(retryDelay(4, time.Second)).To(BeNumerically(">=", 3*time.Second))
|
||||
})
|
||||
})
|
||||
|
||||
63
core/external/provider.go
vendored
63
core/external/provider.go
vendored
@ -148,7 +148,8 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
|
||||
start := time.Now()
|
||||
albumName := album.Name()
|
||||
info, err := e.ag.GetAlbumInfo(ctx, albumName, album.AlbumArtist, album.MbzAlbumID)
|
||||
if errors.Is(err, agents.ErrNotFound) {
|
||||
// Throttled joins not-found: no answer to store, and an unstamped timestamp retries next call.
|
||||
if errors.Is(err, agents.ErrNotFound) || errors.Is(err, agents.ErrRetryLater) {
|
||||
return album, nil
|
||||
}
|
||||
if err != nil {
|
||||
@ -253,28 +254,37 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au
|
||||
start := time.Now()
|
||||
// Get MBID first, if it is not yet available
|
||||
artistName := artist.Name()
|
||||
var mbidErr error
|
||||
if artist.MbzArtistID == "" {
|
||||
mbid, err := e.ag.GetArtistMBID(ctx, artist.ID, artistName)
|
||||
mbidErr = err
|
||||
if mbid != "" && err == nil {
|
||||
artist.MbzArtistID = mbid
|
||||
}
|
||||
}
|
||||
|
||||
// Call all registered agents and collect information
|
||||
// Call all registered agents and collect information. The group carries no context, so a
|
||||
// returned error does not cancel the siblings; only throttling is reported back.
|
||||
g := errgroup.Group{}
|
||||
g.SetLimit(2)
|
||||
g.Go(func() error { _ = e.callGetImage(ctx, e.ag, &artist); return nil })
|
||||
g.Go(func() error { e.callGetBiography(ctx, e.ag, &artist); return nil })
|
||||
g.Go(func() error { e.callGetURL(ctx, e.ag, &artist); return nil })
|
||||
g.Go(func() error { e.callGetSimilarArtists(ctx, e.ag, &artist, maxSimilarArtists, true); return nil })
|
||||
_ = g.Wait()
|
||||
g.Go(func() error { return retryLaterOnly(e.callGetImage(ctx, e.ag, &artist)) })
|
||||
g.Go(func() error { return retryLaterOnly(e.callGetBiography(ctx, e.ag, &artist)) })
|
||||
g.Go(func() error { return retryLaterOnly(e.callGetURL(ctx, e.ag, &artist)) })
|
||||
g.Go(func() error {
|
||||
return retryLaterOnly(e.callGetSimilarArtists(ctx, e.ag, &artist, maxSimilarArtists, true))
|
||||
})
|
||||
throttled := errors.Is(g.Wait(), agents.ErrRetryLater) || errors.Is(mbidErr, agents.ErrRetryLater)
|
||||
|
||||
if utils.IsCtxDone(ctx) {
|
||||
log.Warn(ctx, "ArtistInfo update canceled", "id", artist.ID, "name", artistName, "elapsed", time.Since(start), ctx.Err())
|
||||
return artist, ctx.Err()
|
||||
}
|
||||
|
||||
artist.ExternalInfoUpdatedAt = new(time.Now())
|
||||
// A throttled round keeps the previous timestamp, so the next call retries instead of
|
||||
// serving an empty cache entry for the whole TTL.
|
||||
if !throttled {
|
||||
artist.ExternalInfoUpdatedAt = new(time.Now())
|
||||
}
|
||||
err := e.ds.Artist(ctx).UpdateExternalInfo(&artist.Artist)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error trying to update artist external information", "id", artist.ID, "name", artistName,
|
||||
@ -334,8 +344,9 @@ func (e *provider) TopSongs(ctx context.Context, artistName, id string, count in
|
||||
songs, err := e.getMatchingTopSongs(ctx, e.ag, artist, count)
|
||||
if err != nil {
|
||||
switch {
|
||||
case errors.Is(err, agents.ErrNotFound):
|
||||
log.Trace(ctx, "TopSongs not found", "name", artistName)
|
||||
// Throttled is not an answer, but the caller keeps the empty 200 it got before.
|
||||
case errors.Is(err, agents.ErrNotFound), errors.Is(err, agents.ErrRetryLater):
|
||||
log.Trace(ctx, "TopSongs not found", "name", artistName, err)
|
||||
return nil, model.ErrNotFound
|
||||
case errors.Is(err, context.Canceled):
|
||||
log.Debug(ctx, "TopSongs call canceled", err)
|
||||
@ -385,22 +396,33 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT
|
||||
return mfs, nil
|
||||
}
|
||||
|
||||
func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) {
|
||||
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return
|
||||
// retryLaterOnly discards every failure the caller does not act on, so errgroup's
|
||||
// first-error slot is reserved for the throttling signal.
|
||||
func retryLaterOnly(err error) error {
|
||||
if errors.Is(err, agents.ErrRetryLater) {
|
||||
return err
|
||||
}
|
||||
artist.ExternalUrl = artisURL
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) {
|
||||
func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) error {
|
||||
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
artist.ExternalUrl = artisURL
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) error {
|
||||
bio, err := agent.GetArtistBiography(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return
|
||||
return err
|
||||
}
|
||||
bio = str.SanitizeText(bio)
|
||||
bio = strings.ReplaceAll(bio, "\n", " ")
|
||||
artist.Biography = strings.ReplaceAll(bio, "<a ", "<a target='_blank' ")
|
||||
return nil
|
||||
}
|
||||
|
||||
// callGetImage populates artist's image URLs. A transient agent failure is
|
||||
@ -428,19 +450,20 @@ func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRet
|
||||
}
|
||||
|
||||
func (e *provider) callGetSimilarArtists(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist,
|
||||
limit int, includeNotPresent bool) {
|
||||
limit int, includeNotPresent bool) error {
|
||||
artistName := artist.Name()
|
||||
similar, err := agent.GetSimilarArtists(ctx, artist.ID, artistName, artist.MbzArtistID, limit)
|
||||
if len(similar) == 0 || err != nil {
|
||||
return
|
||||
return err
|
||||
}
|
||||
start := time.Now()
|
||||
sa, err := e.mapSimilarArtists(ctx, similar, limit, includeNotPresent)
|
||||
log.Debug(ctx, "Mapped Similar Artists", "artist", artistName, "numSimilar", len(sa), "elapsed", time.Since(start))
|
||||
if err != nil {
|
||||
return
|
||||
return err
|
||||
}
|
||||
artist.SimilarArtists = sa
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *provider) mapSimilarArtists(ctx context.Context, similar []agents.Artist, limit int, includeNotPresent bool) (model.Artists, error) {
|
||||
|
||||
2
core/external/provider_similarsongs.go
vendored
2
core/external/provider_similarsongs.go
vendored
@ -240,7 +240,7 @@ func (e *provider) similarSongsFallback(ctx context.Context, id string, count in
|
||||
return nil, err
|
||||
}
|
||||
|
||||
e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false)
|
||||
_ = e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false)
|
||||
if utils.IsCtxDone(ctx) {
|
||||
log.Warn(ctx, "SimilarSongs call canceled", ctx.Err())
|
||||
return nil, ctx.Err()
|
||||
|
||||
15
core/external/provider_topsongs_test.go
vendored
15
core/external/provider_topsongs_test.go
vendored
@ -232,6 +232,21 @@ var _ = Describe("Provider - TopSongs", func() {
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
// This endpoint answered with an empty list before retry-later existed; it must keep doing so.
|
||||
It("returns an empty list, not a client error, when the agents are throttled", func() {
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).
|
||||
Return(nil, agents.ErrRetryLater).Once()
|
||||
|
||||
songs, err := p.TopSongs(ctx, "Artist One", "", 5)
|
||||
|
||||
Expect(songs).To(BeEmpty())
|
||||
Expect(err).To(MatchError(model.ErrNotFound), "the handler renders this as an empty 200")
|
||||
Expect(err).ToNot(MatchError(agents.ErrRetryLater))
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
It("returns fewer songs if count is less than available top songs", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
|
||||
22
core/external/provider_updatealbuminfo_test.go
vendored
22
core/external/provider_updatealbuminfo_test.go
vendored
@ -164,4 +164,26 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() {
|
||||
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
It("returns the original album, unstamped, when the agents are throttled", func() {
|
||||
originalAlbum := &model.Album{
|
||||
ID: "al-throttled",
|
||||
Name: "Throttled Album",
|
||||
AlbumArtist: "Throttled Artist",
|
||||
MbzAlbumID: "mbid-throttled",
|
||||
}
|
||||
mockAlbumRepo.SetData(model.Albums{*originalAlbum})
|
||||
|
||||
ag.On("GetAlbumInfo", ctx, "Throttled Album", "Throttled Artist", "mbid-throttled").
|
||||
Return(nil, agents.ErrRetryLater)
|
||||
|
||||
updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-throttled")
|
||||
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(updatedAlbum).NotTo(BeNil())
|
||||
Expect(*updatedAlbum).To(Equal(*originalAlbum))
|
||||
Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil())
|
||||
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
})
|
||||
|
||||
19
core/external/provider_updateartistinfo_test.go
vendored
19
core/external/provider_updateartistinfo_test.go
vendored
@ -104,6 +104,25 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
// Stamping a throttled round would cache the empty result for the whole TTL.
|
||||
It("does not stamp ExternalInfoUpdatedAt when the agents are throttled", func() {
|
||||
originalArtist := &model.Artist{ID: "ar-throttled", Name: "Throttled Artist"}
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist})
|
||||
|
||||
ag.On("GetArtistMBID", ctx, "ar-throttled", "Throttled Artist").Return("", agents.ErrRetryLater).Once()
|
||||
ag.On("GetArtistImages", ctx, "ar-throttled", "Throttled Artist", "").Return(nil, agents.ErrRetryLater).Once()
|
||||
ag.On("GetArtistBiography", ctx, "ar-throttled", "Throttled Artist", "").Return("", agents.ErrRetryLater).Once()
|
||||
ag.On("GetArtistURL", ctx, "ar-throttled", "Throttled Artist", "").Return("", agents.ErrRetryLater).Once()
|
||||
ag.On("GetSimilarArtists", ctx, "ar-throttled", "Throttled Artist", "", 100).Return(nil, agents.ErrRetryLater).Once()
|
||||
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-throttled", 10, false)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(updatedArtist).NotTo(BeNil())
|
||||
Expect(updatedArtist.ExternalInfoUpdatedAt).To(BeNil())
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
It("preserves decoded plain text in biography storage", func() {
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-encoded-bio",
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
@ -126,42 +127,52 @@ func (b *bufferedScrobbler) run(ctx context.Context) {
|
||||
timer.Stop()
|
||||
defer timer.Stop()
|
||||
failures := 0
|
||||
backingOff := false
|
||||
for {
|
||||
if b.processQueue(ctx) {
|
||||
failures = 0
|
||||
timer.Stop()
|
||||
} else {
|
||||
timer.Reset(backoffDelay(failures))
|
||||
if failures < maxRetryShift {
|
||||
failures++
|
||||
// While a backoff window is open the timer is already armed for the rest of it, so a
|
||||
// wake (a new play enqueued) must not drain: that is the hammering this avoids.
|
||||
if !backingOff {
|
||||
if ok, retryIn := b.processQueue(ctx); ok {
|
||||
failures = 0
|
||||
timer.Stop()
|
||||
} else {
|
||||
timer.Reset(max(backoffDelay(failures), retryIn))
|
||||
backingOff = true
|
||||
if failures < maxRetryShift {
|
||||
failures++
|
||||
}
|
||||
}
|
||||
}
|
||||
select {
|
||||
case <-b.wakeSignal:
|
||||
case <-timer.C:
|
||||
backingOff = false
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (b *bufferedScrobbler) processQueue(ctx context.Context) bool {
|
||||
func (b *bufferedScrobbler) processQueue(ctx context.Context) (bool, time.Duration) {
|
||||
buffer := b.ds.ScrobbleBuffer(ctx)
|
||||
userIds, err := buffer.UserIDs(b.service)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error retrieving userIds from scrobble buffer", "scrobbler", b.service, err)
|
||||
return false
|
||||
return false, 0
|
||||
}
|
||||
result := true
|
||||
var retryIn time.Duration
|
||||
for _, userId := range userIds {
|
||||
if !b.processUserQueue(ctx, userId) {
|
||||
ok, d := b.processUserQueue(ctx, userId)
|
||||
if !ok {
|
||||
result = false
|
||||
retryIn = max(retryIn, d)
|
||||
}
|
||||
}
|
||||
return result
|
||||
return result, retryIn
|
||||
}
|
||||
|
||||
func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string) bool {
|
||||
func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string) (bool, time.Duration) {
|
||||
// Scrobbles are drained on a background context that no longer carries the
|
||||
// request's authenticated user. Restore it from the buffered userId so that
|
||||
// scrobblers relying on the user in the context (e.g. plugins) still get it.
|
||||
@ -175,25 +186,25 @@ func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string)
|
||||
entry, err := buffer.Next(b.service, userId)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error reading from scrobble buffer", "scrobbler", b.service, err)
|
||||
return false
|
||||
return false, 0
|
||||
}
|
||||
if entry == nil {
|
||||
return true
|
||||
return true, 0
|
||||
}
|
||||
s, ok := b.loader()
|
||||
if !ok {
|
||||
log.Warn(ctx, "Scrobbler not available, will retry later", "scrobbler", b.service)
|
||||
return false
|
||||
return false, 0
|
||||
}
|
||||
log.Debug(ctx, "Sending scrobble", "scrobbler", b.service, "track", entry.Title, "artist", entry.Artist)
|
||||
err = s.Scrobble(ctx, entry.UserID, Scrobble{
|
||||
MediaFile: entry.MediaFile,
|
||||
TimeStamp: entry.PlayTime,
|
||||
})
|
||||
if errors.Is(err, ErrRetryLater) {
|
||||
if retry, ok := errors.AsType[*agents.RetryLaterError](err); ok {
|
||||
log.Warn(ctx, "Could not send scrobble. Will be retried", "userId", entry.UserID,
|
||||
"track", entry.Title, "artist", entry.Artist, "scrobbler", b.service, err)
|
||||
return false
|
||||
return false, retry.RetryIn
|
||||
}
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error sending scrobble to service. Discarding", "scrobbler", b.service,
|
||||
@ -203,7 +214,7 @@ func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error removing entry from scrobble buffer", "userId", entry.UserID,
|
||||
"track", entry.Title, "artist", entry.Artist, "scrobbler", b.service, err)
|
||||
return false
|
||||
return false, 0
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -2,11 +2,13 @@ package scrobbler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
@ -158,19 +160,128 @@ func TestBufferedScrobblerBackoffSchedule(t *testing.T) {
|
||||
g.Expect(flaky.count.Load()).To(Equal(want), "retry did not fire after the %s backoff", gap)
|
||||
}
|
||||
|
||||
// Once the service recovers, waking the loop drains the buffered entry.
|
||||
// Once the service recovers, the buffered entry drains when the open
|
||||
// backoff window closes (a wake alone must not drain it early).
|
||||
flaky.succeed()
|
||||
bs.sendWakeSignal()
|
||||
synctest.Wait()
|
||||
g.Expect(buffer.Length()).To(Equal(int64(1)), "wake during backoff drained early")
|
||||
time.Sleep(80 * time.Second)
|
||||
synctest.Wait()
|
||||
g.Expect(buffer.Length()).To(Equal(int64(0)))
|
||||
})
|
||||
}
|
||||
|
||||
func TestBufferedScrobblerBackoffWindow(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
buffer := tests.CreateMockedScrobbleBufferRepo()
|
||||
userRepo := tests.CreateMockUserRepo()
|
||||
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
|
||||
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
|
||||
scr := &fakeScrobbler{Authorized: true}
|
||||
scr.SetError(errors.Join(errors.New("boom"), ErrRetryLater))
|
||||
bs := newBufferedScrobbler(ds, scr, "test")
|
||||
defer bs.Stop()
|
||||
|
||||
// First enqueue: one immediate attempt, then a 5s window opens.
|
||||
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "1"}, TimeStamp: time.Now()})
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 1 {
|
||||
t.Fatalf("expected 1 attempt after first enqueue, got %d", got)
|
||||
}
|
||||
|
||||
// A wake inside the window must NOT trigger an early attempt.
|
||||
time.Sleep(1 * time.Second)
|
||||
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "2"}, TimeStamp: time.Now()})
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 1 {
|
||||
t.Fatalf("wake during backoff drained early: %d attempts", got)
|
||||
}
|
||||
|
||||
// When the 5s window closes, the retry happens.
|
||||
time.Sleep(4100 * time.Millisecond)
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 2 {
|
||||
t.Fatalf("expected retry after window, got %d attempts", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestBufferedScrobblerHonorsServerDelay(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
buffer := tests.CreateMockedScrobbleBufferRepo()
|
||||
userRepo := tests.CreateMockUserRepo()
|
||||
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
|
||||
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
|
||||
scr := &fakeScrobbler{Authorized: true}
|
||||
scr.SetError(errors.Join(errors.New("429"), &agents.RetryLaterError{RetryIn: 30 * time.Second}))
|
||||
bs := newBufferedScrobbler(ds, scr, "test")
|
||||
defer bs.Stop()
|
||||
|
||||
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "1"}, TimeStamp: time.Now()})
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 1 {
|
||||
t.Fatalf("expected 1 attempt, got %d", got)
|
||||
}
|
||||
|
||||
// The 5s exponential floor is overridden by the 30s server delay.
|
||||
time.Sleep(20 * time.Second)
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 1 {
|
||||
t.Fatalf("retried before server delay elapsed: %d attempts", got)
|
||||
}
|
||||
time.Sleep(10100 * time.Millisecond)
|
||||
synctest.Wait()
|
||||
if got := scr.ScrobbleAttempts(); got != 2 {
|
||||
t.Fatalf("expected retry after server delay, got %d attempts", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The drain visits users in an arbitrary order, so the longest delay must win regardless
|
||||
// of which user was seen last.
|
||||
func TestBufferedScrobblerTakesTheLongestServerDelayAcrossUsers(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
buffer := tests.CreateMockedScrobbleBufferRepo()
|
||||
userRepo := tests.CreateMockUserRepo()
|
||||
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
|
||||
_ = userRepo.Put(&model.User{ID: "user2", UserName: "bob"})
|
||||
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
|
||||
scr := &recoveringScrobbler{delays: map[string]time.Duration{
|
||||
"user1": 10 * time.Second,
|
||||
"user2": 45 * time.Second,
|
||||
}}
|
||||
bs := newBufferedScrobbler(ds, scr, "test")
|
||||
defer bs.Stop()
|
||||
|
||||
// user2 is enqueued directly so both are buffered before the first drain wakes.
|
||||
_ = buffer.Enqueue("test", "user2", "2", time.Now())
|
||||
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "1"}, TimeStamp: time.Now()})
|
||||
synctest.Wait()
|
||||
if got := scr.count.Load(); got != 2 {
|
||||
t.Fatalf("expected both users drained, got %d attempts", got)
|
||||
}
|
||||
|
||||
time.Sleep(30 * time.Second)
|
||||
synctest.Wait()
|
||||
if got := scr.count.Load(); got != 2 {
|
||||
t.Fatalf("retried on the shorter delay: %d attempts", got)
|
||||
}
|
||||
time.Sleep(15100 * time.Millisecond)
|
||||
synctest.Wait()
|
||||
if got := scr.count.Load(); got != 4 {
|
||||
t.Fatalf("expected a retry after the longest delay, got %d attempts", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// recoveringScrobbler is a race-safe Scrobbler whose error can be toggled while
|
||||
// the buffered scrobbler's goroutine is draining, to exercise retry then recovery.
|
||||
// With delays set, it instead fails every scrobble asking for that user's delay.
|
||||
type recoveringScrobbler struct {
|
||||
err atomic.Pointer[error]
|
||||
count atomic.Int32
|
||||
err atomic.Pointer[error]
|
||||
count atomic.Int32
|
||||
delays map[string]time.Duration
|
||||
}
|
||||
|
||||
func (f *recoveringScrobbler) fail(err error) { f.err.Store(&err) }
|
||||
@ -182,8 +293,11 @@ func (f *recoveringScrobbler) NowPlaying(context.Context, string, *model.MediaFi
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *recoveringScrobbler) Scrobble(_ context.Context, _ string, _ Scrobble) error {
|
||||
func (f *recoveringScrobbler) Scrobble(_ context.Context, userId string, _ Scrobble) error {
|
||||
f.count.Add(1)
|
||||
if f.delays != nil {
|
||||
return errors.Join(errors.New("429"), &agents.RetryLaterError{RetryIn: f.delays[userId]})
|
||||
}
|
||||
if e := f.err.Load(); e != nil {
|
||||
return *e
|
||||
}
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
)
|
||||
|
||||
@ -15,7 +16,8 @@ type Scrobble struct {
|
||||
|
||||
var (
|
||||
ErrNotAuthorized = errors.New("not authorized")
|
||||
ErrRetryLater = errors.New("retry later")
|
||||
// ErrRetryLater is an alias of agents.ErrRetryLater so adapters and plugins share one identity.
|
||||
ErrRetryLater = agents.ErrRetryLater
|
||||
ErrUnrecoverable = errors.New("unrecoverable")
|
||||
)
|
||||
|
||||
|
||||
@ -293,7 +293,7 @@ var _ = Describe("PlayTracker", func() {
|
||||
})
|
||||
|
||||
It("increments play counts even if it cannot scrobble", func() {
|
||||
fake.Error = errors.New("error")
|
||||
fake.SetError(errors.New("error"))
|
||||
|
||||
err := tracker.Submit(ctx, []Submission{{TrackID: "123", Timestamp: time.Now()}})
|
||||
|
||||
@ -1414,7 +1414,25 @@ type fakeScrobbler struct {
|
||||
position atomic.Int32
|
||||
LastScrobble atomic.Pointer[Scrobble]
|
||||
LastPlaybackReport atomic.Pointer[PlaybackSession]
|
||||
Error error
|
||||
err atomic.Pointer[error]
|
||||
scrobbleAttempts atomic.Int32
|
||||
}
|
||||
|
||||
// SetError sets the error returned by IsAuthorized/NowPlaying/Scrobble/PlaybackReport.
|
||||
func (f *fakeScrobbler) SetError(err error) {
|
||||
f.err.Store(&err)
|
||||
}
|
||||
|
||||
func (f *fakeScrobbler) getError() error {
|
||||
if e := f.err.Load(); e != nil {
|
||||
return *e
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ScrobbleAttempts returns how many times Scrobble was called.
|
||||
func (f *fakeScrobbler) ScrobbleAttempts() int32 {
|
||||
return f.scrobbleAttempts.Load()
|
||||
}
|
||||
|
||||
func (f *fakeScrobbler) GetNowPlayingCalled() bool {
|
||||
@ -1440,13 +1458,13 @@ func (f *fakeScrobbler) GetTrack() *model.MediaFile {
|
||||
}
|
||||
|
||||
func (f *fakeScrobbler) IsAuthorized(ctx context.Context, userId string) bool {
|
||||
return f.Error == nil && f.Authorized
|
||||
return f.getError() == nil && f.Authorized
|
||||
}
|
||||
|
||||
func (f *fakeScrobbler) NowPlaying(ctx context.Context, userId string, track *model.MediaFile, position int) error {
|
||||
f.nowPlayingCalled.Store(true)
|
||||
if f.Error != nil {
|
||||
return f.Error
|
||||
if err := f.getError(); err != nil {
|
||||
return err
|
||||
}
|
||||
f.userID.Store(&userId)
|
||||
// Capture username from context (this is what plugin scrobblers do)
|
||||
@ -1478,16 +1496,17 @@ func (f *fakeScrobbler) Scrobble(ctx context.Context, userId string, s Scrobble)
|
||||
}
|
||||
f.LastScrobble.Store(&s)
|
||||
f.ScrobbleCalled.Store(true)
|
||||
if f.Error != nil {
|
||||
return f.Error
|
||||
f.scrobbleAttempts.Add(1)
|
||||
if err := f.getError(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeScrobbler) PlaybackReport(ctx context.Context, info PlaybackSession) error {
|
||||
f.PlaybackReportCalled.Store(true)
|
||||
if f.Error != nil {
|
||||
return f.Error
|
||||
if err := f.getError(); err != nil {
|
||||
return err
|
||||
}
|
||||
f.userID.Store(new(info.UserId))
|
||||
f.LastPlaybackReport.Store(&info)
|
||||
|
||||
18
core/storage/local/deviceid_unix.go
Normal file
18
core/storage/local/deviceid_unix.go
Normal file
@ -0,0 +1,18 @@
|
||||
//go:build !windows
|
||||
|
||||
package local
|
||||
|
||||
import (
|
||||
"io/fs"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
// deviceID identifies the filesystem a file lives on, used to key birth time support per mount.
|
||||
// It is returned opaquely because its width varies by platform, and it is only used as a map key.
|
||||
func deviceID(fi fs.FileInfo) (any, bool) {
|
||||
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
return st.Dev, true
|
||||
}
|
||||
8
core/storage/local/deviceid_windows.go
Normal file
8
core/storage/local/deviceid_windows.go
Normal file
@ -0,0 +1,8 @@
|
||||
//go:build windows
|
||||
|
||||
package local
|
||||
|
||||
import "io/fs"
|
||||
|
||||
// deviceID has no Windows equivalent, and none is needed: birth time comes straight from FileInfo.
|
||||
func deviceID(fs.FileInfo) (any, bool) { return nil, false }
|
||||
@ -6,6 +6,7 @@ import (
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
@ -61,6 +62,8 @@ type localFS struct {
|
||||
fs.FS
|
||||
extractor Extractor
|
||||
root string
|
||||
// devices whose statx never reports a birth time (NFS, rclone/FUSE), so we ask each only once
|
||||
noBirthTime sync.Map
|
||||
}
|
||||
|
||||
// ResolveSymlink implements storage.SymlinkResolverFS. It resolves the whole chain at the
|
||||
@ -84,7 +87,11 @@ func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
v.FileInfo = localFileInfo{info}
|
||||
v.FileInfo = localFileInfo{
|
||||
FileInfo: info,
|
||||
path: filepath.Join(lfs.root, filepath.FromSlash(path)),
|
||||
noBirthTime: &lfs.noBirthTime,
|
||||
}
|
||||
res[path] = v
|
||||
}
|
||||
}
|
||||
@ -95,15 +102,46 @@ func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) {
|
||||
// with metadata.FileInfo
|
||||
type localFileInfo struct {
|
||||
fs.FileInfo
|
||||
path string
|
||||
noBirthTime *sync.Map
|
||||
}
|
||||
|
||||
func (lfi localFileInfo) BirthTime() time.Time {
|
||||
if ts := times.Get(lfi.FileInfo); ts.HasBirthTime() {
|
||||
return ts.BirthTime()
|
||||
}
|
||||
if bt, ok := lfi.statxBirthTime(); ok {
|
||||
return bt
|
||||
}
|
||||
return time.Now()
|
||||
}
|
||||
|
||||
// statxBirthTime reads the birth time from the path, which on Linux is the only way to get it.
|
||||
// Filesystems that never report one are remembered per device, so a scan asks each only once.
|
||||
func (lfi localFileInfo) statxBirthTime() (time.Time, bool) {
|
||||
if lfi.path == "" {
|
||||
return time.Time{}, false
|
||||
}
|
||||
dev, hasDev := deviceID(lfi.FileInfo)
|
||||
memo := lfi.noBirthTime
|
||||
if hasDev && memo != nil {
|
||||
if _, skip := memo.Load(dev); skip {
|
||||
return time.Time{}, false
|
||||
}
|
||||
}
|
||||
ts, err := times.Stat(lfi.path)
|
||||
if err != nil {
|
||||
return time.Time{}, false
|
||||
}
|
||||
if ts.HasBirthTime() {
|
||||
return ts.BirthTime(), true
|
||||
}
|
||||
if hasDev && memo != nil {
|
||||
memo.Store(dev, struct{}{})
|
||||
}
|
||||
return time.Time{}, false
|
||||
}
|
||||
|
||||
func init() {
|
||||
storage.Register(storage.LocalSchemaID, newLocalStorage)
|
||||
}
|
||||
|
||||
@ -6,8 +6,10 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/djherbis/times"
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
@ -440,6 +442,37 @@ var _ = Describe("LocalStorage", func() {
|
||||
// Should be around the current time (within last few minutes)
|
||||
Expect(birthTime).To(BeTemporally("~", time.Now(), 5*time.Minute))
|
||||
})
|
||||
|
||||
It("reads the birth time from the path, not the time of the call", func() {
|
||||
// On Linux, birth time is only available via statx(2) on the path.
|
||||
lfi := localFileInfo{FileInfo: fileInfo, path: testFile}
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
Expect(lfi.BirthTime()).To(BeTemporally("<", time.Now().Add(-200*time.Millisecond)))
|
||||
})
|
||||
|
||||
It("does not remember filesystems that do report a birth time", func() {
|
||||
memo := &sync.Map{}
|
||||
lfi := localFileInfo{FileInfo: fileInfo, path: testFile, noBirthTime: memo}
|
||||
lfi.BirthTime()
|
||||
|
||||
count := 0
|
||||
memo.Range(func(_, _ any) bool { count++; return true })
|
||||
Expect(count).To(BeZero())
|
||||
})
|
||||
|
||||
It("skips statx on filesystems already known to have none", func() {
|
||||
if times.Get(fileInfo).HasBirthTime() {
|
||||
Skip("this platform reports birth time from FileInfo, so statx is never called")
|
||||
}
|
||||
dev, ok := deviceID(fileInfo)
|
||||
Expect(ok).To(BeTrue())
|
||||
|
||||
memo := &sync.Map{}
|
||||
memo.Store(dev, struct{}{})
|
||||
lfi := localFileInfo{FileInfo: fileInfo, path: testFile, noBirthTime: memo}
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
Expect(lfi.BirthTime()).To(BeTemporally("~", time.Now(), 100*time.Millisecond))
|
||||
})
|
||||
})
|
||||
|
||||
It("should delegate all other FileInfo methods", func() {
|
||||
|
||||
4
go.mod
4
go.mod
@ -6,7 +6,7 @@ go 1.27
|
||||
replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3
|
||||
|
||||
// Fork to implement CloseWithError, proposed upstream in https://github.com/djherbis/fscache/pull/22
|
||||
replace github.com/djherbis/fscache => github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2
|
||||
replace github.com/djherbis/fscache => github.com/deluan/fscache v0.9.1-0.20260829205053-654a9d517048
|
||||
|
||||
require (
|
||||
github.com/Masterminds/squirrel v1.5.4
|
||||
@ -17,7 +17,7 @@ require (
|
||||
github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36
|
||||
github.com/djherbis/atime v1.1.0
|
||||
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4
|
||||
github.com/djherbis/stream v1.4.0
|
||||
github.com/djherbis/stream v1.5.0
|
||||
github.com/djherbis/times v1.6.0
|
||||
github.com/dustin/go-humanize v1.0.1
|
||||
github.com/extism/go-sdk v1.7.1
|
||||
|
||||
8
go.sum
8
go.sum
@ -29,8 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
|
||||
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo=
|
||||
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40=
|
||||
github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2 h1:s254V2hsrrCJXYtAn9WPG/5p4QHenfL9E+j6Tiq5MW4=
|
||||
github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2/go.mod h1:eNFa48vJrse+8ysT4IJnnUeXwLZNcR0JQumU/W/QoUI=
|
||||
github.com/deluan/fscache v0.9.1-0.20260829205053-654a9d517048 h1:u3oDvM7pOIouwDGeIoMu9SYgSzVMtiSkOc+xywtKiJk=
|
||||
github.com/deluan/fscache v0.9.1-0.20260829205053-654a9d517048/go.mod h1:Bbk9SqpJcg/saiPfG6byM1G4G/LQndknrsLVOQ+VJqY=
|
||||
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 h1:j7eSXqgtjhlNfwnMEzRdXnJGZTEw4I7J9TeQAll83bU=
|
||||
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
|
||||
github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4=
|
||||
@ -41,8 +41,8 @@ github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36 h1:KtPfdSST6e0vJbM
|
||||
github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36/go.mod h1:RkQWLNITKkXHLP7LXxZSgEq+uFWU25M5qW7qfEhL9Wc=
|
||||
github.com/djherbis/atime v1.1.0 h1:rgwVbP/5by8BvvjBNrbh64Qz33idKT3pSnMSJsxhi0g=
|
||||
github.com/djherbis/atime v1.1.0/go.mod h1:28OF6Y8s3NQWwacXc5eZTsEsiMzp7LF8MbXE+XJPdBE=
|
||||
github.com/djherbis/stream v1.4.0 h1:aVD46WZUiq5kJk55yxJAyw6Kuera6kmC3i2vEQyW/AE=
|
||||
github.com/djherbis/stream v1.4.0/go.mod h1:cqjC1ZRq3FFwkGmUtHwcldbnW8f0Q4YuVsGW1eAFtOk=
|
||||
github.com/djherbis/stream v1.5.0 h1:+ewqpS/ndTmEiJRH142JyOpmmjnkTxdbJtqoAG5sg0Y=
|
||||
github.com/djherbis/stream v1.5.0/go.mod h1:cqjC1ZRq3FFwkGmUtHwcldbnW8f0Q4YuVsGW1eAFtOk=
|
||||
github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c=
|
||||
github.com/djherbis/times v1.6.0/go.mod h1:gOHeRAz2h+VJNZ5Gmc/o7iD9k4wW7NMVqieYCY99oc0=
|
||||
github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
|
||||
|
||||
@ -1,6 +1,11 @@
|
||||
package plugins
|
||||
|
||||
import "slices"
|
||||
import (
|
||||
"regexp"
|
||||
"slices"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
)
|
||||
|
||||
// Capability represents a plugin capability type.
|
||||
// Capabilities are detected by checking which functions a plugin exports.
|
||||
@ -39,3 +44,25 @@ func detectCapabilities(plugin functionExistsChecker) []Capability {
|
||||
func hasCapability(capabilities []Capability, cap Capability) bool {
|
||||
return slices.Contains(capabilities, cap)
|
||||
}
|
||||
|
||||
// retryLaterRe matches one capability's `<capability>(retry_later[:seconds])` token, which is
|
||||
// all a plugin fault carries back across the WASM boundary. The capability is part of the
|
||||
// pattern, so another capability's token in the same message cannot mask this one. The leading
|
||||
// \b keeps a superstring like `useragent(retry_later)` from matching `agent`.
|
||||
var (
|
||||
agentRetryLaterRe = retryLaterRe("agent")
|
||||
scrobblerRetryLaterRe = retryLaterRe("scrobbler")
|
||||
)
|
||||
|
||||
func retryLaterRe(capability string) *regexp.Regexp {
|
||||
return regexp.MustCompile(`\b` + capability + `\(retry_later(?::(\d+))?\)`)
|
||||
}
|
||||
|
||||
// parseRetryLater reports whether msg carries the capability's retry_later token, with its delay.
|
||||
func parseRetryLater(re *regexp.Regexp, msg string) (*agents.RetryLaterError, bool) {
|
||||
m := re.FindStringSubmatch(msg)
|
||||
if m == nil {
|
||||
return nil, false
|
||||
}
|
||||
return &agents.RetryLaterError{RetryIn: agents.ParseRetryIn(m[1])}, true
|
||||
}
|
||||
|
||||
@ -224,3 +224,15 @@ type SimilarSongsResponse struct {
|
||||
// Songs is the list of similar songs.
|
||||
Songs []types.SongRef `json:"songs"`
|
||||
}
|
||||
|
||||
// MetadataAgentError represents an error type for metadata agent operations.
|
||||
type MetadataAgentError string
|
||||
|
||||
const (
|
||||
// MetadataAgentErrorRetryLater indicates the provider is throttling; retry later.
|
||||
// Append ":<seconds>" inside the parentheses to request a specific delay.
|
||||
MetadataAgentErrorRetryLater MetadataAgentError = "agent(retry_later)"
|
||||
)
|
||||
|
||||
// Error implements the error interface for MetadataAgentError.
|
||||
func (e MetadataAgentError) Error() string { return string(e) }
|
||||
|
||||
@ -56,6 +56,9 @@ func agentErr(err error) error {
|
||||
if errors.Is(err, errNotImplemented) || errors.Is(err, errFunctionNotFound) {
|
||||
return errors.Join(agents.ErrNotFound, err)
|
||||
}
|
||||
if retryLater, ok := parseRetryLater(agentRetryLaterRe, err.Error()); ok {
|
||||
return errors.Join(retryLater, err)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@ -5,6 +5,7 @@ package plugins
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/plugins/capabilities"
|
||||
@ -31,6 +32,31 @@ var _ = Describe("agentErr", func() {
|
||||
Entry("a non-zero exit is a fault",
|
||||
errors.New("plugin call exited with code 1"), false),
|
||||
)
|
||||
|
||||
DescribeTable("agentErr retry-later",
|
||||
func(msg string, wantDelay time.Duration) {
|
||||
err := agentErr(errors.New(msg))
|
||||
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
||||
retry, _ := errors.AsType[*agents.RetryLaterError](err)
|
||||
d := retry.RetryIn
|
||||
Expect(d).To(Equal(wantDelay))
|
||||
},
|
||||
Entry("bare token", "agent(retry_later)", time.Duration(0)),
|
||||
Entry("with seconds", "agent(retry_later:120)", 120*time.Second),
|
||||
Entry("capped at 1h", "agent(retry_later:999999)", time.Hour),
|
||||
// Scaling to nanoseconds before capping wraps past 2^64, landing on ~0.29s.
|
||||
Entry("capped before it can overflow", "agent(retry_later:18446744074)", time.Hour),
|
||||
)
|
||||
|
||||
It("leaves other plugin errors untouched", func() {
|
||||
orig := errors.New("some plugin failure")
|
||||
Expect(agentErr(orig)).To(Equal(orig))
|
||||
})
|
||||
|
||||
It("does not treat a superstring token as a throttle", func() {
|
||||
orig := errors.New("useragent(retry_later)")
|
||||
Expect(agentErr(orig)).To(Equal(orig))
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("MetadataAgent", Ordered, func() {
|
||||
|
||||
@ -18,6 +18,18 @@ type ArtistRef = types.ArtistRef
|
||||
// Deprecated: use types.SongRef.
|
||||
type SongRef = types.SongRef
|
||||
|
||||
// MetadataAgentError represents an error type for metadata agent operations.
|
||||
type MetadataAgentError string
|
||||
|
||||
const (
|
||||
// MetadataAgentErrorRetryLater indicates the provider is throttling; retry later.
|
||||
// Append ":<seconds>" inside the parentheses to request a specific delay.
|
||||
MetadataAgentErrorRetryLater MetadataAgentError = "agent(retry_later)"
|
||||
)
|
||||
|
||||
// Error implements the error interface for MetadataAgentError.
|
||||
func (e MetadataAgentError) Error() string { return string(e) }
|
||||
|
||||
// AlbumImagesResponse is the response for GetAlbumImages.
|
||||
type AlbumImagesResponse struct {
|
||||
// Images is the list of album images.
|
||||
|
||||
@ -16,6 +16,18 @@ type ArtistRef = types.ArtistRef
|
||||
// Deprecated: use types.SongRef.
|
||||
type SongRef = types.SongRef
|
||||
|
||||
// MetadataAgentError represents an error type for metadata agent operations.
|
||||
type MetadataAgentError string
|
||||
|
||||
const (
|
||||
// MetadataAgentErrorRetryLater indicates the provider is throttling; retry later.
|
||||
// Append ":<seconds>" inside the parentheses to request a specific delay.
|
||||
MetadataAgentErrorRetryLater MetadataAgentError = "agent(retry_later)"
|
||||
)
|
||||
|
||||
// Error implements the error interface for MetadataAgentError.
|
||||
func (e MetadataAgentError) Error() string { return string(e) }
|
||||
|
||||
// AlbumImagesResponse is the response for GetAlbumImages.
|
||||
type AlbumImagesResponse struct {
|
||||
// Images is the list of album images.
|
||||
|
||||
@ -24,6 +24,11 @@ pub type ArtistRef = nd_pdk_types::ArtistRef;
|
||||
|
||||
#[deprecated(note = "use nd_pdk::types::SongRef")]
|
||||
pub type SongRef = nd_pdk_types::SongRef;
|
||||
/// MetadataAgentError represents an error type for metadata agent operations.
|
||||
pub type MetadataAgentError = &'static str;
|
||||
/// MetadataAgentErrorRetryLater indicates the provider is throttling; retry later.
|
||||
/// Append ":<seconds>" inside the parentheses to request a specific delay.
|
||||
pub const METADATA_AGENT_ERROR_RETRY_LATER: MetadataAgentError = "agent(retry_later)";
|
||||
/// AlbumImagesResponse is the response for GetAlbumImages.
|
||||
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
|
||||
@ -174,11 +174,12 @@ func mapScrobblerError(err error) error {
|
||||
return nil
|
||||
}
|
||||
errMsg := err.Error()
|
||||
retryLater, isRetryLater := parseRetryLater(scrobblerRetryLaterRe, errMsg)
|
||||
switch {
|
||||
case strings.Contains(errMsg, capabilities.ScrobblerErrorNotAuthorized.Error()):
|
||||
return scrobbler.ErrNotAuthorized
|
||||
case strings.Contains(errMsg, capabilities.ScrobblerErrorRetryLater.Error()):
|
||||
return scrobbler.ErrRetryLater
|
||||
case isRetryLater:
|
||||
return retryLater
|
||||
case strings.Contains(errMsg, capabilities.ScrobblerErrorUnrecoverable.Error()):
|
||||
return scrobbler.ErrUnrecoverable
|
||||
default:
|
||||
|
||||
@ -7,6 +7,7 @@ import (
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/core/scrobbler"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
@ -362,4 +363,25 @@ var _ = Describe("mapScrobblerError", func() {
|
||||
err := mapScrobblerError(errors.New("some unknown error"))
|
||||
Expect(err).To(MatchError(scrobbler.ErrUnrecoverable))
|
||||
})
|
||||
|
||||
DescribeTable("mapScrobblerError retry-later",
|
||||
func(msg string, wantDelay time.Duration) {
|
||||
err := mapScrobblerError(errors.New(msg))
|
||||
Expect(errors.Is(err, scrobbler.ErrRetryLater)).To(BeTrue())
|
||||
retry, _ := errors.AsType[*agents.RetryLaterError](err)
|
||||
d := retry.RetryIn
|
||||
Expect(d).To(Equal(wantDelay))
|
||||
},
|
||||
Entry("bare token", "scrobbler(retry_later)", time.Duration(0)),
|
||||
Entry("with seconds", "scrobbler(retry_later:30)", 30*time.Second),
|
||||
Entry("capped at 1h", "scrobbler(retry_later:999999)", time.Hour),
|
||||
// Scaling to nanoseconds before capping wraps past 2^64, landing on ~0.29s.
|
||||
Entry("capped before it can overflow", "scrobbler(retry_later:18446744074)", time.Hour),
|
||||
Entry("wrapped in context", "plugin xyz: scrobbler(retry_later:5)", 5*time.Second),
|
||||
)
|
||||
|
||||
It("still maps unknown errors to unrecoverable", func() {
|
||||
err := mapScrobblerError(errors.New("scrobbler(retry_later_garbage"))
|
||||
Expect(errors.Is(err, scrobbler.ErrUnrecoverable)).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
@ -394,6 +394,7 @@
|
||||
"invalidJson": "A configuração deve ser um JSON válido"
|
||||
},
|
||||
"messages": {
|
||||
"idHelp": "O ID do plugin, derivado do nome do arquivo. Use-o ao referenciar este plugin em opções de configuração, como Agents.",
|
||||
"configHelp": "Configure o plugin usando pares chave-valor. Deixe vazio se o plugin não precisa de configuração.",
|
||||
"clickPermissions": "Clique em uma permissão para ver detalhes",
|
||||
"noConfig": "Nenhuma configuração definida",
|
||||
|
||||
@ -10,7 +10,7 @@ types:
|
||||
.ogg: audio/ogg
|
||||
.oga: audio/ogg
|
||||
.opus: audio/ogg
|
||||
.aac: audio/mp4
|
||||
.aac: audio/aac
|
||||
.alac: audio/mp4
|
||||
.m4a: audio/mp4
|
||||
.m4b: audio/mp4
|
||||
|
||||
@ -397,6 +397,7 @@
|
||||
"invalidJson": "Configuration must be valid JSON"
|
||||
},
|
||||
"messages": {
|
||||
"idHelp": "The plugin ID, derived from its file name. Use it when referencing this plugin in configuration options, such as Agents.",
|
||||
"configHelp": "Configure the plugin using key-value pairs. Leave empty if the plugin requires no configuration.",
|
||||
"configValidationError": "Configuration validation failed:",
|
||||
"schemaRenderError": "Unable to render configuration form. The plugin's schema may be invalid.",
|
||||
|
||||
@ -123,6 +123,13 @@ export const InfoCard = ({ record, manifest, classes, translate, isSmall }) => (
|
||||
isSmall={isSmall}
|
||||
>
|
||||
{record.id}
|
||||
<Typography
|
||||
variant="caption"
|
||||
color="textSecondary"
|
||||
className={classes.fieldHelp}
|
||||
>
|
||||
{translate('resources.plugin.messages.idHelp')}
|
||||
</Typography>
|
||||
</InfoRow>
|
||||
|
||||
{manifest?.name && (
|
||||
@ -201,7 +208,7 @@ export const InfoCard = ({ record, manifest, classes, translate, isSmall }) => (
|
||||
<Typography
|
||||
variant="caption"
|
||||
color="textSecondary"
|
||||
style={{ marginTop: 4, display: 'block' }}
|
||||
className={classes.fieldHelp}
|
||||
>
|
||||
{translate('resources.plugin.messages.clickPermissions')}
|
||||
</Typography>
|
||||
|
||||
41
ui/src/plugin/InfoCard.test.jsx
Normal file
41
ui/src/plugin/InfoCard.test.jsx
Normal file
@ -0,0 +1,41 @@
|
||||
import React from 'react'
|
||||
import { render, screen } from '@testing-library/react'
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
|
||||
vi.mock('../common', () => ({
|
||||
DateField: ({ source }) => <span data-testid={`date-${source}`} />,
|
||||
}))
|
||||
|
||||
const { InfoCard } = await import('./InfoCard')
|
||||
|
||||
const record = {
|
||||
id: 'apple-music',
|
||||
path: '/data/plugins/apple-music.ndp',
|
||||
updatedAt: '2026-01-01T00:00:00Z',
|
||||
createdAt: '2026-01-01T00:00:00Z',
|
||||
}
|
||||
|
||||
const renderCard = () =>
|
||||
render(
|
||||
<InfoCard
|
||||
record={record}
|
||||
manifest={{ name: 'Apple Music Metadata Agent' }}
|
||||
classes={{}}
|
||||
translate={(key) => key}
|
||||
isSmall={false}
|
||||
/>,
|
||||
)
|
||||
|
||||
describe('InfoCard', () => {
|
||||
it('shows the plugin ID', () => {
|
||||
renderCard()
|
||||
expect(screen.getByText('apple-music')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('explains that the ID is the name used in config options', () => {
|
||||
renderCard()
|
||||
expect(
|
||||
screen.getByText('resources.plugin.messages.idHelp'),
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
@ -45,6 +45,10 @@ export const usePluginShowStyles = makeStyles(
|
||||
fontSize: '0.85rem',
|
||||
wordBreak: 'break-all',
|
||||
},
|
||||
fieldHelp: {
|
||||
marginTop: theme.spacing(0.5),
|
||||
display: 'block',
|
||||
},
|
||||
permissionsContainer: {
|
||||
display: 'flex',
|
||||
flexWrap: 'wrap',
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user