mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 10:27:08 +02:00
* fix(deezer): treat an exhausted quota as a throttle, not as a missing artist
Deezer reports quota exhaustion in the response body, with HTTP 200 and no
rate-limit headers. The client only looked for errors when the status was
not 200, so a throttled reply was decoded into an empty result type, and an
empty search became ErrNotFound. The agent then compounded it: it tested
`errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so
any failed search — which also returns no artists — reported not-found too.
The artwork worker settles an entity as "no image" on agents.ErrNotFound.
So being throttled did not make Navidrome back off; it made it record the
artist as having no artwork, and move on to do the same to the next one.
Errors are now parsed out of the body regardless of status, and the quota
code is joined with agents.RetryLaterError so the circuit breaker and the
artwork retry budget see a throttle for what it is. The agent checks err
before the empty-result case.
Last.fm already handles this exact shape (client.go errCodeRateLimit, with
a comment noting the 200-with-body-error pattern); this brings Deezer in
line with it, including the zero-delay RetryLaterError so both providers
share the default cooldown rather than a per-provider number.
Measured against the live API to pin the shape: a 120-request burst
returned 54 results and 66 quota replies, every one of them HTTP 200 with
{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}
and no Retry-After or rate-limit headers. A single request 5s later
succeeded, so the window is short and a cooldown fully clears it.
* refactor(deezer): fold the error envelope into one type
The envelope declared the code and message inline, parseBodyError copied
them field by field into a second struct with the same shape, and a zero
Code stood in for "no error reported". Making the envelope hold a pointer
to the error type removes all three: absent is nil, present is the error
itself, and the value returned needs no conversion.
searchArtist loses its empty-result branch. searchArtists converts an
empty result to errNotFound and returns early on any error, so it never
answers with no artists and no error, and the branch could not run. What
it left behind was a comment explaining an ordering that only mattered
while the branch existed.
ErrNotFound is unexported: nothing outside this package referenced it,
and it sat three lines from agents.ErrNotFound, which is a different
error with the opposite meaning for callers.
Throttling now joins agents.ErrRetryLater, the sentinel documented as the
zero-delay RetryLaterError, rather than allocating an equivalent value.
* refactor(deezer): return agents.ErrNotFound from the client
The client raised a package-local sentinel that the agent then translated
into agents.ErrNotFound, one call site each. Deezer was the only adapter
carrying its own: last.fm and listenbrainz have none.
The client already reports throttling with agents.ErrRetryLater, so it
already speaks the agent vocabulary; saying "not found" in the same words
costs nothing and lets searchArtist drop to plain error propagation.
* test(scrobbler): remove a race in the longest-server-delay test
newBufferedScrobbler starts its drain goroutine, and run() drains once
before it ever waits on the wake signal. The test enqueued user2, then
enqueued user1 via Scrobble, so that startup drain could land between
the two: it saw only user2, took its 45s delay, and set backingOff. The
wake from the second enqueue is then deliberately ignored — a wake
during a backoff window must not drain, which is the hammering the
window exists to prevent — so user1 was never attempted and the first
assertion read 1 instead of 2.
Buffering both users before the goroutine exists removes the window.
The test no longer goes through Scrobble, which the sibling tests
already cover; what this one is about is which delay wins.
Reproduced deterministically by forcing the interleaving with a
synctest.Wait between the two enqueues, which fails with the same
"expected both users drained, got 1 attempts" seen in CI. With both
enqueued first, that same forced drain passes.
296 lines
9.7 KiB
Go
296 lines
9.7 KiB
Go
package deezer
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"net/http"
|
|
"os"
|
|
"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"
|
|
)
|
|
|
|
var _ = Describe("deezerAgent", func() {
|
|
var ctx context.Context
|
|
|
|
BeforeEach(func() {
|
|
ctx = context.Background()
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.Deezer.Enabled = true
|
|
})
|
|
|
|
Describe("deezerConstructor", func() {
|
|
It("uses configured languages", func() {
|
|
conf.Server.Deezer.Languages = []string{"pt", "en"}
|
|
agent := deezerConstructor(&tests.MockDataStore{}).(*deezerAgent)
|
|
Expect(agent.languages).To(Equal([]string{"pt", "en"}))
|
|
})
|
|
})
|
|
|
|
Describe("searchArtist", func() {
|
|
var agent *deezerAgent
|
|
var httpClient *fakeHttpClient
|
|
|
|
BeforeEach(func() {
|
|
httpClient = &fakeHttpClient{}
|
|
agent = &deezerAgent{
|
|
dataStore: &tests.MockDataStore{},
|
|
client: newClient(httpClient),
|
|
}
|
|
})
|
|
|
|
It("picks the exact-name match with the most fans when several share the name", func() {
|
|
// Deezer RANKING order returns a low-popularity homonym first (see issue #5802)
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[
|
|
{"id":61045802,"name":"Queen","nb_fan":75},
|
|
{"id":141954732,"name":"Queen","nb_fan":397},
|
|
{"id":135041032,"name":"Queen(Ares)","nb_fan":133},
|
|
{"id":183179807,"name":"Queen","nb_fan":53},
|
|
{"id":412,"name":"Queen","nb_fan":12744378}
|
|
],"total":5}`)),
|
|
})
|
|
|
|
artist, err := agent.searchArtist(ctx, "Queen")
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(artist.ID).To(Equal(412))
|
|
})
|
|
|
|
It("matches the name case-insensitively", func() {
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[
|
|
{"id":1,"name":"QUEEN","nb_fan":10},
|
|
{"id":2,"name":"queen","nb_fan":20}
|
|
],"total":2}`)),
|
|
})
|
|
|
|
artist, err := agent.searchArtist(ctx, "Queen")
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(artist.ID).To(Equal(2))
|
|
})
|
|
|
|
// The artwork worker settles an artist as "no image" on agents.ErrNotFound, so a throttled
|
|
// lookup reaching that here would record a permanent absence.
|
|
It("surfaces an exhausted quota instead of reporting the artist as not found", func() {
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(
|
|
`{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}`)),
|
|
})
|
|
|
|
_, err := agent.searchArtist(ctx, "Queen")
|
|
|
|
Expect(err).To(HaveOccurred())
|
|
Expect(err).ToNot(MatchError(agents.ErrNotFound))
|
|
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
|
|
})
|
|
|
|
It("returns ErrNotFound when no result matches the name exactly", func() {
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[
|
|
{"id":1,"name":"Queens of the Stone Age","nb_fan":100}
|
|
],"total":1}`)),
|
|
})
|
|
|
|
_, err := agent.searchArtist(ctx, "Queen")
|
|
|
|
Expect(err).To(MatchError(agents.ErrNotFound))
|
|
})
|
|
})
|
|
|
|
Describe("GetArtistImages", func() {
|
|
var agent *deezerAgent
|
|
var httpClient *fakeHttpClient
|
|
|
|
BeforeEach(func() {
|
|
httpClient = &fakeHttpClient{}
|
|
agent = &deezerAgent{
|
|
dataStore: &tests.MockDataStore{},
|
|
client: newClient(httpClient),
|
|
}
|
|
})
|
|
|
|
It("returns the real images when the artist has a picture", func() {
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[
|
|
{"id":412,"name":"Queen","nb_fan":12744378,
|
|
"picture_xl":"https://cdn-images.dzcdn.net/images/artist/abc/1000x1000-000000-80-0-0.jpg",
|
|
"picture_big":"https://cdn-images.dzcdn.net/images/artist/abc/500x500-000000-80-0-0.jpg"}
|
|
],"total":1}`)),
|
|
})
|
|
|
|
images, err := agent.GetArtistImages(ctx, "", "Queen", "")
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(images).To(HaveLen(2))
|
|
Expect(images[0].URL).To(ContainSubstring("1000x1000"))
|
|
})
|
|
|
|
It("returns ErrNotFound when the artist only has empty-id placeholder pictures", func() {
|
|
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[
|
|
{"id":412,"name":"Queen","nb_fan":12744378,
|
|
"picture_xl":"https://cdn-images.dzcdn.net/images/artist//1000x1000-000000-80-0-0.jpg",
|
|
"picture_big":"https://cdn-images.dzcdn.net/images/artist//500x500-000000-80-0-0.jpg",
|
|
"picture_medium":"https://cdn-images.dzcdn.net/images/artist//250x250-000000-80-0-0.jpg",
|
|
"picture_small":"https://cdn-images.dzcdn.net/images/artist//56x56-000000-80-0-0.jpg"}
|
|
],"total":1}`)),
|
|
})
|
|
|
|
images, err := agent.GetArtistImages(ctx, "", "Queen", "")
|
|
|
|
Expect(err).To(MatchError(agents.ErrNotFound))
|
|
Expect(images).To(BeEmpty())
|
|
})
|
|
})
|
|
|
|
Describe("GetArtistBiography - Language Fallback", func() {
|
|
var agent *deezerAgent
|
|
var httpClient *langAwareHttpClient
|
|
|
|
BeforeEach(func() {
|
|
httpClient = newLangAwareHttpClient()
|
|
|
|
// Mock search artist (returns Michael Jackson)
|
|
fSearch, _ := os.Open("tests/fixtures/deezer.search.artist.json")
|
|
httpClient.searchResponse = &http.Response{Body: fSearch, StatusCode: 200}
|
|
|
|
// Mock JWT token
|
|
testJWT := createTestJWT(5 * time.Minute)
|
|
httpClient.jwtResponse = &http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"jwt":"%s","refresh_token":""}`, testJWT))),
|
|
}
|
|
})
|
|
|
|
setupAgent := func(languages []string) {
|
|
conf.Server.Deezer.Languages = languages
|
|
agent = &deezerAgent{
|
|
dataStore: &tests.MockDataStore{},
|
|
client: newClient(httpClient),
|
|
languages: languages,
|
|
}
|
|
}
|
|
|
|
It("returns content in first language when available (1 bio API call)", func() {
|
|
setupAgent([]string{"fr", "en"})
|
|
|
|
// French biography available
|
|
fFr, _ := os.Open("tests/fixtures/deezer.artist.bio.fr.json")
|
|
httpClient.bioResponses["fr"] = &http.Response{Body: fFr, StatusCode: 200}
|
|
|
|
bio, err := agent.GetArtistBiography(ctx, "", "Michael Jackson", "")
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(bio).To(ContainSubstring("Guy-Manuel de Homem Christo et Thomas Bangalter"))
|
|
Expect(httpClient.bioRequestCount).To(Equal(1))
|
|
Expect(httpClient.bioRequests[0].Header.Get("Accept-Language")).To(Equal("fr"))
|
|
})
|
|
|
|
It("falls back to second language when first returns empty (2 bio API calls)", func() {
|
|
setupAgent([]string{"ja", "en"})
|
|
|
|
// Japanese returns empty biography
|
|
fJa, _ := os.Open("tests/fixtures/deezer.artist.bio.empty.json")
|
|
httpClient.bioResponses["ja"] = &http.Response{Body: fJa, StatusCode: 200}
|
|
// English returns full biography
|
|
fEn, _ := os.Open("tests/fixtures/deezer.artist.bio.en.json")
|
|
httpClient.bioResponses["en"] = &http.Response{Body: fEn, StatusCode: 200}
|
|
|
|
bio, err := agent.GetArtistBiography(ctx, "", "Michael Jackson", "")
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(bio).To(ContainSubstring("Schoolmates Thomas and Guy-Manuel"))
|
|
Expect(httpClient.bioRequestCount).To(Equal(2))
|
|
Expect(httpClient.bioRequests[0].Header.Get("Accept-Language")).To(Equal("ja"))
|
|
Expect(httpClient.bioRequests[1].Header.Get("Accept-Language")).To(Equal("en"))
|
|
})
|
|
|
|
It("returns ErrNotFound when all languages return empty", func() {
|
|
setupAgent([]string{"ja", "xx"})
|
|
|
|
// Both languages return empty biography
|
|
fJa, _ := os.Open("tests/fixtures/deezer.artist.bio.empty.json")
|
|
httpClient.bioResponses["ja"] = &http.Response{Body: fJa, StatusCode: 200}
|
|
fXx, _ := os.Open("tests/fixtures/deezer.artist.bio.empty.json")
|
|
httpClient.bioResponses["xx"] = &http.Response{Body: fXx, StatusCode: 200}
|
|
|
|
_, err := agent.GetArtistBiography(ctx, "", "Michael Jackson", "")
|
|
|
|
Expect(err).To(MatchError(agents.ErrNotFound))
|
|
Expect(httpClient.bioRequestCount).To(Equal(2))
|
|
})
|
|
})
|
|
})
|
|
|
|
// langAwareHttpClient is a mock HTTP client that returns different responses based on the Accept-Language header
|
|
type langAwareHttpClient struct {
|
|
searchResponse *http.Response
|
|
jwtResponse *http.Response
|
|
bioResponses map[string]*http.Response
|
|
bioRequests []*http.Request
|
|
bioRequestCount int
|
|
}
|
|
|
|
func newLangAwareHttpClient() *langAwareHttpClient {
|
|
return &langAwareHttpClient{
|
|
bioResponses: make(map[string]*http.Response),
|
|
bioRequests: make([]*http.Request, 0),
|
|
}
|
|
}
|
|
|
|
func (c *langAwareHttpClient) Do(req *http.Request) (*http.Response, error) {
|
|
// Handle search artist request
|
|
if req.URL.Host == "api.deezer.com" && req.URL.Path == "/search/artist" {
|
|
if c.searchResponse != nil {
|
|
return c.searchResponse, nil
|
|
}
|
|
return &http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":[],"total":0}`)),
|
|
}, nil
|
|
}
|
|
|
|
// Handle JWT token request
|
|
if req.URL.Host == "auth.deezer.com" && req.URL.Path == "/login/anonymous" {
|
|
if c.jwtResponse != nil {
|
|
return c.jwtResponse, nil
|
|
}
|
|
return &http.Response{
|
|
StatusCode: 500,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"error":"no mock"}`)),
|
|
}, nil
|
|
}
|
|
|
|
// Handle bio request (GraphQL API)
|
|
if req.URL.Host == "pipe.deezer.com" && req.URL.Path == "/api" {
|
|
c.bioRequestCount++
|
|
c.bioRequests = append(c.bioRequests, req)
|
|
lang := req.Header.Get("Accept-Language")
|
|
if resp, ok := c.bioResponses[lang]; ok {
|
|
return resp, nil
|
|
}
|
|
// Return empty bio by default
|
|
return &http.Response{
|
|
StatusCode: 200,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"data":{"artist":{"bio":{"full":""}}}}`)),
|
|
}, nil
|
|
}
|
|
|
|
panic("URL not mocked: " + req.URL.String())
|
|
}
|