From bce8d27370df79adb25f91e632e6796e31eb2f85 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Tue, 6 Oct 2026 01:55:15 -0400 Subject: [PATCH] fix(explore): stop asking ListenBrainz after it refuses this client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every popularity request answered 401 on 2026-10-05: 399 in one minute of a real library's discography backfill, one rate-limited request per artist to be told the same thing, each with its own log line. A token is a property of the installation, not of the artist, so the first 401 is the answer for the rest of that client's life. The refusal is latched, logged once at warning level, and returned as ErrListenBrainzUnauthorized — separate from the generic HTTP error because it is the one failure no retry can fix. A 500 still does not latch, so a transient failure leaves the artist unmarked and the next run asks again. The backfill also stops feeding artists once the client is refused: the rest of the pass would be the same 401, and the artists stay unmarked for a run that has a token. Closes #284 --- backend/explore/listenbrainz.go | 49 +++++++++++ backend/explore/listenbrainz_refusal_test.go | 90 ++++++++++++++++++++ backend/explore/searchindex.go | 18 ++++ 3 files changed, 157 insertions(+) create mode 100644 backend/explore/listenbrainz_refusal_test.go diff --git a/backend/explore/listenbrainz.go b/backend/explore/listenbrainz.go index 725d47b..bc05729 100644 --- a/backend/explore/listenbrainz.go +++ b/backend/explore/listenbrainz.go @@ -13,6 +13,7 @@ import ( "net/http" "slices" "strings" + "sync/atomic" "time" ) @@ -25,6 +26,19 @@ const ( // responds with a non-2xx status code. var ErrListenBrainzHTTP = errors.New("listenbrainz HTTP error") +// ErrListenBrainzUnauthorized is a 401: the endpoint wants a token, and +// no retry will change that. +// +// It is separate from ErrListenBrainzHTTP because it is the one +// failure that is about *this client* rather than about the thing being +// asked for — which is what makes it the one worth latching. The +// popularity endpoints answered 401 to every request on 2026-10-05, so +// a discography backfill spent one rate-limited request per artist to +// be told the same thing: 399 of them in a minute of a real library's +// backfill, each one a log line and a wasted slot in the shared +// limiter. +var ErrListenBrainzUnauthorized = errors.New("listenbrainz requires a token") + // ListenBrainzClient is a thin HTTP client for the ListenBrainz // popularity and labs APIs. All requests are rate-limited via the // shared RateLimiter and cached via the shared Cache. @@ -39,6 +53,13 @@ type ListenBrainzClient struct { // SetBaseURL shape — so a test that points one client at an // httptest server does not stop being parallel-safe. baseURL string + + // refused latches the first 401. A token is a property of the + // installation, not of the artist being asked about, so the answer + // is the same for every later request and asking again is pure + // cost. Per client rather than global so a test can have one that + // is refused and one that is not. + refused atomic.Bool } // NewListenBrainzClient creates a ListenBrainz API client. @@ -56,6 +77,14 @@ func NewListenBrainzClient( } } +// Unauthorized reports whether this client has been refused with a 401 +// during its life. A caller that is about to do a long pass of +// requests should ask before starting it: the answer will not change +// mid-pass. +func (c *ListenBrainzClient) Unauthorized() bool { + return c.refused.Load() +} + // SetBaseURL redirects this client at another host. Tests only. func (c *ListenBrainzClient) SetBaseURL(url string) { c.baseURL = strings.TrimSuffix(url, "/") @@ -489,6 +518,11 @@ func (c *ListenBrainzClient) doPost( func (c *ListenBrainzClient) doRequest( ctx context.Context, method string, url string, body []byte, ) ([]byte, error) { + // Asked and answered, for the rest of this client's life. + if c.refused.Load() { + return nil, fmt.Errorf("%w: %s", ErrListenBrainzUnauthorized, url) + } + c.logger.Debug("listenbrainz rate limiter wait", "url", url) if err := c.limiter.Wait(ctx); err != nil { @@ -533,6 +567,21 @@ func (c *ListenBrainzClient) doRequest( "status", resp.StatusCode, ) + if resp.StatusCode == http.StatusUnauthorized { + // Recorded once, at warning level, because the next thing this + // client does is stop asking: a log line per artist is the + // symptom this latch exists to remove. + if c.refused.CompareAndSwap(false, true) { + c.logger.Warn("listenbrainz refused this client: "+ + "popularity data needs a token, so the rest of this "+ + "run will not ask for it", + "url", url, + ) + } + + return nil, fmt.Errorf("%w: %s", ErrListenBrainzUnauthorized, url) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return nil, fmt.Errorf( "%w: %d %s", ErrListenBrainzHTTP, resp.StatusCode, truncateBody(respBody), diff --git a/backend/explore/listenbrainz_refusal_test.go b/backend/explore/listenbrainz_refusal_test.go new file mode 100644 index 0000000..033fcf7 --- /dev/null +++ b/backend/explore/listenbrainz_refusal_test.go @@ -0,0 +1,90 @@ +package explore + +import ( + "context" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "yellowjacket/backend/database" +) + +// #284: the popularity endpoints answered 401 to every request, so a +// backfill spent one rate-limited request per artist to be told the same +// thing — 399 of them in a minute against a real library. A token is a +// property of the installation, not of the artist, so the first refusal +// is the answer for the whole client. +func TestListenBrainzLatchesARefusal(t *testing.T) { + t.Parallel() + + var requests atomic.Int64 + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusUnauthorized) + }, + )) + t.Cleanup(srv.Close) + + c := NewListenBrainzClient( + NewRateLimiter(), NewCache(database.NewTestDB(t), slog.Default()), slog.Default(), + ) + c.SetBaseURL(srv.URL) + + for i := range 5 { + _, err := c.TopRecordingsForArtist(context.Background(), "an-mbid") + + if !errors.Is(err, ErrListenBrainzUnauthorized) { + t.Fatalf("call %d: err = %v, want ErrListenBrainzUnauthorized", i, err) + } + } + + if got := requests.Load(); got != 1 { + t.Errorf("requests = %d, want 1: the rest of the calls are the same answer", got) + } + + if !c.Unauthorized() { + t.Error("Unauthorized() = false after a 401") + } +} + +// A failure that a retry could fix must not latch: the artist stays +// unmarked and the next run asks again. +func TestListenBrainzDoesNotLatchATransientFailure(t *testing.T) { + t.Parallel() + + var requests atomic.Int64 + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusInternalServerError) + }, + )) + t.Cleanup(srv.Close) + + c := NewListenBrainzClient( + NewRateLimiter(), NewCache(database.NewTestDB(t), slog.Default()), slog.Default(), + ) + c.SetBaseURL(srv.URL) + + for range 3 { + _, err := c.TopRecordingsForArtist(context.Background(), "an-mbid") + + if !errors.Is(err, ErrListenBrainzHTTP) { + t.Fatalf("err = %v, want ErrListenBrainzHTTP", err) + } + } + + if got := requests.Load(); got != 3 { + t.Errorf("requests = %d, want 3", got) + } + + if c.Unauthorized() { + t.Error("Unauthorized() = true after a 500") + } +} diff --git a/backend/explore/searchindex.go b/backend/explore/searchindex.go index 4e4c7c5..4832a4c 100644 --- a/backend/explore/searchindex.go +++ b/backend/explore/searchindex.go @@ -518,6 +518,13 @@ func (si *SearchIndex) BackfillLibraryDiscographies(ctx context.Context) { break } + // A refused client is refused for every artist: the rest of + // this pass would be the same 401, once per artist (#284). The + // artists stay unmarked, so a run with a token picks them up. + if indexLB.Unauthorized() { + break + } + work <- mbid } @@ -534,6 +541,17 @@ func (si *SearchIndex) BackfillLibraryDiscographies(ctx context.Context) { return } + if indexLB.Unauthorized() { + si.logger.Warn("discography backfill stopped early: "+ + "listenbrainz refused this client, and a token is what it wants", + "artists", total, "of", len(mbids), + ) + + job.logf(jobs.LevelWarn, "Stopped early: ListenBrainz needs a token") + + return + } + job.logf(jobs.LevelInfo, "Filled in "+strconv.Itoa(total)+" artists") si.logger.Info("discography backfill complete", "artists", total)