diff --git a/ISSUES.md b/ISSUES.md new file mode 100644 index 0000000..9a0f432 --- /dev/null +++ b/ISSUES.md @@ -0,0 +1,240 @@ +# GoStations Issues + +**Project:** gostations (console radio station browser/player) +**Analysis date:** 2026-06-05 +**Scope:** Focused review of obvious bugs, crash paths, error handling failures, and high-impact correctness issues. Performed via source inspection, test runs (`go test`), live execution, and forced error-path reproduction (bad DNS, connection failures, config edge cases). + +This document captures findings from the current source tree. The existing `build/gostations` binary is stale (different build paths in debug info) — always rebuild from current sources for testing. + +--- + +## Critical Bugs (Panics and Hard Crashes) + +### 1. Nil pointer dereference on `http.Get` failure (most common network error path) + +**Location:** `browser.go:50` (GetStations) + +```go +resp, err := http.Get(urlstr) +if err != nil { + log.Print(err.Error()) +} +defer func(Body io.ReadCloser) { + err := Body.Close() + ... +}(resp.Body) // <--- resp is nil on most errors +... +err = json.NewDecoder(resp.Body).Decode(&data) +``` + +**Impact:** Any transient or permanent failure contacting the radio-browser API (DNS, connect refused, timeout, etc.) causes an immediate panic instead of graceful degradation. + +**Reproduction:** +- Set `radio_browser.api=localhost` (or any host that resolves but refuses HTTPS on 443) in a temp `radiostations.ini` under `XDG_CONFIG_HOME`. +- Run `gostations -c "Gambia"`. +- Observed: `panic: runtime error: invalid memory address or nil pointer dereference` at the `defer` evaluation line. + +**Related:** No `http.Client` with timeout/context is used anywhere. Searches can hang indefinitely. + +### 2. Panic in API host selection on DNS failure or empty result + +**Location:** `browser.go:25-38` (RandomIP, nslookup, reverseLookup, GetApiHost) + +```go +func RandomIP(iplist []net.IP) net.IP { + rand.NewSource(time.Now().Unix()) // result discarded! + randomIndex := rand.Intn(len(iplist)) // panics if len==0 + ... +} + +func nslookup(hostname string) net.IP { + iprecords, _ := net.LookupIP(hostname) // error ignored + return RandomIP(iprecords) +} + +func reverseLookup(ipAddr net.IP) string { + ptr, _ := net.LookupAddr(ipAddr.String()) + return ptr[0] // panics if len==0 +} +``` + +**Impact:** `GetApiHost()` is called on **every** `StationSearch` (before the actual station query). Failure here kills the entire program with either: +- `panic: invalid argument to Intn` +- index-out-of-range panic + +**Reproduction:** Set `radio_browser.api=nonexistent.invalid.host.example` → full panic stack through `StationSearch` → `main`. + +**Additional problems:** +- The `NewSource` call is dead code (return value thrown away; does not affect the global RNG used by `Intn`). In Go 1.14-era this made "random" API host selection deterministic. Modern autoseeding hides the bug but the intent is broken. +- No caching of the chosen API host (DNS + reverse lookup repeated for every search). +- `GetStations` is called with the result of `GetApiHost()` with zero defensive coding. + +### 3. `subExecute` always returns an error on success + produces garbage output + +**Location:** `commander.go:28-38` + +```go +func subExecute(program string, args ...string) ([]byte, error) { + cmd := exec.Command(program, args...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + if err != nil { + fmt.Printf("%v\n", err) + } + return cmd.CombinedOutput() // !!! +} +``` + +**Impact:** +- `CombinedOutput()` sees `Stdout` already set → returns `nil, errors.New("exec: Stdout already set")`. +- Callers ignore the error: `stdout, _ := subExecute(...)`; `fmt.Println(stdout)`. +- After every successful play (mpv exits cleanly on 'q'), the user sees a stray `[]` line, then the menu reappears. +- This is the origin of the `[]` visible in the README examples after "Exiting... (Quit)". + +**Test failure:** `TestSubExecute_Live` (under `-short`) fails with exactly this error. + +**Usage site:** `radiomenu.go:32` (inside the wmenu action callback, after the blocking play). + +### 4. `createIniFile` continues writing after failures (fragile, relies on runtime luck) + +**Location:** `filer.go:10-46` + +- Records `MkdirAll` / `Create` errors but proceeds unconditionally to `file.Write`. +- On failure, `file` is `nil` (documented `os` behavior). +- Relies on `*os.File` methods returning errors instead of panicking when called on `nil` (observed behavior in current Go; not guaranteed or clean). +- Defer for `Close` is registered even on failure paths. +- Multiple "invalid argument" errors get appended for every subsequent write. + +**Impact:** Auto-generation of `radiostations.ini` on first run (or missing file) is not robust. `configStat` then does `log.Fatal` on the first error in the list (with a typo: "Erorr"). + +**Test note:** The happy-path tests pass because they exercise success cases only. Error-collection tests would expose the mess. + +--- + +## Error Handling, Correctness, and Robustness Issues + +### 5. Widespread ignored errors + lossy error paths + +- `stations.go:61`: `stations, _ := StationSearch(...)` — search errors are silently dropped. Main proceeds with potentially `nil` slice. +- `browser.go:94-102` (StationSearch): On any `GetStations` error the data is discarded and `nil` is returned. On `notok` path, `err` is returned even when it is `nil`. +- `browser.go:62-66` (GetStations): `err = json...` overwrites any prior `http.Get` error variable; partial data + last `err` is returned in some cases. +- `config.go:75,80,85,90`: All the convenience wrappers (`api()`, `player()`, `options()`, `maxitems()`) discard `Config` errors. +- Lookups in `nslookup`/`reverseLookup` discard errors. +- `radiomenu.go:35`: `log.Fatal` inside a menu action callback. + +### 6. Misleading / broken config error message + +**Location:** `config.go:69` + +```go +if optval == "" { + return "", errors.New("no value for option '%s'") // %s is never formatted +} +``` + +Always returns the literal string containing `'%s'`. Reproduced via temp ini missing expected keys. + +### 7. Command injection risk (Unix `isInstalled`) + +**Location:** `commander.go:19` + +```go +cmd = exec.Command("/bin/sh", "-c", "command -v "+name) +``` + +`name` comes from the user-editable `player.command` value in `radiostations.ini`. A malicious or accidentally malformed value executes arbitrary shell commands during the precheck. + +(`subExecute` itself uses `exec.Command(program, ...)` directly and is safer for the player step, but the check is not.) + +### 8. Inverted test logic for "live" tests + missing coverage + +- `*_Live` tests in `*_test.go` files only run when `-short` is passed (the `if !testing.Short() { t.Skip }` guard is backwards). +- `browser_test.go` is essentially empty. +- No tests exercise the panic paths in `GetStations`, `RandomIP`, `nslookup`, config missing-option, `createIniFile` failure, etc. +- `TestPrecheck_Live` etc. rely on real global state (player present in PATH, config file creation). + +### 9. Other correctness / quality issues + +- `stations.go:33-40`: Custom `flag.Usage` + `flag.PrintDefaults()` produces slightly mangled help (tabs immediately after bool flags like `-vShow...`). +- `main` has a zero-arg special case for usage, but `-h` is handled by the `flag` package (works by accident). No explicit help flag handling. +- `radiomenu.go:34`: Recursive `menu.Run()` inside the selection action after playback (potential for deep stacks on repeated listen/return cycles; state of the wmenu instance is reused in a non-obvious way). +- `config.go:52`: `configparser.Delimiter = "="` is mutated globally on every `Config()` call. +- `filer.go` + `config.go`: Manual string concatenation for paths (`+ "/gostations/" + ...`) instead of `filepath.Join`. `configStat` also hard-codes the `gostations` subdirectory name. +- `str2int` (config.go:14) returns 9999 on any `Atoi` error (including overflow on 32-bit platforms). Test expects `4294967296` which only works on 64-bit `int`. +- `GetStations` URL: `?...&limit=...` when `params.Encode() == ""` produces a leading `?&`. +- No version information is embedded at runtime except via fragile `-ldflags "-X main.version=..."` (the `var version string` at package level). +- Heavy use of `log.Print`/`log.Fatal` mixes diagnostic output with user-facing behavior and aborts without cleanup. + +--- + +## Non-Bug Recommendations / Improvement Opportunities + +- **Caching & performance:** Resolve the radio-browser API host once and reuse it (or fall back to a static list of known endpoints). The current DNS dance is expensive and repeated. +- **HTTP client:** Use a single `*http.Client` with reasonable `Timeout` and context support. +- **Player execution:** Rewrite or replace `subExecute`. For long-running players the current "live + capture" approach is fundamentally conflicted. Consider just letting the player inherit stdio and not trying to capture/return anything after it exits. +- **Menu restart:** After playback, either restart the menu cleanly (new `RadioMenu` call) or use wmenu's built-in looping/return mechanisms instead of recursion + `os.Exit(0)` from inside an action. +- **Config handling:** Perform validation once at startup (as noted in the existing README TODO). Cache the parsed config. Use `fmt.Errorf` with proper verbs. Provide a `--config` flag or at least clear diagnostics when the ini is invalid/missing keys. +- **Error strategy:** Return errors up to `main`, print a user-friendly message, and exit with non-zero status. Avoid `log.Fatal` for normal control flow. +- **Randomness:** Fix `RandomIP` to actually use a per-call `*rand.Rand` if the goal is to pick among A records. +- **Testing:** Add table-driven tests (or httptest) for the error paths in browser.go. Fix the `-short` guard logic or rename the tests. Add a build tag or separate integration suite. +- **Modernization:** + - Stop using vendoring (or at least stop committing the entire `vendor/` tree for a small CLI). + - Update ancient dependencies (testify v1.4.0, configparser from 2019, etc.). + - Use `//go:embed` or `debug.ReadBuildInfo` for version instead of ldflags + global. + - Use `os.UserConfigDir()` + `filepath.Join` properly (with fallback for the old XDG logic if desired). +- **Security / robustness:** Validate/sanitize the player command (or at least document that the ini must be trusted). Add a `--player` override flag. +- **UX:** Truncation at 40 chars is aggressive on modern terminals. Consider making the menu nicer (the existing TODO mentions color/dashboard). +- **Existing README TODO items** (still relevant): + - "Change the precheck, to do ini file validation once, rather than every time a config value is called for." + - "Add a way to capture favorite selections." + - "Add color or a dashboard to the menu and player." + +--- + +## How to Reproduce Key Issues Quickly + +1. **DNS / host selection panic:** + ```sh + mkdir -p /tmp/badgost/gostations + cat > /tmp/badgost/gostations/radiostations.ini < 0 { + for _, e := range errs { + if e != nil { + log.Printf("Error creating default config: %s", e.Error()) + } + } + log.Fatal("Cannot continue without a config file.") + } + } + return configFile +} + +func createIniFile(fpath string) []error { + log.Printf("Creating config file: %s\n", fpath) + var errorlist []error + if err := os.MkdirAll(filepath.Dir(fpath), 0770); err != nil { + errorlist = append(errorlist, err) + } + + file, err := os.Create(fpath) + if err != nil { + errorlist = append(errorlist, err) + } else { + // only write if file was created + writes := []string{ + "[DEFAULT]\n", + "radio_browser.api=all.api.radio-browser.info\n", + "player.command=mpv\n", + "player.options=--no-video\n", + "menu_items.max=9999\n", + } + for _, w := range writes { + if _, err := file.Write([]byte(w)); err != nil { + errorlist = append(errorlist, err) + } + } + if cerr := file.Close(); cerr != nil { + errorlist = append(errorlist, cerr) + } + } + + return errorlist +} + +func (c *Config) load() error { + configparser.Delimiter = "=" // set once + cfg, err := configparser.Read(c.path) + if err != nil { + return fmt.Errorf("read config %s: %w", c.path, err) + } + sec, err := cfg.Section("DEFAULT") + if err != nil { + return fmt.Errorf("find DEFAULT section in %s: %w", c.path, err) + } + c.section = sec + return nil +} + +// Get returns the raw string value for key, or error if missing/empty. +func Get(key string) (string, error) { + if err := Init(); err != nil { + return "", err + } + if defaultConfig.section == nil { + return "", errors.New("config not loaded") + } + val := defaultConfig.section.Options()[key] + if val == "" { + return "", fmt.Errorf("no value for option %q in config", key) + } + return val, nil +} + +// MustGet is like Get but fatals on error (for precheck paths only). +func MustGet(key string) string { + v, err := Get(key) + if err != nil { + log.Fatalf("config error for %s: %v", key, err) + } + return v +} + +func str2int(s string) int { + i, err := strconv.Atoi(s) + if err != nil { + return 9999 + } + return i +} + +// API returns the radio browser API host (or default on error for compat). +func API() string { + if v, err := Get("radio_browser.api"); err == nil && v != "" { + return v + } + return "all.api.radio-browser.info" +} + +// Player returns the player command. +func Player() string { + if v, err := Get("player.command"); err == nil && v != "" { + return v + } + return "mpv" +} + +// Options returns extra player options string. +func Options() string { + v, _ := Get("player.options") + return v +} + +// MaxItems returns the menu limit. +func MaxItems() int { + if v, err := Get("menu_items.max"); err == nil && v != "" { + return str2int(v) + } + return 9999 +} + +// Path returns the resolved config file path (for diagnostics). +func Path() string { + if defaultConfig != nil { + return defaultConfig.path + } + return configStat("radiostations.ini") +} diff --git a/internal/player/player.go b/internal/player/player.go new file mode 100644 index 0000000..9f6ba27 --- /dev/null +++ b/internal/player/player.go @@ -0,0 +1,91 @@ +package player + +import ( + "fmt" + "os" + "os/exec" + "runtime" + "strings" +) + +// Player abstracts execution of a media player for a stream URL. +// Implementations may use exec (legacy) or IPC (preferred for mpv). +type Player interface { + // Play starts playback of the given URL. For long-running players this blocks + // until the player exits (or returns immediately for background/IPC impls). + Play(url string, extraArgs ...string) error + Stop() error // best effort +} + +// IsInstalled reports whether the named executable is on PATH. +// Fixed to avoid shell injection (no "/bin/sh -c 'command -v ' + name"). +func IsInstalled(name string) bool { + if name == "" { + return false + } + // Direct lookpath is best; fall back to exec the name with a harmless arg. + if _, err := exec.LookPath(name); err == nil { + return true + } + // Some players may be scripts without direct lookpath in certain envs; try exec --version like. + cmd := exec.Command(name, "--version") + _ = cmd.Run() // ignore output/err; if it started at all, likely present + // More reliable: use LookPath on common variants? For now LookPath is primary. + return false +} + +// NewLegacy returns a simple exec-based player (stdio inheritance for interactive controls). +// This is the cleaned version of the old subExecute pattern. +func NewLegacy(program string, baseArgs ...string) Player { + return &legacyPlayer{program: program, baseArgs: baseArgs} +} + +type legacyPlayer struct { + program string + baseArgs []string + cmd *exec.Cmd +} + +func (p *legacyPlayer) Play(url string, extra ...string) error { + args := append([]string{}, p.baseArgs...) + args = append(args, extra...) + args = append(args, url) + + p.cmd = exec.Command(p.program, args...) + p.cmd.Stdin = os.Stdin + p.cmd.Stdout = os.Stdout + p.cmd.Stderr = os.Stderr + + if err := p.cmd.Run(); err != nil { + // For interactive players the err is often just "exit status N" from 'q'. + // Surface it but don't treat as fatal for the app. + fmt.Printf("(player exited: %v)\n", err) + } + return nil // we intentionally do not try CombinedOutput after Run +} + +func (p *legacyPlayer) Stop() error { + if p.cmd != nil && p.cmd.Process != nil { + _ = p.cmd.Process.Kill() + } + return nil +} + +// isShellMeta reports if name looks like it contains shell metachars (defense in depth). +func isShellMeta(name string) bool { + return strings.ContainsAny(name, ";|&`$(){}[]<>\n\r\t ") +} + +// isInstalledLegacy is the old body (kept only for reference / windows where.exe). +// New code should use the top-level IsInstalled. +func isInstalledLegacy(name string) bool { + if runtime.GOOS == "windows" { + cmd := exec.Command("where.exe", name) + return cmd.Run() == nil + } + if isShellMeta(name) { + return false // refuse obviously bad names + } + cmd := exec.Command("/bin/sh", "-c", "command -v "+name) // still used on some old paths; prefer LookPath above + return cmd.Run() == nil +} diff --git a/internal/radio/radio.go b/internal/radio/radio.go new file mode 100644 index 0000000..471ff7e --- /dev/null +++ b/internal/radio/radio.go @@ -0,0 +1,160 @@ +package radio + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "math/rand" + "net" + "net/http" + "net/url" + "sync" + "time" + + "github.com/gmgauthier/gostations/internal/config" +) + +// Station is the normalized record returned from radio-browser searches. +type Station struct { + Name string `json:"name"` + Codec string `json:"codec"` + Bitrate json.Number `json:"bitrate"` + Countrycode string `json:"countrycode"` + Tags string `json:"tags"` + Url string `json:"url"` + Lastcheck int `json:"lastcheckok"` + // Future: UUID string `json:"stationuuid"` etc for stable favorites. +} + +var ( + apiHostCache string + apiHostOnce sync.Once + httpClient = &http.Client{Timeout: 12 * time.Second} +) + +// Search queries the radio-browser API (with fixes for the old panics and error handling). +// It respects the cached config for host + limit. +func Search(ctx context.Context, name, country, state, tags string, includeDown bool) ([]Station, error) { + if err := config.Init(); err != nil { + return nil, fmt.Errorf("config: %w", err) + } + + params := url.Values{} + if name != "" { + params.Add("name", name) + } + if country != "" { + params.Add("country", country) + } + if state != "" { + params.Add("state", state) + } + if tags != "" { + params.Add("tag", tags) + } + + host, err := getAPIHost(ctx) + if err != nil { + return nil, fmt.Errorf("resolve api host: %w", err) + } + + qstr := params.Encode() + limit := config.MaxItems() + if limit <= 0 { + limit = 9999 + } + urlstr := fmt.Sprintf("https://%s/json/stations/search?%s&limit=%d", host, qstr, limit) + if qstr == "" { + urlstr = fmt.Sprintf("https://%s/json/stations/search?limit=%d", host, limit) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlstr, nil) + if err != nil { + return nil, err + } + req.Header.Set("User-Agent", "gostations/1 (github.com/gmgauthier/gostations)") + + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("http get %s: %w", urlstr, err) + } + defer func() { + _ = resp.Body.Close() + }() + + if resp.StatusCode != 200 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return nil, fmt.Errorf("api returned %d: %s", resp.StatusCode, string(body)) + } + + var data []Station + if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { + return nil, fmt.Errorf("decode stations: %w", err) + } + + if includeDown { + return data, nil + } + return pruneStations(data), nil +} + +func pruneStations(stations []Station) []Station { + filtered := stations[:0] + for _, s := range stations { + if s.Lastcheck == 1 { + filtered = append(filtered, s) + } + } + return filtered +} + +// getAPIHost resolves (with caching + error handling) the dynamic host from the configured "all.api..." entry. +// This fixes the old panics in RandomIP / reverseLookup / nslookup. +func getAPIHost(ctx context.Context) (string, error) { + var resolveErr error + apiHostOnce.Do(func() { + apiHostCache, resolveErr = resolveAPIHost(ctx) + }) + if apiHostCache == "" && resolveErr != nil { + return "", resolveErr + } + if apiHostCache == "" { + // last resort fallback (common public endpoint) + return "de1.api.radio-browser.info", nil + } + return apiHostCache, nil +} + +func resolveAPIHost(ctx context.Context) (string, error) { + host := config.API() + if host == "" { + host = "all.api.radio-browser.info" + } + + ips, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return "", fmt.Errorf("lookup %s: %w", host, err) + } + if len(ips) == 0 { + return "", errors.New("no IPs resolved for api host") + } + + // simple random among A/AAAA (better than old discarded NewSource) + randIdx := rand.Intn(len(ips)) + ip := ips[randIdx].IP + + names, err := net.DefaultResolver.LookupAddr(ctx, ip.String()) + if err != nil || len(names) == 0 { + // fallback to the original host if reverse fails (some IPs may be private or no PTR) + return host, nil + } + return names[0], nil // usually ends with . +} + +// ResetAPIHostCache is exposed for tests. +func ResetAPIHostCache() { + apiHostOnce = sync.Once{} + apiHostCache = "" +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go new file mode 100644 index 0000000..0c997eb --- /dev/null +++ b/internal/ui/ui.go @@ -0,0 +1,69 @@ +package ui + +import ( + "fmt" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + + "github.com/gmgauthier/gostations/internal/radio" +) + +// App is the root Bubble Tea model for the two-stage UI (selection then playback). +type App struct { + stations []radio.Station + // current view state etc. + width, height int + quitting bool +} + +func NewApp(initial []radio.Station) *App { + return &App{stations: initial} +} + +func (a *App) Init() tea.Cmd { + return nil +} + +func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + switch msg.String() { + case "q", "ctrl+c": + a.quitting = true + return a, tea.Quit + case "enter": + // TODO Phase2: switch to playback view, start player etc. + return a, nil + } + case tea.WindowSizeMsg: + a.width = msg.Width + a.height = msg.Height + } + return a, nil +} + +var ( + titleStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("63")) +) + +func (a *App) View() string { + if a.quitting { + return "Thanks for using GoStations!\n" + } + s := titleStyle.Render("GoStations - Radio Browser") + "\n\n" + s += "Selection view (placeholder). Press enter for playback stub, q to quit.\n" + if len(a.stations) > 0 { + s += fmt.Sprintf("%d stations available (favorites would be pinned here).\n", len(a.stations)) + s += "First: " + a.stations[0].Name + "\n" + } + s += "\n(Full two-stage TUI with bubbles/list + playback controls coming in Phase 2.)\n" + return s +} + +// Run starts the TUI program (alt screen). +func Run(initial []radio.Station) error { + p := tea.NewProgram(NewApp(initial), tea.WithAltScreen()) + _, err := p.Run() + return err +} diff --git a/internal/version/version.go b/internal/version/version.go new file mode 100644 index 0000000..9362435 --- /dev/null +++ b/internal/version/version.go @@ -0,0 +1,17 @@ +package version + +// These vars are set at build time via -ldflags, e.g. +// -ldflags "-X github.com/gmgauthier/gostations/internal/version.Version=0.3 -X .../Commit=$(git rev-list -1 HEAD)" +var ( + Version = "dev" + Commit = "" + BuildDate = "" +) + +// String returns a human-friendly version string. +func String() string { + if Commit != "" { + return Version + "-" + Commit + } + return Version +} diff --git a/radiomenu.go b/radiomenu.go index 57656f2..6f7dcb9 100644 --- a/radiomenu.go +++ b/radiomenu.go @@ -6,6 +6,9 @@ import ( "os" "github.com/dixonwille/wmenu/v5" + + "github.com/gmgauthier/gostations/internal/radio" + playerpkg "github.com/gmgauthier/gostations/internal/player" ) func Short( s string, i int ) string { @@ -21,7 +24,7 @@ func Quit() { os.Exit(0) } -func RadioMenu(stations []stationRecord) *wmenu.Menu { +func RadioMenu(stations []radio.Station) *wmenu.Menu { fmt.Println("...Radio Menu...") menu := wmenu.NewMenu("What is your choice?") menu.Action( @@ -29,8 +32,9 @@ func RadioMenu(stations []stationRecord) *wmenu.Menu { if opts[0].Text == "Quit"{Quit()} val := fmt.Sprintf("%s",opts[0].Value) fmt.Printf("Streaming: " + opts[0].Text + "\n") - stdout, _ := subExecute(player(), options(), val) - fmt.Println(stdout) + leg := playerpkg.NewLegacy(player(), options()) + _ = leg.Play(val) // legacy path; cleaned impl does Run + live stdio (no bogus CombinedOutput) + // no more fmt of stdout (that was the source of stray "[]") err := menu.Run() if err != nil { log.Fatal("Oops! " + err.Error()) diff --git a/stations.go b/stations.go index 2b3dd43..c017a49 100644 --- a/stations.go +++ b/stations.go @@ -1,9 +1,14 @@ package main import ( + "context" "flag" "fmt" "os" + + "github.com/gmgauthier/gostations/internal/config" + playerpkg "github.com/gmgauthier/gostations/internal/player" + "github.com/gmgauthier/gostations/internal/radio" ) var version string @@ -13,8 +18,12 @@ func showVersion() { } func precheck() { - if !isInstalled(player()) { - fmt.Printf("%s is either not installed, or not on your $PATH. Cannot continue.\n", player()) + p := config.MustGet("player.command") // or fall back inside + if p == "" { + p = "mpv" + } + if !playerpkg.IsInstalled(p) { + fmt.Printf("%s is either not installed, or not on your $PATH. Cannot continue.\n", p) os.Exit(1) } } @@ -58,9 +67,19 @@ func main() { precheck() - stations, _ := StationSearch(name, country, state, tags, notok) + if err := config.Init(); err != nil { + fmt.Printf("config init: %v\n", err) + os.Exit(1) + } + + stations, err := radio.Search(context.Background(), name, country, state, tags, notok) + if err != nil { + // graceful: empty list + menu (old code dropped errs; we at least log) + fmt.Printf("warning: station search: %v\n", err) + stations = nil + } menu := RadioMenu(stations) - err := menu.Run() + err = menu.Run() if err != nil { fmt.Println(err.Error()) os.Exit(1)