diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 8d18b96..a60313b 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -72,7 +72,6 @@ changelog: exclude: - "^docs:" - "^test:" - - "^chore:" - "Merge pull request" - "Merge branch" diff --git a/install.sh b/install.sh index 204917d..28a2587 100644 --- a/install.sh +++ b/install.sh @@ -27,7 +27,7 @@ warn() { } error() { - printf "${RED}[ERROR]${NC} %s\n" "$1" + printf "${RED}[ERROR]${NC} %s\n" "$1" >&2 exit 1 } @@ -63,7 +63,7 @@ verify_checksum() { info "Verifying checksum..." - CHECKSUMS=$(curl -sSL "${CHECKSUM_URL}") || { + CHECKSUMS=$(curl -fsSL "${CHECKSUM_URL}") || { warn "Could not download checksums, skipping verification" return 0 } @@ -92,8 +92,8 @@ verify_checksum() { # Download and install install() { - OS=$(detect_os) - ARCH=$(detect_arch) + OS=$(detect_os) || exit 1 + ARCH=$(detect_arch) || exit 1 info "Detected OS: ${OS}, Arch: ${ARCH}" @@ -120,10 +120,10 @@ install() { # Create temp directory TMP_DIR=$(mktemp -d) - trap "rm -rf ${TMP_DIR}" EXIT + trap "rm -rf ${TMP_DIR}" EXIT INT TERM # Download - curl -sSL "${DOWNLOAD_URL}" -o "${TMP_DIR}/${FILENAME}" + curl -fsSL "${DOWNLOAD_URL}" -o "${TMP_DIR}/${FILENAME}" # Verify checksum cd "${TMP_DIR}" @@ -137,6 +137,15 @@ install() { fi # Install + if [ ! -d "${INSTALL_DIR}" ]; then + info "Creating ${INSTALL_DIR}" + if [ -w "$(dirname "${INSTALL_DIR}")" ]; then + mkdir -p "${INSTALL_DIR}" + else + sudo mkdir -p "${INSTALL_DIR}" + fi + fi + if [ -w "${INSTALL_DIR}" ]; then mv "${BINARY_NAME}" "${INSTALL_DIR}/${BINARY_NAME}" else @@ -152,7 +161,10 @@ install() { if "${INSTALL_DIR}/${BINARY_NAME}" --version >/dev/null 2>&1; then info "Verified: $(${INSTALL_DIR}/${BINARY_NAME} --version | head -1)" else - warn "Binary installed but verification failed" + warn "Binary installed but could not run it" + if [ "$OS" = "linux" ]; then + warn "Linux requires libasound2 (ALSA). Install it with: sudo apt-get install libasound2" + fi fi } diff --git a/internal/config/config.go b/internal/config/config.go index 424e317..d07495d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/gdamore/tcell/v2" "gopkg.in/yaml.v3" @@ -65,6 +66,8 @@ type Config struct { Autostart bool `yaml:"autostart"` Favorites []string `yaml:"favorites"` Theme Theme `yaml:"theme"` + + saveMu sync.Mutex `yaml:"-"` } func GetConfigPath() (string, error) { @@ -104,6 +107,9 @@ func Load() (*Config, error) { // Save writes the configuration to disk atomically using temp file + rename. func (c *Config) Save() error { + c.saveMu.Lock() + defer c.saveMu.Unlock() + configPath, err := GetConfigPath() if err != nil { return err diff --git a/internal/player/player.go b/internal/player/player.go index f8f6ba4..bdf4074 100644 --- a/internal/player/player.go +++ b/internal/player/player.go @@ -650,13 +650,24 @@ func (p *Player) reconnectWithRotation(s *station.Station, streamURLs []string, return fmt.Errorf("reconnection failed: %w", lastErr) } +type httpStatusError struct { + StatusCode int + Status string +} + +func (e *httpStatusError) Error() string { + return fmt.Sprintf("stream returned status %d: %s", e.StatusCode, e.Status) +} + func isNonRetryableError(err error) bool { - errStr := err.Error() - // HTTP errors that won't change with retry on this specific URL - return strings.Contains(errStr, "status 401") || - strings.Contains(errStr, "status 403") || - strings.Contains(errStr, "status 404") || - strings.Contains(errStr, "status 410") + var statusErr *httpStatusError + if errors.As(err, &statusErr) { + switch statusErr.StatusCode { + case 401, 403, 404, 410: + return true + } + } + return false } func (p *Player) playStreamURL(ctx context.Context, s *station.Station, streamURL string) error { @@ -681,7 +692,7 @@ func (p *Player) playStreamURL(ctx context.Context, s *station.Station, streamUR if resp.StatusCode != http.StatusOK { resp.Body.Close() - return fmt.Errorf("stream returned status %d: %s", resp.StatusCode, resp.Status) + return &httpStatusError{StatusCode: resp.StatusCode, Status: resp.Status} } var icyMetaint int @@ -869,6 +880,15 @@ func (p *Player) readNetworkStream(ctx context.Context, respBody io.ReadCloser, } metaLen := int(metaLenByte) * 16 + if metaLen > 4080 { + log.Warn().Int("metaLen", metaLen).Msg("ICY metadata too large, skipping") + if _, err := io.CopyN(io.Discard, bufReader, int64(metaLen)); err != nil { + if ctx.Err() != nil { + return + } + } + continue + } if metaLen > 0 { metaData := make([]byte, metaLen) n, err := io.ReadFull(bufReader, metaData) @@ -1083,12 +1103,12 @@ func parseStreamInfoFromURL(url string) StreamInfo { info.Format = "AAC" } - bitrates := []int{320, 256, 192, 130, 128, 64, 32} - for _, br := range bitrates { + // SomaFM URL convention: groovesalad130.pls = MP3 128kbps (130 is an internal ID, not actual bitrate) + for _, br := range []int{320, 256, 192, 130, 128, 64, 32} { brStr := fmt.Sprintf("%d", br) if strings.Contains(url, brStr+".pls") || strings.Contains(url, brStr+".") { info.Bitrate = br - if br == 130 { // SomaFM uses 130 for 128kbps streams + if br == 130 { info.Bitrate = 128 } break diff --git a/internal/player/player_test.go b/internal/player/player_test.go index 92ecede..cb47af5 100644 --- a/internal/player/player_test.go +++ b/internal/player/player_test.go @@ -73,10 +73,6 @@ func TestPlayerStateString(t *testing.T) { func TestNewPlayer(t *testing.T) { p := NewPlayer() - if p == nil { - t.Fatal("NewPlayer returned nil") - } - if p.isPlaying { t.Error("New player should not be playing") } @@ -88,28 +84,39 @@ func TestNewPlayer(t *testing.T) { func TestIsNonRetryableError(t *testing.T) { tests := []struct { + name string err error expected bool }{ - {errors.New("status 401"), true}, - {errors.New("status 403"), true}, - {errors.New("status 404"), true}, - {errors.New("status 410"), true}, - {errors.New("status 500"), false}, - {errors.New("connection refused"), false}, - {errors.New("timeout"), false}, + {"401", &httpStatusError{StatusCode: 401, Status: "Unauthorized"}, true}, + {"403", &httpStatusError{StatusCode: 403, Status: "Forbidden"}, true}, + {"404", &httpStatusError{StatusCode: 404, Status: "Not Found"}, true}, + {"410", &httpStatusError{StatusCode: 410, Status: "Gone"}, true}, + {"500", &httpStatusError{StatusCode: 500, Status: "Internal Server Error"}, false}, + {"503", &httpStatusError{StatusCode: 503, Status: "Service Unavailable"}, false}, + {"wrapped 404", fmt.Errorf("stream failed: %w", &httpStatusError{StatusCode: 404}), true}, + {"generic error", errors.New("connection refused"), false}, + {"timeout", errors.New("timeout"), false}, } for _, tt := range tests { - t.Run(tt.err.Error(), func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { result := isNonRetryableError(tt.err) if result != tt.expected { - t.Errorf("isNonRetryableError(%q) = %v, want %v", tt.err, result, tt.expected) + t.Errorf("isNonRetryableError(%v) = %v, want %v", tt.err, result, tt.expected) } }) } } +func TestHttpStatusErrorMessage(t *testing.T) { + err := &httpStatusError{StatusCode: 404, Status: "404 Not Found"} + expected := "stream returned status 404: 404 Not Found" + if err.Error() != expected { + t.Errorf("got %q, want %q", err.Error(), expected) + } +} + func TestParseStreamInfoFromURL(t *testing.T) { tests := []struct { url string