Skip to content

Commit 2291775

Browse files
somanshreddyclaude
andcommitted
cli: namespace CLI-originated download/encode error codes with cli_ prefix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1fcdae3 commit 2291775

2 files changed

Lines changed: 121 additions & 20 deletions

File tree

cmd/heygen/video_download.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func newVideoDownloadCmd(ctx *cmdContext) *cobra.Command {
105105
})
106106
if err != nil {
107107
return &clierrors.CLIError{
108-
Code: "internal_error",
108+
Code: "cli_response_encode_error",
109109
Message: fmt.Sprintf("failed to encode response: %v", err),
110110
Hint: "Please report this CLI bug with the command you ran.",
111111
ExitCode: clierrors.ExitGeneral,
@@ -128,7 +128,7 @@ func extractAssetURL(raw json.RawMessage, videoID string, info assetInfo) (strin
128128
}
129129
if err := json.Unmarshal(raw, &resp); err != nil {
130130
return "", &clierrors.CLIError{
131-
Code: "response_parse_error",
131+
Code: "cli_response_parse_error",
132132
Message: "failed to parse the video response",
133133
Hint: "The API response could not be parsed. Retry; if it persists, report it (possible CLI/API mismatch).",
134134
ExitCode: clierrors.ExitGeneral,
@@ -191,7 +191,7 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
191191
req, err := http.NewRequestWithContext(ctx, http.MethodGet, videoURL, nil)
192192
if err != nil {
193193
return &clierrors.CLIError{
194-
Code: "response_parse_error",
194+
Code: "cli_response_parse_error",
195195
Message: fmt.Sprintf("the download URL returned by the API is unusable: %v", err),
196196
Hint: "Re-fetch a fresh URL: heygen video get <video_id>",
197197
ExitCode: clierrors.ExitGeneral,
@@ -200,6 +200,10 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
200200

201201
resp, err := downloadClient.Do(req)
202202
if err != nil {
203+
// network_error is intentionally NOT cli_-prefixed. It is a shared code
204+
// also emitted by the API-executor transport path
205+
// (internal/client/executor.go); kept bare so both sites agree. Do not
206+
// prefix one site without the other.
203207
return &clierrors.CLIError{
204208
Code: "network_error",
205209
Message: fmt.Sprintf("failed to download video: %v", err),
@@ -214,14 +218,14 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
214218
// re-fetch); any other status is the asset host (our storage/CDN) failing.
215219
if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound {
216220
return &clierrors.CLIError{
217-
Code: "download_url_expired",
221+
Code: "cli_download_url_expired",
218222
Message: fmt.Sprintf("download link expired or unavailable (HTTP %d)", resp.StatusCode),
219223
Hint: "This download link has expired. Re-fetch a fresh URL: heygen video get <video_id>",
220224
ExitCode: clierrors.ExitGeneral,
221225
}
222226
}
223227
return &clierrors.CLIError{
224-
Code: "asset_download_failed",
228+
Code: "cli_download_failed",
225229
Message: fmt.Sprintf("download failed with HTTP %d", resp.StatusCode),
226230
Hint: "The asset host returned an error fetching the file. This is usually transient — retry shortly.",
227231
ExitCode: clierrors.ExitGeneral,
@@ -234,7 +238,7 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
234238
tmp, err := os.CreateTemp(dir, ".heygen-download-*.tmp")
235239
if err != nil {
236240
return &clierrors.CLIError{
237-
Code: "file_io_error",
241+
Code: "cli_file_io_error",
238242
Message: fmt.Sprintf("failed to create temp file in %q: %v", dir, err),
239243
Hint: "Could not write locally. Check the destination path and free disk space.",
240244
ExitCode: clierrors.ExitGeneral,
@@ -247,16 +251,16 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
247251
if copyErr != nil {
248252
_ = os.Remove(tmpPath)
249253
return &clierrors.CLIError{
250-
Code: "download_interrupted",
254+
Code: "cli_download_interrupted",
251255
Message: fmt.Sprintf("download interrupted: %v", copyErr),
252-
Hint: "The transfer was cut off. Retry; the partial file was cleaned up.",
256+
Hint: "The transfer was cut off. Retry the download. The partial file was cleaned up.",
253257
ExitCode: clierrors.ExitGeneral,
254258
}
255259
}
256260
if closeErr != nil {
257261
_ = os.Remove(tmpPath)
258262
return &clierrors.CLIError{
259-
Code: "file_io_error",
263+
Code: "cli_file_io_error",
260264
Message: fmt.Sprintf("failed to finalize download: %v", closeErr),
261265
Hint: "Could not finalize the file locally. Check free disk space.",
262266
ExitCode: clierrors.ExitGeneral,
@@ -269,7 +273,7 @@ func downloadFile(ctx context.Context, videoURL, dest string) error {
269273
if err := os.Rename(tmpPath, dest); err != nil {
270274
_ = os.Remove(tmpPath)
271275
return &clierrors.CLIError{
272-
Code: "file_io_error",
276+
Code: "cli_file_io_error",
273277
Message: fmt.Sprintf("failed to move download to %q: %v", dest, err),
274278
Hint: "Could not write to the destination path. Check permissions and free disk space.",
275279
ExitCode: clierrors.ExitGeneral,

cmd/heygen/video_download_test.go

Lines changed: 107 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -406,16 +406,16 @@ func TestVideoDownload_DownloadFails(t *testing.T) {
406406
if res.ExitCode != 1 {
407407
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
408408
}
409-
if !strings.Contains(res.Stderr, `"code":"asset_download_failed"`) {
410-
t.Fatalf("stderr should carry asset_download_failed (asset-host 5xx):\n%s", res.Stderr)
409+
if !strings.Contains(res.Stderr, `"code":"cli_download_failed"`) {
410+
t.Fatalf("stderr should carry cli_download_failed (asset-host 5xx):\n%s", res.Stderr)
411411
}
412412
if _, err := os.Stat(dest); !os.IsNotExist(err) {
413413
t.Fatalf("expected no output file, stat err = %v", err)
414414
}
415415
}
416416

417417
// A presigned asset URL that 403s or 404s is expired/revoked — a client-side
418-
// download_url_expired, distinct from the asset host failing (asset_download_failed).
418+
// cli_download_url_expired, distinct from the asset host failing (cli_download_failed).
419419
func TestVideoDownload_ExpiredURL(t *testing.T) {
420420
for _, status := range []int{http.StatusForbidden, http.StatusNotFound} {
421421
t.Run(fmt.Sprintf("HTTP_%d", status), func(t *testing.T) {
@@ -444,8 +444,8 @@ func TestVideoDownload_ExpiredURL(t *testing.T) {
444444
if res.ExitCode != 1 {
445445
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
446446
}
447-
if !strings.Contains(res.Stderr, `"code":"download_url_expired"`) {
448-
t.Fatalf("stderr should carry download_url_expired (HTTP %d on presigned URL):\n%s", status, res.Stderr)
447+
if !strings.Contains(res.Stderr, `"code":"cli_download_url_expired"`) {
448+
t.Fatalf("stderr should carry cli_download_url_expired (HTTP %d on presigned URL):\n%s", status, res.Stderr)
449449
}
450450
if !strings.Contains(res.Stderr, "video get") {
451451
t.Fatalf("expired-URL hint should point to re-fetching via video get:\n%s", res.Stderr)
@@ -455,7 +455,7 @@ func TestVideoDownload_ExpiredURL(t *testing.T) {
455455
}
456456

457457
// The video-get response failing to parse, or yielding an unusable download URL,
458-
// classifies as response_parse_error (not the opaque error code).
458+
// classifies as cli_response_parse_error (not the opaque error code).
459459
func TestVideoDownload_ResponseParseError(t *testing.T) {
460460
t.Run("malformed video-get JSON", func(t *testing.T) {
461461
dest := filepath.Join(t.TempDir(), "x.mp4")
@@ -473,8 +473,8 @@ func TestVideoDownload_ResponseParseError(t *testing.T) {
473473
if res.ExitCode != 1 {
474474
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
475475
}
476-
if !strings.Contains(res.Stderr, `"code":"response_parse_error"`) {
477-
t.Fatalf("stderr should carry response_parse_error:\n%s", res.Stderr)
476+
if !strings.Contains(res.Stderr, `"code":"cli_response_parse_error"`) {
477+
t.Fatalf("stderr should carry cli_response_parse_error:\n%s", res.Stderr)
478478
}
479479
})
480480

@@ -495,8 +495,8 @@ func TestVideoDownload_ResponseParseError(t *testing.T) {
495495
if res.ExitCode != 1 {
496496
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
497497
}
498-
if !strings.Contains(res.Stderr, `"code":"response_parse_error"`) {
499-
t.Fatalf("stderr should carry response_parse_error for an unusable URL:\n%s", res.Stderr)
498+
if !strings.Contains(res.Stderr, `"code":"cli_response_parse_error"`) {
499+
t.Fatalf("stderr should carry cli_response_parse_error for an unusable URL:\n%s", res.Stderr)
500500
}
501501
})
502502
}
@@ -520,3 +520,100 @@ func writeJSON(t *testing.T, w http.ResponseWriter, payload any) {
520520
}
521521
_, _ = w.Write(raw)
522522
}
523+
524+
// Transport failure reaching the asset host classifies as network_error (the
525+
// grandfathered shared transport code, intentionally unprefixed).
526+
func TestVideoDownload_NetworkError(t *testing.T) {
527+
dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
528+
deadURL := dead.URL
529+
dead.Close() // deadURL now refuses connections
530+
531+
dest := filepath.Join(t.TempDir(), "x.mp4")
532+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
533+
if r.URL.Path == "/v3/videos/vid_123" {
534+
writeJSON(t, w, map[string]any{
535+
"data": map[string]any{"video_url": deadURL + "/x.mp4", "status": "completed"},
536+
})
537+
return
538+
}
539+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
540+
}))
541+
defer srv.Close()
542+
543+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
544+
if res.ExitCode != 1 {
545+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
546+
}
547+
if !strings.Contains(res.Stderr, `"code":"network_error"`) {
548+
t.Fatalf("stderr should carry network_error (transport failure):\n%s", res.Stderr)
549+
}
550+
}
551+
552+
// A response that promises more bytes than it delivers, then drops, cuts io.Copy
553+
// off mid-stream and classifies as cli_download_interrupted.
554+
func TestVideoDownload_Interrupted(t *testing.T) {
555+
dest := filepath.Join(t.TempDir(), "x.mp4")
556+
var srv *httptest.Server
557+
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
558+
switch r.URL.Path {
559+
case "/v3/videos/vid_123":
560+
writeJSON(t, w, map[string]any{
561+
"data": map[string]any{"video_url": srv.URL + "/download/x.mp4", "status": "completed"},
562+
})
563+
case "/download/x.mp4":
564+
hj, ok := w.(http.Hijacker)
565+
if !ok {
566+
t.Fatal("test server does not support hijacking")
567+
}
568+
conn, buf, err := hj.Hijack()
569+
if err != nil {
570+
t.Fatalf("hijack: %v", err)
571+
}
572+
// Promise 1000 bytes, send 7, then drop the connection → the client's
573+
// io.Copy hits an unexpected EOF.
574+
_, _ = buf.WriteString("HTTP/1.1 200 OK\r\nContent-Length: 1000\r\n\r\npartial")
575+
_ = buf.Flush()
576+
_ = conn.Close()
577+
default:
578+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
579+
}
580+
}))
581+
defer srv.Close()
582+
583+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
584+
if res.ExitCode != 1 {
585+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
586+
}
587+
if !strings.Contains(res.Stderr, `"code":"cli_download_interrupted"`) {
588+
t.Fatalf("stderr should carry cli_download_interrupted (mid-stream cutoff):\n%s", res.Stderr)
589+
}
590+
}
591+
592+
// A destination whose parent directory does not exist fails temp-file creation
593+
// and classifies as cli_file_io_error.
594+
func TestVideoDownload_FileIOError(t *testing.T) {
595+
dest := filepath.Join(t.TempDir(), "no-such-dir", "x.mp4") // parent dir absent
596+
var srv *httptest.Server
597+
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
598+
switch r.URL.Path {
599+
case "/v3/videos/vid_123":
600+
writeJSON(t, w, map[string]any{
601+
"data": map[string]any{"video_url": srv.URL + "/download/x.mp4", "status": "completed"},
602+
})
603+
case "/download/x.mp4":
604+
w.WriteHeader(http.StatusOK)
605+
_, _ = w.Write([]byte("video-bytes"))
606+
default:
607+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
608+
}
609+
}))
610+
defer srv.Close()
611+
612+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
613+
if res.ExitCode != 1 {
614+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
615+
}
616+
if !strings.Contains(res.Stderr, `"code":"cli_file_io_error"`) {
617+
t.Fatalf("stderr should carry cli_file_io_error (temp-create in missing dir):\n%s", res.Stderr)
618+
}
619+
}

0 commit comments

Comments
 (0)