improve CLI error semantics and soundcloud canonicalization

Auto-upgrade outdated configs on startup, add actionable SSL verification hints in rip error paths, and harden SoundCloud search/metadata with canonical URL handling and richer source IDs.
2026-06-17 15:05:39 +02:00 · 2026-04-20 15:16:59 +02:00
parent 0748d5a325
commit 0ba8faa943
9 changed files with 502 additions and 106 deletions
--- a/cmd/rip/main.go
+++ b/cmd/rip/main.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"html"
 	"io"
@@ -38,11 +39,24 @@ func main() {
 	}
 	if gopts.command == "" {
 		fmt.Println("usage: rip <command>")
-		fmt.Println("commands: url, file, config, database, id, search, lastfm, soundcloud-smoke, qobuz-smoke, qobuz-rip-smoke, qobuz-convert-rip-smoke, qobuz-album-rip-smoke, qobuz-playlist-rip-smoke, qobuz-artist-rip-smoke, qobuz-label-rip-smoke, qobuz-search-smoke, tidal-search-smoke, tidal-metadata-smoke, tidal-video-smoke, tidal-rip-smoke, tidal-album-rip-smoke, tidal-playlist-rip-smoke, tidal-artist-rip-smoke")
+		fmt.Println("commands: url, file, config, database, id, search, lastfm")
+		fmt.Println("tip: run `rip dev-help` to list developer smoke commands")
 		os.Exit(2)
 	}

 	cfg, err := config.Load(gopts.configPath)
+	if err != nil {
+		if errors.Is(err, config.ErrOutdatedConfig) {
+			resolvedPath, upErr := config.UpgradeOutdated(gopts.configPath)
+			if upErr != nil {
+				fmt.Fprintf(os.Stderr, "config error: %v\n", err)
+				fmt.Fprintf(os.Stderr, "config auto-upgrade failed: %v\n", upErr)
+				os.Exit(1)
+			}
+			fmt.Fprintf(os.Stderr, "config upgraded at %s\n", resolvedPath)
+			cfg, err = config.Load(gopts.configPath)
+		}
+	}
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "config error: %v\n", err)
 		os.Exit(1)
@@ -57,6 +71,15 @@ func main() {
 	ctx := context.Background()

 	switch os.Args[1] {
+	case "dev-help":
+		fmt.Println("developer smoke commands:")
+		fmt.Println("  soundcloud-smoke")
+		fmt.Println("  qobuz-smoke, qobuz-rip-smoke, qobuz-convert-rip-smoke")
+		fmt.Println("  qobuz-album-rip-smoke, qobuz-playlist-rip-smoke, qobuz-artist-rip-smoke, qobuz-label-rip-smoke")
+		fmt.Println("  qobuz-search-smoke")
+		fmt.Println("  tidal-search-smoke, tidal-metadata-smoke, tidal-video-smoke")
+		fmt.Println("  tidal-rip-smoke, tidal-album-rip-smoke, tidal-playlist-rip-smoke, tidal-artist-rip-smoke")
+		return
 	case "url":
 		if len(os.Args) < 3 {
 			fmt.Println("usage: rip url <url...> [--force|--ignore-db]")
@@ -94,11 +117,11 @@ func main() {
 		}

 		if err = mainApp.Resolve(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "resolve error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		if err = mainApp.Rip(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		fmt.Printf("url rip complete (%d item(s))\n", added)
@@ -167,11 +190,11 @@ func main() {
 		}

 		if err = mainApp.Resolve(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "resolve error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		if err = mainApp.Rip(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		fmt.Printf("file rip complete (%d item(s))\n", added)
@@ -318,11 +341,11 @@ func main() {
 			os.Exit(1)
 		}
 		if err = mainApp.Resolve(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "resolve error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		if err = mainApp.Rip(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		fmt.Printf("id rip complete: source=%s type=%s id=%s\n", source, mediaType, itemID)
@@ -349,6 +372,10 @@ func main() {
 			fmt.Fprintf(os.Stderr, "search option error: %v\n", err)
 			os.Exit(2)
 		}
+		if sopts.first && sopts.outputFile != "" {
+			fmt.Fprintln(os.Stderr, "cannot choose --first and --output-file together")
+			os.Exit(2)
+		}
 		if !isAllowedSearchSource(source) {
 			fmt.Fprintf(os.Stderr, "unsupported search source %q\n", source)
 			os.Exit(2)
@@ -357,8 +384,8 @@ func main() {
 			fmt.Fprintf(os.Stderr, "unsupported media type %q\n", mediaType)
 			os.Exit(2)
 		}
-		if source == "soundcloud" && mediaType != "track" {
-			fmt.Fprintln(os.Stderr, "soundcloud search currently supports media type track only")
+		if source == "soundcloud" && mediaType != "track" && mediaType != "playlist" {
+			fmt.Fprintln(os.Stderr, "soundcloud search currently supports media types track and playlist")
 			os.Exit(2)
 		}
 		if sopts.query == "" {
@@ -448,11 +475,11 @@ func main() {
 				return
 			}
 			if err = mainApp.Resolve(ctx); err != nil {
-				fmt.Fprintf(os.Stderr, "resolve error: %v\n", err)
+				fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts))
 				os.Exit(1)
 			}
 			if err = mainApp.Rip(ctx); err != nil {
-				fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
+				fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts))
 				os.Exit(1)
 			}
 			fmt.Printf("search download complete (%d item(s))\n", added)
@@ -502,11 +529,11 @@ func main() {
 			return
 		}
 		if err = mainApp.Resolve(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "resolve error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		if err = mainApp.Rip(ctx); err != nil {
-			fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
+			fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts))
 			os.Exit(1)
 		}
 		fmt.Printf("search download complete (%d item(s))\n", added)
@@ -546,22 +573,18 @@ func main() {
 			return
 		}

-		playlistGroups := groupLastFMResolvedTracksBySource(resolvedTracks)
-		addedPlaylists := 0
-		for source, ids := range playlistGroups {
-			playlistID := fmt.Sprintf("lastfm:%s:%s", source, strings.ToLower(strings.ReplaceAll(title, " ", "_")))
-			playlistName := title
-			if len(playlistGroups) > 1 {
-				playlistName = fmt.Sprintf("%s (%s)", title, strings.Title(source))
-			}
-			if addErr := mainApp.AddPlaylistByTrackIDs(ctx, source, playlistID, playlistName, ids); addErr != nil {
-				fmt.Printf("playlist queue failed: source=%s err=%v\n", source, addErr)
-				continue
-			}
-			addedPlaylists++
-			fmt.Printf("queued lastfm playlist: %s (%d tracks, %s)\n", playlistName, len(ids), source)
+		playlistID := fmt.Sprintf("lastfm:%s", strings.ToLower(strings.ReplaceAll(title, " ", "_")))
+		refs := make([]app.PlaylistTrackRef, 0, len(resolvedTracks))
+		for _, item := range resolvedTracks {
+			refs = append(refs, app.PlaylistTrackRef{Source: item.Source, ID: item.ID})
 		}
-		if addedPlaylists == 0 {
+		if addErr := mainApp.AddMixedPlaylistByTrackRefs(ctx, playlistID, title, refs); addErr != nil {
+			fmt.Printf("playlist queue failed: err=%v\n", addErr)
+			fmt.Println("no lastfm playlists queued")
+			return
+		}
+		fmt.Printf("queued lastfm playlist: %s (%d tracks)\n", title, len(refs))
+		if len(refs) == 0 {
 			fmt.Println("no lastfm playlists queued")
 			return
 		}
@@ -573,7 +596,7 @@ func main() {
 			fmt.Fprintf(os.Stderr, "rip error: %v\n", err)
 			os.Exit(1)
 		}
-		fmt.Printf("lastfm rip complete (%d track(s) across %d playlist(s))\n", len(resolvedTracks), addedPlaylists)
+		fmt.Printf("lastfm rip complete (%d track(s) across 1 playlist)\n", len(resolvedTracks))
 	case "soundcloud-smoke":
 		if len(os.Args) < 3 {
 			fmt.Println("usage: rip soundcloud-smoke <soundcloud_url>")
@@ -1309,6 +1332,21 @@ func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) {
 	}
 }

+func errorWithActionableHint(err error, opts globalOptions) string {
+	if err == nil {
+		return ""
+	}
+	msg := err.Error()
+	if opts.noSSLVerify {
+		return msg
+	}
+	lower := strings.ToLower(msg)
+	if strings.Contains(lower, "x509") || strings.Contains(lower, "certificate") || strings.Contains(lower, "tls") || strings.Contains(lower, "ssl") {
+		return msg + " (hint: try again with --no-ssl-verify)"
+	}
+	return msg
+}
+
 func parseSmokeOptions(args []string, minQuality int, maxQuality int) (smokeOptions, error) {
 	opts := smokeOptions{}
 	for _, arg := range args {
@@ -1654,7 +1692,6 @@ func fetchLastFMPlaylist(ctx context.Context, verifySSL bool, playlistURL string
 func fetchLastFMPlaylistViaMirror(ctx context.Context, verifySSL bool, playlistURL string) (string, []lastFMTrack, error) {
 	client := netutil.NewHTTPClient(30*time.Second, verifySSL)
 	all := make([]lastFMTrack, 0, 200)
-	seen := map[string]struct{}{}
 	title := ""

 	for page := 1; page <= 50; page++ {
@@ -1672,17 +1709,8 @@ func fetchLastFMPlaylistViaMirror(ctx context.Context, verifySSL bool, playlistU
 		if len(tracks) == 0 {
 			break
 		}
-		newOnPage := 0
-		for _, tr := range tracks {
-			key := strings.ToLower(strings.TrimSpace(tr.Title + "\x00" + tr.Artist))
-			if _, dup := seen[key]; dup {
-				continue
-			}
-			seen[key] = struct{}{}
-			all = append(all, tr)
-			newOnPage++
-		}
-		if newOnPage == 0 || !strings.Contains(body, "Show more") {
+		all = append(all, tracks...)
+		if !strings.Contains(body, "Show more") {
 			break
 		}
 	}
@@ -1874,19 +1902,6 @@ func resolveLastFMTracks(ctx context.Context, mainApp *app.Main, opts lastFMOpti
 	return resolved, nil
 }

-func groupLastFMResolvedTracksBySource(resolved []resolvedLastFMTrack) map[string][]string {
-	out := map[string][]string{}
-	for _, item := range resolved {
-		source := strings.TrimSpace(item.Source)
-		id := strings.TrimSpace(item.ID)
-		if source == "" || id == "" {
-			continue
-		}
-		out[source] = append(out[source], id)
-	}
-	return out
-}
-
 func fetchSoundcloudOEmbed(ctx context.Context, verifySSL bool, trackURL string) (map[string]any, error) {
 	parsed, err := url.Parse(trackURL)
 	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
@@ -2229,8 +2244,8 @@ func promptSearchInteractive(defaultLimit int) (string, string, searchOptions, e
 			fmt.Println("Invalid media type.")
 			continue
 		}
-		if source == "soundcloud" && mediaType != "track" {
-			fmt.Println("SoundCloud search supports track only.")
+		if source == "soundcloud" && mediaType != "track" && mediaType != "playlist" {
+			fmt.Println("SoundCloud search supports track and playlist only.")
 			continue
 		}

@@ -2379,8 +2394,9 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) []
 				id := asString(itm["id"])
 				title := asString(itm["title"])
 				artist := nestedSearchString(itm, "artist", "name")
+				trackCount := searchInt(itm["tracks_count"])
 				if id != "" && title != "" {
-					results = append(results, searchResult{ID: id, Title: title, Artist: artist})
+					results = append(results, searchResult{ID: id, Title: title, Artist: artist, TrackCount: trackCount})
 				}
 			}
 		}
--- a/cmd/rip/main_test.go
+++ b/cmd/rip/main_test.go
@@ -1,6 +1,9 @@
 package main

-import "testing"
+import (
+	"errors"
+	"testing"
+)

 func TestParseFileInputJSONItems(t *testing.T) {
 	content := []byte(`[
@@ -167,26 +170,6 @@ func TestNormalizeCodecRejectsUnknown(t *testing.T) {
 	}
 }

-func TestGroupLastFMResolvedTracksBySourcePreservesOrderAndDuplicates(t *testing.T) {
-	resolved := []resolvedLastFMTrack{
-		{Source: "tidal", ID: "1"},
-		{Source: "tidal", ID: "1"},
-		{Source: "qobuz", ID: "2"},
-		{Source: "tidal", ID: "3"},
-		{Source: "", ID: "4"},
-	}
-	groups := groupLastFMResolvedTracksBySource(resolved)
-	if len(groups["tidal"]) != 3 {
-		t.Fatalf("tidal ids len = %d, want 3", len(groups["tidal"]))
-	}
-	if len(groups["qobuz"]) != 1 {
-		t.Fatalf("qobuz ids len = %d, want 1", len(groups["qobuz"]))
-	}
-	if groups["tidal"][0] != "1" || groups["tidal"][1] != "1" || groups["tidal"][2] != "3" {
-		t.Fatalf("unexpected tidal ordering: %+v", groups["tidal"])
-	}
-}
-
 func TestExtractLastFMTracksFromMirrorMarkdown(t *testing.T) {
 	md := `Title: My Playlist | user playlists | Last.fm
 | Play | Image | Loved | Name | Artist name | Buy | Options | Duration |
@@ -204,3 +187,29 @@ func TestExtractLastFMTracksFromMirrorMarkdown(t *testing.T) {
 		t.Fatalf("unexpected first track: %+v", tracks[0])
 	}
 }
+
+func TestParseSearchArgsAllowsFirstAndOutputFileButCallerCanReject(t *testing.T) {
+	opts, err := parseSearchArgs([]string{"q", "--first", "--output-file", "/tmp/out.json"}, 20)
+	if err != nil {
+		t.Fatalf("parseSearchArgs() error = %v", err)
+	}
+	if !opts.first || opts.outputFile == "" {
+		t.Fatalf("expected first=true and output file set, got %+v", opts)
+	}
+}
+
+func TestErrorWithActionableHintForSSL(t *testing.T) {
+	err := errors.New("x509: certificate signed by unknown authority")
+	msg := errorWithActionableHint(err, globalOptions{})
+	if msg == err.Error() {
+		t.Fatalf("expected ssl hint in message")
+	}
+}
+
+func TestErrorWithActionableHintNoHintWhenDisabled(t *testing.T) {
+	err := errors.New("tls handshake failure")
+	msg := errorWithActionableHint(err, globalOptions{noSSLVerify: true})
+	if msg != err.Error() {
+		t.Fatalf("unexpected hint when noSSLVerify set")
+	}
+}