refactor(provider): return reporting failures instead of discarding them

PlaybackReporter and ProgressReporter now return an error. The UI logs it at
the three fire-and-forget call sites, so jellyfin, emby, and navidrome gain
the observability audiobookshelf had — they were dropping their client errors
silently. Player state is read on the UI goroutine and passed into the
closures, so the reports stay race-free.

docs/provider-development.md

@@ -40,7 +40,7 @@ interfaces are defined in `provider/interfaces.go`.
 | `ArtistBrowser` | Hierarchical artist browsing | `Artists()`, `ArtistAlbums(id)` |
 | `AlbumBrowser` | Paginated album browsing with sort | `AlbumList(sort, offset, size)`, `AlbumSortTypes()` |
 | `AlbumTrackLoader` | Album track listing | `AlbumTracks(albumID)` |
-| `Scrobbler` | Playback reporting | `Scrobble(track, submission)` |
+| `PlaybackReporter` | Playback reporting at track start and finish | `CanReportPlayback(track)`, `ReportNowPlaying(track, position, canSeek) error`, `ReportScrobble(track, elapsed, duration, canSeek) error` |
 | `PlaylistWriter` | Add track to playlist | `AddTrackToPlaylist(ctx, playlistID, track)` |
 | `PlaylistCreator` | Create new playlist | `CreatePlaylist(ctx, name)` |
 | `PlaylistDeleter` | Remove playlists/tracks | `DeletePlaylist(name)`, `RemoveTrack(name, index)` |
@@ -49,7 +49,7 @@ interfaces are defined in `provider/interfaces.go`.
 | `Closer` | Cleanup on shutdown | `Close()` |
 | `Authenticator` | Interactive sign-in flow | `Authenticate() error` (in `playlist` package) |
 | `ResumeTarget` | Server-side resume position | `ResumeTarget(playlistID, tracks)` |
-| `ProgressReporter` | Interim position updates while playing, in addition to `PlaybackReporter`'s start/finish reports | `ReportProgress(track, position)` |
+| `ProgressReporter` | Interim position updates while playing, in addition to `PlaybackReporter`'s start/finish reports | `ReportProgress(track, position) error` |
 | `BrowseLabeler` | Relabel the browse overlay's two levels (e.g. Authors/Books instead of Artists/Albums) | `BrowseLabels()` |
 
 ## Steps
@@ -190,7 +190,7 @@ implements, the UI will automatically:
 - Show the browse overlay ("N") if any registered provider implements `ArtistBrowser` or `AlbumBrowser`
 - Show the search overlay ("F") if any registered provider implements `Searcher`
 - Enable add-to-playlist in search results if the searched provider implements `PlaylistWriter`
-- Scrobble playback if `Scrobbler` is implemented
+- Report playback at track start and finish if `PlaybackReporter` is implemented, logging any failure the provider returns
 - Run interactive auth on first use if `Authenticator` is implemented
 - Place the cursor on the in-progress track and start it at the stored position if `ResumeTarget` is implemented
 - Push an interim listening position every 15 seconds while a track plays if `ProgressReporter` is implemented

external/audiobookshelf/provider.go

@@ -9,7 +9,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/bjarneo/cliamp/applog"
 	"github.com/bjarneo/cliamp/config"
 	"github.com/bjarneo/cliamp/playlist"
 	"github.com/bjarneo/cliamp/provider"
@@ -501,26 +500,26 @@ func (p *Provider) CanReportPlayback(track playlist.Track) bool {
 	return track.Meta(provider.MetaAudiobookshelfID) != ""
 }
 
-func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, _ bool) {
+func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, _ bool) error {
 	if position <= 0 {
-		return
+		return nil
 	}
-	p.report(track, position, false)
+	return p.report(track, position, false)
 }
 
-func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, _ bool) {
-	p.report(track, elapsed, true)
+func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, _ bool) error {
+	return p.report(track, elapsed, true)
 }
 
 // ReportProgress pushes an interim listening position.
-func (p *Provider) ReportProgress(track playlist.Track, position time.Duration) {
-	p.report(track, position, false)
+func (p *Provider) ReportProgress(track playlist.Track, position time.Duration) error {
+	return p.report(track, position, false)
 }
 
-func (p *Provider) report(track playlist.Track, position time.Duration, complete bool) {
+func (p *Provider) report(track playlist.Track, position time.Duration, complete bool) error {
 	itemID := track.Meta(provider.MetaAudiobookshelfID)
 	if itemID == "" {
-		return
+		return nil
 	}
 	offset := metaFloat(track, provider.MetaAudiobookshelfOffset)
 	total := metaFloat(track, provider.MetaAudiobookshelfTotal)
@@ -531,8 +530,9 @@ func (p *Provider) report(track playlist.Track, position time.Duration, complete
 	finished := complete && total > 0 && current >= total-finishSlack
 	episodeID := track.Meta(provider.MetaAudiobookshelfEpisode)
 	if err := p.client.UpdateProgress(itemID, episodeID, current, total, finished); err != nil {
-		applog.Warn("audiobookshelf: progress update failed for item %s episode %q: %v", itemID, episodeID, err)
+		return fmt.Errorf("audiobookshelf: update progress for item %s: %w", itemID, err)
 	}
+	return nil
 }
 
 // ResumeTarget returns where to continue an item: the track index and the

external/audiobookshelf/provider_test.go

@@ -827,3 +827,30 @@ func TestSearchTracksHonoursCancellation(t *testing.T) {
 		})
 	}
 }
+
+func TestReportProgressReturnsTheFailure(t *testing.T) {
+	p := mockProvider(func(req *http.Request) (*http.Response, error) {
+		return statusResponse(http.StatusNotFound, "404 Not Found"), nil
+	})
+
+	track := playlist.Track{
+		DurationSecs: 3600,
+		ProviderMeta: map[string]string{
+			provider.MetaAudiobookshelfID:    "book-1",
+			provider.MetaAudiobookshelfTotal: "7200",
+		},
+	}
+
+	err := p.ReportProgress(track, 120*time.Second)
+	if err == nil {
+		t.Fatal("ReportProgress() error = nil, want the rejected write surfaced to the caller")
+	}
+	if !strings.Contains(err.Error(), "book-1") || !strings.Contains(err.Error(), "404") {
+		t.Fatalf("error = %v, want the item id and the status", err)
+	}
+
+	// A track this provider does not own is not an error.
+	if err := p.ReportProgress(playlist.Track{}, 120*time.Second); err != nil {
+		t.Fatalf("ReportProgress() for a foreign track = %v, want nil", err)
+	}
+}

external/emby/provider.go

@@ -94,12 +94,12 @@ func (p *Provider) CanReportPlayback(track playlist.Track) bool {
 	return track.Meta(provider.MetaEmbyID) != ""
 }
 
-func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool) {
-	_ = p.client.ReportNowPlaying(track, position, canSeek)
+func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool) error {
+	return p.client.ReportNowPlaying(track, position, canSeek)
 }
 
-func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, canSeek bool) {
-	_ = p.client.ReportScrobble(track, elapsed, canSeek)
+func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, canSeek bool) error {
+	return p.client.ReportScrobble(track, elapsed, canSeek)
 }
 
 // Playlists returns all albums across all Emby music views.

external/jellyfin/provider.go

@@ -82,12 +82,12 @@ func (p *Provider) CanReportPlayback(track playlist.Track) bool {
 	return track.Meta(provider.MetaJellyfinID) != ""
 }
 
-func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool) {
-	_ = p.client.ReportNowPlaying(track, position, canSeek)
+func (p *Provider) ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool) error {
+	return p.client.ReportNowPlaying(track, position, canSeek)
 }
 
-func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, canSeek bool) {
-	_ = p.client.ReportScrobble(track, elapsed, canSeek)
+func (p *Provider) ReportScrobble(track playlist.Track, elapsed, _ time.Duration, canSeek bool) error {
+	return p.client.ReportScrobble(track, elapsed, canSeek)
 }
 
 // Playlists returns all albums across all Jellyfin music views.

external/navidrome/client.go

@@ -514,21 +514,21 @@ func (c *NavidromeClient) CanReportPlayback(track playlist.Track) bool {
 	return !c.scrobbleDisabled && track.Meta(provider.MetaNavidromeID) != ""
 }
 
-func (c *NavidromeClient) ReportNowPlaying(track playlist.Track, _ time.Duration, _ bool) {
-	c.scrobble(track.Meta(provider.MetaNavidromeID), false)
+func (c *NavidromeClient) ReportNowPlaying(track playlist.Track, _ time.Duration, _ bool) error {
+	return c.scrobble(track.Meta(provider.MetaNavidromeID), false)
 }
 
-func (c *NavidromeClient) ReportScrobble(track playlist.Track, _, _ time.Duration, _ bool) {
-	c.scrobble(track.Meta(provider.MetaNavidromeID), true)
+func (c *NavidromeClient) ReportScrobble(track playlist.Track, _, _ time.Duration, _ bool) error {
+	return c.scrobble(track.Meta(provider.MetaNavidromeID), true)
 }
 
 // scrobble reports playback of a track to the Subsonic server.
 // If submission is false, it registers a "now playing" notification only.
 // If submission is true, it records a full play (updates play count, last.fm, etc.).
-// The call is best-effort: errors are silently discarded.
-func (c *NavidromeClient) scrobble(id string, submission bool) {
+// The call is best-effort: the error is returned for logging, never acted on.
+func (c *NavidromeClient) scrobble(id string, submission bool) error {
 	if id == "" {
-		return
+		return nil
 	}
 	params := url.Values{
 		"id":         {id},
@@ -541,7 +541,8 @@ func (c *NavidromeClient) scrobble(id string, submission bool) {
 	}
 	resp, err := httpClient.Get(c.buildURL("scrobble", params))
 	if err != nil {
-		return // fire-and-forget; ignore network errors
+		return fmt.Errorf("navidrome: scrobble: %w", err)
 	}
 	resp.Body.Close()
+	return nil
 }

provider/interfaces.go

@@ -44,8 +44,10 @@ type AlbumTrackLoader interface {
 // playback-completion reports for tracks they originated.
 type PlaybackReporter interface {
 	CanReportPlayback(track playlist.Track) bool
-	ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool)
-	ReportScrobble(track playlist.Track, elapsed, duration time.Duration, canSeek bool)
+	// ReportNowPlaying and ReportScrobble return the failure so the caller can
+	// record it; reporting is best-effort and never blocks playback.
+	ReportNowPlaying(track playlist.Track, position time.Duration, canSeek bool) error
+	ReportScrobble(track playlist.Track, elapsed, duration time.Duration, canSeek bool) error
 }
 
 // ProgressReporter is implemented by providers that track listening position
@@ -54,7 +56,7 @@ type PlaybackReporter interface {
 type ProgressReporter interface {
 	PlaybackReporter
 	// ReportProgress sends an interim position update for a playing track.
-	ReportProgress(track playlist.Track, position time.Duration)
+	ReportProgress(track playlist.Track, position time.Duration) error
 }
 
 // ResumeTarget is implemented by providers that track listening position

ui/model/audiobookshelf_resume_test.go

@@ -148,12 +148,15 @@ type progressProv struct {
 
 func (p *progressProv) CanReportPlayback(playlist.Track) bool { return true }
 
-func (p *progressProv) ReportNowPlaying(playlist.Track, time.Duration, bool) {}
+func (p *progressProv) ReportNowPlaying(playlist.Track, time.Duration, bool) error { return nil }
 
-func (p *progressProv) ReportScrobble(playlist.Track, time.Duration, time.Duration, bool) {}
+func (p *progressProv) ReportScrobble(playlist.Track, time.Duration, time.Duration, bool) error {
+	return nil
+}
 
-func (p *progressProv) ReportProgress(_ playlist.Track, position time.Duration) {
+func (p *progressProv) ReportProgress(_ playlist.Track, position time.Duration) error {
 	p.reports <- position
+	return nil
 }
 
 func TestTickProgressReportThrottles(t *testing.T) {

ui/model/notifications.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/bjarneo/cliamp/applog"
 	"github.com/bjarneo/cliamp/internal/playback"
 	"github.com/bjarneo/cliamp/luaplugin"
 	"github.com/bjarneo/cliamp/playlist"
@@ -115,7 +116,12 @@ func (m *Model) nowPlaying(track playlist.Track) {
 		return
 	}
 	canSeek := m.player.Seekable()
-	go reporter.ReportNowPlaying(track, m.player.Position(), canSeek)
+	position := m.player.Position()
+	go func() {
+		if err := reporter.ReportNowPlaying(track, position, canSeek); err != nil {
+			applog.Warn("now-playing report failed for %q: %v", track.Title, err)
+		}
+	}()
 }
 
 // maybeScrobble fires a playback-complete report for the given track if all
@@ -163,7 +169,11 @@ func (m *Model) maybeScrobble(track playlist.Track, elapsed, duration time.Durat
 		return // less than 50% played
 	}
 	canSeek := m.player.Seekable()
-	go reporter.ReportScrobble(track, elapsed, duration, canSeek)
+	go func() {
+		if err := reporter.ReportScrobble(track, elapsed, duration, canSeek); err != nil {
+			applog.Warn("scrobble failed for %q: %v", track.Title, err)
+		}
+	}()
 }
 
 // findPlaybackReporter returns the first registered provider that can report
@@ -213,5 +223,10 @@ func (m *Model) tickProgressReport(now time.Time) {
 		return
 	}
 	m.lastProgressReport = now
-	go reporter.ReportProgress(track, m.player.Position())
+	position := m.player.Position()
+	go func() {
+		if err := reporter.ReportProgress(track, position); err != nil {
+			applog.Warn("progress report failed for %q: %v", track.Title, err)
+		}
+	}()
 }