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.
This commit is contained in:
Deluan 2026-08-30 21:46:29 -04:00
parent dbd26ba2e7
commit b35ae4b0c9
2 changed files with 31 additions and 1 deletions

View File

@ -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)

View File

@ -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")
})
})