Fix concurrency races, error handling, and lint warnings

internal/api/somafm_test.go

@@ -243,9 +243,7 @@ func TestNewSomaFMClient(t *testing.T) {
 
 	if client == nil {
 		t.Fatal("NewSomaFMClient() returned nil")
-	}
-
-	if client.client == nil {
+	} else if client.client == nil {
 		t.Error("NewSomaFMClient() client.client is nil")
 	}
 }

internal/cache/cache_test.go

@@ -247,14 +247,13 @@ func TestNewCache(t *testing.T) {
 
 	if cache == nil {
 		t.Fatal("NewCache() returned nil")
-	}
-
-	if cache.baseDir == "" {
-		t.Error("NewCache() cache.baseDir is empty")
-	}
-
-	if cache.expiry != DefaultExpiry {
-		t.Errorf("NewCache() cache.expiry = %v, want %v", cache.expiry, DefaultExpiry)
+	} else {
+		if cache.baseDir == "" {
+			t.Error("NewCache() cache.baseDir is empty")
+		}
+		if cache.expiry != DefaultExpiry {
+			t.Errorf("NewCache() cache.expiry = %v, want %v", cache.expiry, DefaultExpiry)
+		}
 	}
 }
 

internal/player/player.go

@@ -211,9 +211,9 @@ func (p *Player) initSpeaker(sampleRate beep.SampleRate) error {
 
 func (p *Player) Stop() {
 	p.mu.Lock()
-	defer p.mu.Unlock()
 
 	if p.cancelFunc == nil && !p.isPlaying {
+		p.mu.Unlock()
 		return
 	}
 
@@ -225,6 +225,9 @@ func (p *Player) Stop() {
 	speaker.Clear()
 	p.isPlaying = false
 	p.isPaused = false
+	p.mu.Unlock()
+
+	p.wg.Wait()
 
 	p.streamAliveMu.Lock()
 	p.streamAlive = false
@@ -1036,6 +1039,10 @@ func (p *Player) fetchAndParsePLS(ctx context.Context, plsURL string) ([]string,
 	}
 	defer resp.Body.Close()
 
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("PLS file returned status %d: %s", resp.StatusCode, resp.Status)
+	}
+
 	var urls []string
 	scanner := bufio.NewScanner(resp.Body)
 	for scanner.Scan() {

internal/service/station_service.go

@@ -180,19 +180,23 @@ func (s *StationService) GetCurrentTrackForStation(stationID string) (string, er
 }
 
 func (s *StationService) StartPeriodicRefresh(interval time.Duration, callback func([]station.Station)) {
+	s.StopPeriodicRefresh()
+
 	s.mu.Lock()
 	s.onRefresh = callback
 	s.stopRefresh = make(chan struct{})
 	s.refreshTicker = time.NewTicker(interval)
+	ticker := s.refreshTicker
+	stopCh := s.stopRefresh
 	s.mu.Unlock()
 
 	go func() {
 		for {
 			select {
-			case <-s.refreshTicker.C:
+			case <-ticker.C:
 				s.refreshStationsInBackground()
-			case <-s.stopRefresh:
-				s.refreshTicker.Stop()
+			case <-stopCh:
+				ticker.Stop()
 				return
 			}
 		}
@@ -230,5 +234,5 @@ func (s *StationService) refreshStationsInBackground() {
 		callback(newStations)
 	}
 
-	log.Debug().Int("count", len(s.stations)).Msg("Station data refreshed in background")
+	log.Debug().Int("count", len(newStations)).Msg("Station data refreshed in background")
 }

internal/service/station_service_test.go

@@ -212,13 +212,10 @@ func TestGetStation(t *testing.T) {
 				if result != nil {
 					t.Errorf("GetStation(%d) = %v, want nil", tt.index, result)
 				}
-			} else {
-				if result == nil {
-					t.Fatalf("GetStation(%d) = nil, want station", tt.index)
-				}
-				if result.ID != tt.expectedID {
-					t.Errorf("GetStation(%d).ID = %q, want %q", tt.index, result.ID, tt.expectedID)
-				}
+			} else if result == nil {
+				t.Fatalf("GetStation(%d) = nil, want station", tt.index)
+			} else if result.ID != tt.expectedID {
+				t.Errorf("GetStation(%d).ID = %q, want %q", tt.index, result.ID, tt.expectedID)
 			}
 		})
 	}

internal/ui/modals.go

@@ -57,6 +57,8 @@ func (ui *UI) showPlaybackErrorModal(message string) {
 		ui.pages.RemovePage("error-modal")
 		ui.app.SetFocus(ui.stationList)
 		if ui.currentStation != nil {
+			ui.safeCloseChannel()
+			ui.recreateStopChannel()
 			ui.startPlayingAnimation()
 			go func() {
 				err := ui.player.Play(ui.currentStation)

internal/ui/ui.go

@@ -447,10 +447,14 @@ func (ui *UI) createHeader() tview.Primitive {
 }
 
 func (ui *UI) updateLogoPanel(s *station.Station) {
+	stationID := s.ID
 	go func() {
 		img, err := ui.stationService.LoadImage(s.XLImage)
 		if err != nil {
 			ui.app.QueueUpdateDraw(func() {
+				if ui.currentStation == nil || ui.currentStation.ID != stationID {
+					return
+				}
 				ui.logoPanel.SetDrawFunc(func(screen tcell.Screen, x, y, width, height int) (int, int, int, int) {
 					errorMsg := fmt.Sprintf("Failed to load image: %v", err)
 					tview.Print(screen, errorMsg, x, y, 10, tview.AlignCenter, tcell.ColorRed)
@@ -461,6 +465,9 @@ func (ui *UI) updateLogoPanel(s *station.Station) {
 		}
 
 		ui.app.QueueUpdateDraw(func() {
+			if ui.currentStation == nil || ui.currentStation.ID != stationID {
+				return
+			}
 			ui.logoPanel.SetImage(img)
 		})
 	}()

internal/ui/ui_test.go

@@ -10,14 +10,13 @@ func TestNewPlayingSpinner(t *testing.T) {
 
 	if spinner == nil {
 		t.Fatal("NewPlayingSpinner() returned nil")
-	}
-
-	if len(spinner.Frames) == 0 {
-		t.Error("PlayingSpinner.Frames is empty")
-	}
-
-	if spinner.FPS <= 0 {
-		t.Error("PlayingSpinner.FPS should be positive")
+	} else {
+		if len(spinner.Frames) == 0 {
+			t.Error("PlayingSpinner.Frames is empty")
+		}
+		if spinner.FPS <= 0 {
+			t.Error("PlayingSpinner.FPS should be positive")
+		}
 	}
 }
 
@@ -322,17 +321,15 @@ func TestNewStatusRenderer(t *testing.T) {
 
 	if renderer == nil {
 		t.Fatal("NewStatusRenderer() returned nil")
-	}
-
-	if renderer.maxAnimFrame <= 0 {
-		t.Error("maxAnimFrame should be positive")
-	}
-
-	if renderer.ticksPerFrame <= 0 {
-		t.Error("ticksPerFrame should be positive")
-	}
-
-	if renderer.bufferTicksPerUpdate <= 0 {
-		t.Error("bufferTicksPerUpdate should be positive")
+	} else {
+		if renderer.maxAnimFrame <= 0 {
+			t.Error("maxAnimFrame should be positive")
+		}
+		if renderer.ticksPerFrame <= 0 {
+			t.Error("ticksPerFrame should be positive")
+		}
+		if renderer.bufferTicksPerUpdate <= 0 {
+			t.Error("bufferTicksPerUpdate should be positive")
+		}
 	}
 }