Better errors and error handling

Fix issue with ignore errors for playlists, now ignore
errors if we get info JSON with a non empty ID.

goutubedl.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -29,13 +30,19 @@ type nopPrinter struct{}
 
 func (nopPrinter) Print(v ...interface{}) {}
 
-// Error youtube-dl specific error
-type Error string
+// YoutubedlError is a error from youtube-dl
+type YoutubedlError string
 
-func (e Error) Error() string {
+func (e YoutubedlError) Error() string {
 	return string(e)
 }
 
+// ErrNotAPlaylist error when single entry when expected a playlist
+var ErrNotAPlaylist = errors.New("single entry when expected a playlist")
+
+// ErrNotASingleEntry error when playlist when expected a single entry
+var ErrNotASingleEntry = errors.New("playlist when expected a single entry")
+
 // Info youtube-dl info
 type Info struct {
 	// Generated from youtube-dl README using:
@@ -241,9 +248,10 @@ func infoFromURL(ctx context.Context, rawURL string, options Options) (info Info
 	cmd := exec.CommandContext(
 		ctx,
 		Path,
+		// see comment below about ignoring errors for playlists
+		"--ignore-errors",
 		"--no-call-home",
 		"--no-cache-dir",
-		"--ignore-errors",
 		"--skip-download",
 		"--restrict-filenames",
 		// provide URL via stdin for security, youtube-dl has some run command args
@@ -302,16 +310,8 @@ func infoFromURL(ctx context.Context, rawURL string, options Options) (info Info
 		}
 	}
 
-	// HACK: --ignore-errors still return error message and exit code != 0
-	// so workaround is to assume things went ok if we get some json on stdout
-	if len(stdoutBuf.Bytes()) == 0 {
-		if errMessage != "" {
-			return Info{}, nil, Error(errMessage)
-		} else if cmdErr != nil {
-			return Info{}, nil, cmdErr
-		}
-	}
-
+	infoSeemsOk := false
+	if len(stdoutBuf.Bytes()) > 0 {
 		if infoErr := json.Unmarshal(stdoutBuf.Bytes(), &info); infoErr != nil {
 			return Info{}, nil, infoErr
 		}
@@ -319,13 +319,28 @@ func infoFromURL(ctx context.Context, rawURL string, options Options) (info Info
 		isPlaylist := info.Type == "playlist" || info.Type == "multi_video"
 		switch {
 		case options.Type == TypePlaylist && !isPlaylist:
-		return Info{}, nil, fmt.Errorf("is not a playlist")
+			return Info{}, nil, ErrNotAPlaylist
 		case options.Type == TypeSingle && isPlaylist:
-		return Info{}, nil, fmt.Errorf("is not a single entry")
+			return Info{}, nil, ErrNotASingleEntry
 		default:
 			// any type
 		}
 
+		// HACK: --ignore-errors still return error message and exit code != 0
+		// so workaround is to assume things went ok if we get some ok json on stdout
+		infoSeemsOk = info.ID != ""
+	}
+
+	if !infoSeemsOk {
+		if errMessage != "" {
+			return Info{}, nil, YoutubedlError(errMessage)
+		} else if cmdErr != nil {
+			return Info{}, nil, cmdErr
+		}
+
+		return Info{}, nil, fmt.Errorf("unknown error")
+	}
+
 	get := func(url string) (*http.Response, error) {
 		c := http.DefaultClient
 		if options.HTTPClient != nil {
@@ -371,7 +386,7 @@ func infoFromURL(ctx context.Context, rawURL string, options Options) (info Info
 		}
 	}
 
-	// as we ignore errors some entries might show up as null
+	// as we ignore errors for playlists some entries might show up as null
 	if options.Type == TypePlaylist {
 		var filteredEntrise []Info
 		for _, e := range info.Entries {

goutubedl_test.go

@@ -181,6 +181,20 @@ func TestPlaylist(t *testing.T) {
 	}
 }
 
+func TestTestUnsupportedURL(t *testing.T) {
+	defer leaktest.Check(t)()
+
+	_, ydlResultErr := New(context.Background(), "https://www.google.com", Options{})
+	if ydlResultErr == nil {
+		t.Errorf("expected unsupported url")
+	}
+	expectedErr := "Unsupported URL: https://www.google.com"
+	if ydlResultErr != nil && ydlResultErr.Error() != expectedErr {
+		t.Errorf("expected error %q got %q", expectedErr, ydlResultErr.Error())
+
+	}
+}
+
 func TestPlaylistWithPrivateVideo(t *testing.T) {
 	defer leaktest.Check(t)()
 
@@ -233,22 +247,26 @@ func TestSubtitles(t *testing.T) {
 	}
 }
 
-func TestErrorIsNotAPlaylist(t *testing.T) {
+func TestErrorNotAPlaylist(t *testing.T) {
+	defer leakChecks(t)()
+
 	_, ydlResultErr := New(context.Background(), testVideoRawURL, Options{
 		Type:              TypePlaylist,
 		DownloadThumbnail: false,
 	})
-	if ydlResultErr.Error() != "is not a playlist" {
-		t.Errorf("expected is playlist error")
+	if ydlResultErr != ErrNotAPlaylist {
+		t.Errorf("expected is playlist error, got %s", ydlResultErr)
 	}
 }
 
-func TestErrorIsNotASingleEntry(t *testing.T) {
+func TestErrorNotASingleEntry(t *testing.T) {
+	defer leakChecks(t)()
+
 	_, ydlResultErr := New(context.Background(), playlistRawURL, Options{
 		Type:              TypeSingle,
 		DownloadThumbnail: false,
 	})
-	if ydlResultErr.Error() != "is not a single entry" {
-		t.Errorf("expected is not a playlist error")
+	if ydlResultErr != ErrNotASingleEntry {
+		t.Errorf("expected is single entry error, got %s", ydlResultErr)
 	}
 }