Skip to content

fix: list a station from the pages fetched before one fails - #427

Open
Rafa7046 wants to merge 1 commit into
devgianlu:masterfrom
Rafa7046:fix/list-station-first-page
Open

Rafa7046 wants to merge 1 commit into
devgianlu:masterfrom
Rafa7046:fix/list-station-first-page

Conversation

@Rafa7046

Copy link
Copy Markdown

Problem

GET /context/tracks?uri=spotify:station:track:<id> never becomes ready. It keeps answering ready: false, length: 0, cached: 0, while albums and single tracks list fine (metadata enabled).

Reproduction

  1. Call GET /context/tracks?uri=spotify:station:track:4cOdK2wGLETKBW3PvgPWqT and keep polling it.
  2. The response stays ready: false, length: 0 for good, and about every 2 s the daemon logs:
level=warning msg="failed enumerating context: spotify:station:track:4cOdK2wGLETKBW3PvgPWqT" error="failed fetching page 2: failed fetching next page: invalid status code from page at hm://radio-router/v3/tracks/spotify:station:track:4cOdK2wGLETKBW3PvgPWqT?...&count=50&prev_tracks=...: 404"

The page indices in that log start at 0. Pages 0 and 1 load; the failing request lists their 50 tracks as prev_tracks. Then the radio-router answers 404 for page 2. enumerateContextTracks treats any page error as fatal, so it throws away the tracks it already has. Nothing is cached, and the next poll starts the same enumeration again.

Fix

Stations get a narrow exception in enumerateContextTracks. A station is an endless radio context, so there is never a complete listing to wait for. The radio-router may also answer 404 for a later page, and when it does, the pages it already served are all there is. So for a spotify:station: context that has already yielded tracks, a page failure now ends the listing with the tracks fetched so far, the same way the end of the context or the page cap does.

Everything else keeps the current behaviour:

  • A station failure before any track was fetched is still an error, so an empty listing is never cached as ready.
  • A cancelled or timed-out enumeration (ctx.Err() != nil) is still an error, so a cut-off listing is never cached as the whole one.
  • Every other context type still fails the whole enumeration on any page error. A transient failure on a long playlist is retried, not cached as a short listing.

Only the context listing and its metadata sweep use this function. Autoplay and playback are unchanged.

Tests

  • TestEnumerateContextTracksKeepsStationPagesBeforeAFailure: a station's page 0 yields tracks and page 1 fails. Result: page 0's tracks, no error.
  • TestEnumerateContextTracksReportsStationErrorsBeforeAnyTrack: a station's page 0 is empty and page 1 fails. Still an error.
  • TestEnumerateContextTracksReportsStationCancellation: a station cancelled after a good page. Still an error.
  • TestEnumerateContextTracksReportsLaterPageErrors: a playlist whose page 1 fails still errors, as before.
  • TestEnumerateContextTracksReportsPageErrors (existing): a failing page 0 still errors.
  • TestContextTracksHandlerListsStationFirstPage: through the API handler, a station whose page 1 fails answers ready: true with length == cached == the count from page 0. Without the fix it stays not-ready.

go test -tags "test_unit test_integration" ./..., go vet and gofmt -l . are clean.

GET /context/tracks never became ready for a spotify:station:track:<id>
context: the enumeration got the station's opening tracks, then the
radio-router answered 404 for a later page, so it failed, discarded what
it had and was retried on every poll, forever.

A station is endless, so there is no complete listing to wait for: once it
yielded tracks, a page failure now ends the listing with the pages before
it. A failure before any track and a cancelled enumeration stay errors, and
every other context still fails the whole enumeration on any page, so a
transient error on a long playlist is retried rather than cached as a short
listing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant