cli: classify video-download failures into specific codes (#207)

* cli: classify video-download failures into specific codes

Replace the generic `error` on every download-path failure with specific codes:
response_parse_error (unparseable video-get response / unusable URL), network_error
(transport), download_url_expired (403/404 on the presigned asset URL),
asset_download_failed (asset-host 5xx/other), download_interrupted (mid-stream),
file_io_error (local temp/finalize/rename), internal_error (encoding our own output).
Each carries an actionable hint. No exit-code changes.

asset_download_failed is intentionally CLI-specific: the API's own `download_failed`
is a 400 for a bad user-provided input URL (opposite meaning), so reusing it would
collide.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* cli: namespace CLI-originated download/encode error codes with cli_ prefix

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: somanshreddy

cmd/heygen/video_download.go

@@ -104,7 +104,12 @@ func newVideoDownloadCmd(ctx *cmdContext) *cobra.Command {
 				"path":    dest,
 			})
 			if err != nil {
-				return clierrors.New(fmt.Sprintf("failed to encode response: %v", err))
+				return &clierrors.CLIError{
+					Code:     "cli_response_encode_error",
+					Message:  fmt.Sprintf("failed to encode response: %v", err),
+					Hint:     "Please report this CLI bug with the command you ran.",
+					ExitCode: clierrors.ExitGeneral,
+				}
 			}
 
 			return ctx.formatter.Data(data, "", nil)
@@ -122,7 +127,12 @@ func extractAssetURL(raw json.RawMessage, videoID string, info assetInfo) (strin
 		Data map[string]json.RawMessage `json:"data"`
 	}
 	if err := json.Unmarshal(raw, &resp); err != nil {
-		return "", clierrors.New("failed to parse video response")
+		return "", &clierrors.CLIError{
+			Code:     "cli_response_parse_error",
+			Message:  "failed to parse the video response",
+			Hint:     "The API response could not be parsed. Retry; if it persists, report it (possible CLI/API mismatch).",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 
 	var status string
@@ -180,45 +190,94 @@ func assetHint(field string) string {
 func downloadFile(ctx context.Context, videoURL, dest string) error {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, videoURL, nil)
 	if err != nil {
-		return clierrors.New(fmt.Sprintf("failed to build download request: %v", err))
+		return &clierrors.CLIError{
+			Code:     "cli_response_parse_error",
+			Message:  fmt.Sprintf("the download URL returned by the API is unusable: %v", err),
+			Hint:     "Re-fetch a fresh URL: heygen video get <video_id>",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 
 	resp, err := downloadClient.Do(req)
 	if err != nil {
-		return clierrors.New(fmt.Sprintf("failed to download video: %v", err))
+		// network_error is intentionally NOT cli_-prefixed. It is a shared code
+		// also emitted by the API-executor transport path
+		// (internal/client/executor.go); kept bare so both sites agree. Do not
+		// prefix one site without the other.
+		return &clierrors.CLIError{
+			Code:     "network_error",
+			Message:  fmt.Sprintf("failed to download video: %v", err),
+			Hint:     "This is usually a temporary network issue. Check your connection and retry.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return clierrors.New(fmt.Sprintf("download failed with HTTP %d", resp.StatusCode))
+		// A presigned asset URL that 403s/404s is expired or revoked (client can
+		// re-fetch); any other status is the asset host (our storage/CDN) failing.
+		if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound {
+			return &clierrors.CLIError{
+				Code:     "cli_download_url_expired",
+				Message:  fmt.Sprintf("download link expired or unavailable (HTTP %d)", resp.StatusCode),
+				Hint:     "This download link has expired. Re-fetch a fresh URL: heygen video get <video_id>",
+				ExitCode: clierrors.ExitGeneral,
+			}
+		}
+		return &clierrors.CLIError{
+			Code:     "cli_download_failed",
+			Message:  fmt.Sprintf("download failed with HTTP %d", resp.StatusCode),
+			Hint:     "The asset host returned an error fetching the file. This is usually transient — retry shortly.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 
 	// Write to a temp file in the same directory, then rename on success.
 	// This prevents destroying an existing file on partial download failure.
 	dir := filepath.Dir(dest)
 	tmp, err := os.CreateTemp(dir, ".heygen-download-*.tmp")
 	if err != nil {
-		return clierrors.New(fmt.Sprintf("failed to create temp file in %q: %v", dir, err))
+		return &clierrors.CLIError{
+			Code:     "cli_file_io_error",
+			Message:  fmt.Sprintf("failed to create temp file in %q: %v", dir, err),
+			Hint:     "Could not write locally. Check the destination path and free disk space.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 	tmpPath := tmp.Name()
 
 	_, copyErr := io.Copy(tmp, resp.Body)
 	closeErr := tmp.Close()
 	if copyErr != nil {
 		_ = os.Remove(tmpPath)
-		return clierrors.New(fmt.Sprintf("download interrupted: %v", copyErr))
+		return &clierrors.CLIError{
+			Code:     "cli_download_interrupted",
+			Message:  fmt.Sprintf("download interrupted: %v", copyErr),
+			Hint:     "The transfer was cut off. Retry the download. The partial file was cleaned up.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 	if closeErr != nil {
 		_ = os.Remove(tmpPath)
-		return clierrors.New(fmt.Sprintf("failed to finalize download: %v", closeErr))
+		return &clierrors.CLIError{
+			Code:     "cli_file_io_error",
+			Message:  fmt.Sprintf("failed to finalize download: %v", closeErr),
+			Hint:     "Could not finalize the file locally. Check free disk space.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 
 	// Atomic rename. On Windows this may fail if dest is open elsewhere;
 	// os.Rename across filesystems also fails, but temp file is in the
 	// same directory so this is safe.
 	if err := os.Rename(tmpPath, dest); err != nil {
 		_ = os.Remove(tmpPath)
-		return clierrors.New(fmt.Sprintf("failed to move download to %q: %v", dest, err))
+		return &clierrors.CLIError{
+			Code:     "cli_file_io_error",
+			Message:  fmt.Sprintf("failed to move download to %q: %v", dest, err),
+			Hint:     "Could not write to the destination path. Check permissions and free disk space.",
+			ExitCode: clierrors.ExitGeneral,
+		}
 	}
 
 	return nil

cmd/heygen/video_download_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -405,11 +406,101 @@ func TestVideoDownload_DownloadFails(t *testing.T) {
 	if res.ExitCode != 1 {
 		t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
 	}
+	if !strings.Contains(res.Stderr, `"code":"cli_download_failed"`) {
+		t.Fatalf("stderr should carry cli_download_failed (asset-host 5xx):\n%s", res.Stderr)
+	}
 	if _, err := os.Stat(dest); !os.IsNotExist(err) {
 		t.Fatalf("expected no output file, stat err = %v", err)
 	}
 }
 
+// A presigned asset URL that 403s or 404s is expired/revoked — a client-side
+// cli_download_url_expired, distinct from the asset host failing (cli_download_failed).
+func TestVideoDownload_ExpiredURL(t *testing.T) {
+	for _, status := range []int{http.StatusForbidden, http.StatusNotFound} {
+		t.Run(fmt.Sprintf("HTTP_%d", status), func(t *testing.T) {
+			tmpDir := t.TempDir()
+			dest := filepath.Join(tmpDir, "expired.mp4")
+
+			var srv *httptest.Server
+			srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch {
+				case r.Method == http.MethodGet && r.URL.Path == "/v3/videos/vid_123":
+					writeJSON(t, w, map[string]any{
+						"data": map[string]any{
+							"video_url": srv.URL + "/download/expired.mp4",
+							"status":    "completed",
+						},
+					})
+				case r.Method == http.MethodGet && r.URL.Path == "/download/expired.mp4":
+					w.WriteHeader(status)
+				default:
+					t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+				}
+			}))
+			defer srv.Close()
+
+			res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+			if res.ExitCode != 1 {
+				t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+			}
+			if !strings.Contains(res.Stderr, `"code":"cli_download_url_expired"`) {
+				t.Fatalf("stderr should carry cli_download_url_expired (HTTP %d on presigned URL):\n%s", status, res.Stderr)
+			}
+			if !strings.Contains(res.Stderr, "video get") {
+				t.Fatalf("expired-URL hint should point to re-fetching via video get:\n%s", res.Stderr)
+			}
+		})
+	}
+}
+
+// The video-get response failing to parse, or yielding an unusable download URL,
+// classifies as cli_response_parse_error (not the opaque error code).
+func TestVideoDownload_ResponseParseError(t *testing.T) {
+	t.Run("malformed video-get JSON", func(t *testing.T) {
+		dest := filepath.Join(t.TempDir(), "x.mp4")
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.Path == "/v3/videos/vid_123" {
+				w.WriteHeader(http.StatusOK)
+				_, _ = w.Write([]byte("{ this is not valid json"))
+				return
+			}
+			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}))
+		defer srv.Close()
+
+		res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+		if res.ExitCode != 1 {
+			t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+		}
+		if !strings.Contains(res.Stderr, `"code":"cli_response_parse_error"`) {
+			t.Fatalf("stderr should carry cli_response_parse_error:\n%s", res.Stderr)
+		}
+	})
+
+	t.Run("unusable download URL", func(t *testing.T) {
+		dest := filepath.Join(t.TempDir(), "x.mp4")
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.Path == "/v3/videos/vid_123" {
+				writeJSON(t, w, map[string]any{
+					"data": map[string]any{"video_url": "http://%zz", "status": "completed"},
+				})
+				return
+			}
+			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}))
+		defer srv.Close()
+
+		res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+		if res.ExitCode != 1 {
+			t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+		}
+		if !strings.Contains(res.Stderr, `"code":"cli_response_parse_error"`) {
+			t.Fatalf("stderr should carry cli_response_parse_error for an unusable URL:\n%s", res.Stderr)
+		}
+	})
+}
+
 func TestVideoDownload_AuthRequired(t *testing.T) {
 	srv := setupTestServer(t, map[string]testHandler{})
 	defer srv.Close()
@@ -429,3 +520,100 @@ func writeJSON(t *testing.T, w http.ResponseWriter, payload any) {
 	}
 	_, _ = w.Write(raw)
 }
+
+// Transport failure reaching the asset host classifies as network_error (the
+// grandfathered shared transport code, intentionally unprefixed).
+func TestVideoDownload_NetworkError(t *testing.T) {
+	dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	deadURL := dead.URL
+	dead.Close() // deadURL now refuses connections
+
+	dest := filepath.Join(t.TempDir(), "x.mp4")
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/v3/videos/vid_123" {
+			writeJSON(t, w, map[string]any{
+				"data": map[string]any{"video_url": deadURL + "/x.mp4", "status": "completed"},
+			})
+			return
+		}
+		t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+	}))
+	defer srv.Close()
+
+	res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+	if res.ExitCode != 1 {
+		t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, `"code":"network_error"`) {
+		t.Fatalf("stderr should carry network_error (transport failure):\n%s", res.Stderr)
+	}
+}
+
+// A response that promises more bytes than it delivers, then drops, cuts io.Copy
+// off mid-stream and classifies as cli_download_interrupted.
+func TestVideoDownload_Interrupted(t *testing.T) {
+	dest := filepath.Join(t.TempDir(), "x.mp4")
+	var srv *httptest.Server
+	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/v3/videos/vid_123":
+			writeJSON(t, w, map[string]any{
+				"data": map[string]any{"video_url": srv.URL + "/download/x.mp4", "status": "completed"},
+			})
+		case "/download/x.mp4":
+			hj, ok := w.(http.Hijacker)
+			if !ok {
+				t.Fatal("test server does not support hijacking")
+			}
+			conn, buf, err := hj.Hijack()
+			if err != nil {
+				t.Fatalf("hijack: %v", err)
+			}
+			// Promise 1000 bytes, send 7, then drop the connection → the client's
+			// io.Copy hits an unexpected EOF.
+			_, _ = buf.WriteString("HTTP/1.1 200 OK\r\nContent-Length: 1000\r\n\r\npartial")
+			_ = buf.Flush()
+			_ = conn.Close()
+		default:
+			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}
+	}))
+	defer srv.Close()
+
+	res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+	if res.ExitCode != 1 {
+		t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, `"code":"cli_download_interrupted"`) {
+		t.Fatalf("stderr should carry cli_download_interrupted (mid-stream cutoff):\n%s", res.Stderr)
+	}
+}
+
+// A destination whose parent directory does not exist fails temp-file creation
+// and classifies as cli_file_io_error.
+func TestVideoDownload_FileIOError(t *testing.T) {
+	dest := filepath.Join(t.TempDir(), "no-such-dir", "x.mp4") // parent dir absent
+	var srv *httptest.Server
+	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/v3/videos/vid_123":
+			writeJSON(t, w, map[string]any{
+				"data": map[string]any{"video_url": srv.URL + "/download/x.mp4", "status": "completed"},
+			})
+		case "/download/x.mp4":
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte("video-bytes"))
+		default:
+			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}
+	}))
+	defer srv.Close()
+
+	res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
+	if res.ExitCode != 1 {
+		t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, `"code":"cli_file_io_error"`) {
+		t.Fatalf("stderr should carry cli_file_io_error (temp-create in missing dir):\n%s", res.Stderr)
+	}
+}