From b35ae4b0c9fe02d389c5b1beecd6aea36a65bf1c Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 30 Aug 2026 21:46:29 -0400 Subject: [PATCH 1/2] fix(artwork): honor a provider's explicit retry-later delay in the circuit breaker An explicit RetryLaterError now opens the agent's breaker immediately for the provider's own delay, instead of counting it as one generic failure that needs five to open and then always probes after a fixed minute. --- core/artwork/gate.go | 19 ++++++++++++++++++- core/artwork/gate_test.go | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/core/artwork/gate.go b/core/artwork/gate.go index ebb65bcbe..aa2ad8f2f 100644 --- a/core/artwork/gate.go +++ b/core/artwork/gate.go @@ -1,6 +1,7 @@ package artwork import ( + "cmp" "context" "errors" "io" @@ -95,6 +96,9 @@ type breaker struct { // generation identifies the current open episode, so an answer from a call admitted before // the breaker opened cannot be mistaken for evidence that it has recovered. generation int + // probeAfter overrides the probe delay for the current episode when a provider named its own + // back-off; zero falls back to breakerProbeAfter. + probeAfter time.Duration } func newBreaker() *breaker { return &breaker{} } @@ -107,7 +111,7 @@ func (b *breaker) allow() (bool, int) { if b.failures < breakerThreshold { return true, 0 } - if time.Since(b.openedAt) >= breakerProbeAfter { + if time.Since(b.openedAt) >= cmp.Or(b.probeAfter, breakerProbeAfter) { b.openedAt = time.Now() // start a fresh probe window so only one caller passes return true, b.generation } @@ -121,11 +125,24 @@ func (b *breaker) record(name string, gen int, err error) { } b.mu.Lock() defer b.mu.Unlock() + // An explicit back-off is a definitive "stop for this long", so it opens the breaker at once + // with the provider's own delay instead of waiting for the failure threshold. + if retry, ok := errors.AsType[*agents.RetryLaterError](err); ok && retry.RetryIn > 0 { + b.recoveries = 0 + b.failures = breakerThreshold + b.openedAt = time.Now() + b.probeAfter = retry.RetryIn + b.generation++ + log.Warn("Artwork: Circuit breaker opened for agent, provider asked to back off", "agent", name, + "probeAfter", retry.RetryIn) + return + } if isTransientExternal(err) { b.recoveries = 0 b.failures++ if b.failures == breakerThreshold { b.openedAt = time.Now() + b.probeAfter = 0 b.generation++ log.Warn("Artwork: Circuit breaker opened for agent", "agent", name, "consecutiveFailures", b.failures, "probeAfter", breakerProbeAfter, err) diff --git a/core/artwork/gate_test.go b/core/artwork/gate_test.go index abe723508..a954d4cac 100644 --- a/core/artwork/gate_test.go +++ b/core/artwork/gate_test.go @@ -2,7 +2,9 @@ package artwork import ( "errors" + "time" + "github.com/navidrome/navidrome/core/agents" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -40,4 +42,15 @@ var _ = Describe("breaker", func() { Expect(allowed(b)).To(BeFalse(), "answers from calls admitted before the breaker opened must not close it") }) + + It("opens at once when a provider asks to retry later, honoring its delay", func() { + b := newBreaker() + Expect(allowed(b)).To(BeTrue(), "starts closed") + + // A single explicit back-off opens the breaker without reaching the failure threshold. + b.record("agentA", 0, &agents.RetryLaterError{RetryIn: 5 * time.Second}) + + Expect(allowed(b)).To(BeFalse(), "an explicit back-off opens the breaker immediately") + Expect(b.probeAfter).To(Equal(5*time.Second), "the provider's delay drives the probe interval") + }) }) From 8dfd8f842d7c95ff8052427bbc1fbc951c064f35 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 30 Aug 2026 21:46:56 -0400 Subject: [PATCH 2/2] chore(lastfm): clearer log when a scrobble is dropped for being too old Last.fm ignore code 3 means the timestamp is older than 14 days; spell that out instead of the generic ignored-scrobble warning. --- adapters/lastfm/client.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/adapters/lastfm/client.go b/adapters/lastfm/client.go index e468aa638..c50a11aeb 100644 --- a/adapters/lastfm/client.go +++ b/adapters/lastfm/client.go @@ -183,9 +183,15 @@ func (c *client) scrobble(ctx context.Context, sessionKey string, info ScrobbleI if err != nil { return err } - if resp.Scrobbles.Scrobble.IgnoredMessage.Code != "0" { - log.Warn(ctx, "LastFM: scrobble was ignored", "code", resp.Scrobbles.Scrobble.IgnoredMessage.Code, - "text", resp.Scrobbles.Scrobble.IgnoredMessage.Text, "info", info) + if code := resp.Scrobbles.Scrobble.IgnoredMessage.Code; code != "0" { + if code == "3" { + // Last.fm rejects scrobbles whose timestamp is older than 14 days; the buffer drops it. + log.Warn(ctx, "LastFM: scrobble dropped, timestamp too old (older than 14 days)", + "code", code, "info", info) + } else { + log.Warn(ctx, "LastFM: scrobble was ignored", "code", code, + "text", resp.Scrobbles.Scrobble.IgnoredMessage.Text, "info", info) + } } if resp.Scrobbles.Attr.Accepted != 1 { log.Warn(ctx, "LastFM: scrobble was not accepted", "code", resp.Scrobbles.Scrobble.IgnoredMessage.Code,